Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed length calculation to successfully parse keys of certificates and added printable string #4

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hasalaha
Copy link

@hasalaha hasalaha commented Oct 4, 2023

I tried ASN1Parser to parse Certificate Data using

SecCertificateCopyNormalizedIssuerSequence, (https://developer.apple.com/documentation/security/2799310-seccertificatecopynormalizedissu)
but the Library crashed.
Same for SecKeyCopyExternalRepresentation (https://developer.apple.com/documentation/security/1643698-seckeycopyexternalrepresentation/)

For the first error I added "Printable String" as DER-Type, for the second I fixed the length calculation. See added Testcase "ParseCertificateTests".

If you like it, please accept pull request.

Copy link
Owner

@DominikHorn DominikHorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Mats,

Its lovely to see that this old project of mine is actively being used. Thank you very much for making the effort to contribute back. This is greatly appreciated.

One small nit: Please keep the formatting consistent for DERParser+TLV.swift. It is hard to see what actually changed from the diff.

In general, LGTM. Especially like the cleanup/refactoring and added test.

@@ -9,87 +9,96 @@ import Foundation
import BigInt

extension DERParser {
/// As documented in https://docs.microsoft.com/en-us/windows/win32/seccertenroll/about-encoded-length-and-value-bytes
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Please keep the formatting consistent. It is hard to see actual changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants