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.

Sign up for GitHub

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

Jump to bottom

pubkey_cert_records:decode_extensions throws on malformed extensions #6402

Closed
andreasanta opened this issue Oct 30, 2022 · 2 comments
Closed

pubkey_cert_records:decode_extensions throws on malformed extensions #6402

andreasanta opened this issue Oct 30, 2022 · 2 comments
Assignees
@IngelaAndin
Labels
bug Issue is reported as a bug team:PS Assigned to OTP team PS

Comments

@andreasanta
Copy link
Contributor

andreasanta commented Oct 30, 2022 •
edited
Loading

Describe the bug
When a certificate contains a known extension which is malformed, pubkey_cert_records:decode_extensions throws instead of returning a validation error, or simply skipping the extension. This breaks valid certificate validation paths and does not allow graceful handling of the error.

To Reproduce

public_key:pkix_decode_cert(
   base64:decode(
<<"MIICXDCCAgKgAwIBAgIBATAKBggqhkjOPQQDAjApMRkwFwYDVQQFExBjOTY4NDI4OTMyNzUwOGRiMQwwCgYDVQQMDANURUUwHhcNMjIxMDI5MTczMTA3WhcNMjkwNDE2MjAzNDUzWjAfMR0wGwYDVQQDExRBbmRyb2lkIEtleXN0b3JlIEtleTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABFmIQDus/jIZ0cPnRCITCzUUuCjQBw8MetO6154mmTL8O/fFlGgYkZ6C8jSSntKC/lMwaZHxAgW1AGgoCrPuX5ejggEjMIIBHzALBgNVHQ8EBAMCB4AwCAYDVR0fBAEAMIIBBAYKKwYBBAHWeQIBEQSB9TCB8gIBAgoBAQIBAwoBAQQgyvsSa116xqleaXs6xA84wqpAPWFgaaTjCWBnZpHslmoEADBEv4VFQAQ+MDwxFjAUBAxjb20ud2hhdHNhcHACBA0+oAQxIgQgOYfQQ9EK769ahxCzZxQY/lfg4ZtlPJ34JVj+tf/OXUQweqEFMQMCAQKiAwIBA6MEAgIBAKUIMQYCAQYCAQSqAwIBAb+DdwIFAL+FPQgCBgGEJMweob+FPgMCAQC/hUAqMCgEIFNB5rJkaXmnDldlMAeh8xAWlCHsm92fGlZI91reAFrxAQH/CgEAv4VBBQIDAV+Qv4VCBQIDAxUYMAoGCCqGSM49BAMCA0gAMEUCIF0BwvRQipVoaz5SIhsYbIeK+FHbAjWPgOxWgQ6Juq64AiEA83ZLsK37DjZ/tZNRi271VHQqIU8mdqUIMboVUiy3DaM=">>
   ),
otp).

Expected behavior
Ideally, the extension should not be included in the final parsed result, and optionally an error should be returned.

Affected versions
Erlang/OTP 25 [erts-13.0.2]

Additional context
The certificate above does not adhere to RFC5280 and has an empty cRLDistributionPoints extension. However, the certificate is generated by Android in hardware and it is valid for all other purposes. It is part of a valid path and the remaining extensions are valid. Unfortunately, OTP's behaviour in this case is "fail all" and does not allow us to recover gracefully.
@andreasanta andreasanta added the bug Issue is reported as a bug label Oct 30, 2022
@rickard-green rickard-green added the team:PS Assigned to OTP team PS label Oct 31, 2022
IngelaAndin added a commit to IngelaAndin/otp that referenced this issue Nov 11, 2022
@IngelaAndin
Make encoding errors of known extensions handable by the verify_fun
44cdc54 
close erlang#6402
@IngelaAndin IngelaAndin mentioned this issue Nov 11, 2022
Closed
@IngelaAndin IngelaAndin self-assigned this Nov 11, 2022
IngelaAndin added a commit to IngelaAndin/otp that referenced this issue Nov 22, 2022
@IngelaAndin
Make encoding errors of known extensions handable by the verify_fun
d705f67 
close erlang#6402
IngelaAndin added a commit to IngelaAndin/otp that referenced this issue Nov 22, 2022
@IngelaAndin
public_key: Let user handle known extensions decoding issues
6e3758d 
Make encoding errors of known extensions handable by the verify_fun

close erlang#6402
@IngelaAndin IngelaAndin added the stalled waiting for input by the Erlang/OTP team label Dec 7, 2022
IngelaAndin added a commit to IngelaAndin/otp that referenced this issue Feb 17, 2023
@IngelaAndin
public_key: Move decode of CRLDistributionPoints extension
cd8b628 
As different solutions of verifying certificate revocation exists
move the decode of 'CRLDistributionPoints' so that it will only be decode
when it is acctualy used in the verification process. This would enable
interopability with systems that use certificates with an invalid empty
CRLDistributionPoints extension that they want to ignore and make verification
by other means.

Closes erlang#6402
@IngelaAndin IngelaAndin mentioned this issue Feb 17, 2023
Merged
@IngelaAndin
Copy link
Contributor

@andreasanta would you also test my PR in your environment?

@IngelaAndin IngelaAndin removed the stalled waiting for input by the Erlang/OTP team label Feb 20, 2023
@IngelaAndin
Copy link
Contributor

@andreasanta I think this should solve your problem and it will be merged soon for OTP-25.3

IngelaAndin added a commit that referenced this issue Feb 23, 2023
@IngelaAndin
Merge pull request #6883 from IngelaAndin/ingela/public_key/decode_cr…
205da37 
…l_distpoint_ext_later/GH-6402/OTP-18316

public_key: Move decode of CRLDistributionPoints extension
@voltone voltone mentioned this issue Apr 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@IngelaAndin IngelaAndin

Labels
bug Issue is reported as a bug team:PS Assigned to OTP team PS
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@rickard-green @IngelaAndin @andreasanta