Skip to content

Conversation

@janimo
Copy link

@janimo janimo commented Sep 21, 2021

This is a really minimal (incomplete?) change to allow V3 certificates without extensions (like those used by Tor).
Probably the MissingOrMalformedExtensions error should be renamed to MalformedExtensions if this looks ok?

Copy link

@robin-kunzler robin-kunzler left a comment

Choose a reason for hiding this comment

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

@janimo : Thanks a lot for addressing this and creating the PR.

This looks good, and it solves the issue for me (it accepts the certificates without extensions that I would like to use).

Regarding the 'completeness' of this change:

  • I looked into the X509 certificate structure and noticed that after the 'subjectPublicKeyInfo' field, the certificate might optionally contain 'issuerUniqueID' and 'subjectUniqueID'. See here. This would still appear before the 'extensions'.
  • Thus, theoretically, a certificate without extensions that has one of the UniqueID fields would be rejected. However, AFAIU such certificates were also rejected before this change, since the code assumes them to be absent and tries to parse anything that comes at this point as extensions.
  • This behaviour is probably fine since the UniqueIDs are discouraged (see here). The RFC recommends to be capable of parsing them, but that should be considered a separate issue that may not be so important, IMO.

@briansmith : Does this change look reasonable to you?

Copy link
Owner

@briansmith briansmith left a comment

Choose a reason for hiding this comment

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

This looks good to me except it needs a test that verifies that an end-entity certificate with no extensions works in all aspects except name validation.

@janimo
Copy link
Author

janimo commented Dec 11, 2021

I've addressed the two comments. AIUI extra test cases apart from the current simple one are blocked on #248 ?

@djc
Copy link

djc commented Feb 10, 2023

@janimo would you be interested in rebasing this PR and submitting a PR to rustls/webpki?

If not, would you mind if someone else takes your PR and rebases it?

@janimo
Copy link
Author

janimo commented Feb 10, 2023

@djc hello, I cannot do it in the next few days, so anyone is welcome to adapt it. Thanks.

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.

4 participants