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

crypto/x509: reject BMPStrings that use invalid characters #71862

Open
rolandshoemaker opened this issue Feb 20, 2025 · 3 comments
Open

crypto/x509: reject BMPStrings that use invalid characters #71862

rolandshoemaker opened this issue Feb 20, 2025 · 3 comments
Assignees
Labels
BugReport Issues describing a possible bug in the Go implementation. NeedsFix The path to resolution is known, but the work has not been done.

Comments

@rolandshoemaker
Copy link
Member

The BMPString type is a UCS-2 encoded string. UCS-2 is a defunct uint16 code point based string encoding that was subsumed into UTF-16. There were a large number of unused code points in UCS-2 that became surrogate points in UTF-16 (characters which used multiple code points).

In our BMPString "parser", we just decode the string as UTF-16, which is mostly okay, except it accepts surrogate characters, which should be rejected. This can lead to confusing behavior where we parse an invalid BMPString when we should reject it.

BMPString is basically unused in the webpki (it's disallowed by the CABF BRs), and 5280 only allows it for backwards compatibility with old DNs. We should just reject invalid BMPStrings. We could also consider removing BMPString support entirely.

Thanks to Jinfeng Guo for reporting this issue.

@rolandshoemaker rolandshoemaker added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 20, 2025
@rolandshoemaker rolandshoemaker self-assigned this Feb 20, 2025
@mateusz834
Copy link
Member

mateusz834 commented Feb 20, 2025

@rolandshoemaker There is also T61String (aka TeletexString), which (i think) no one knows what it really is. See mine CL 487755 which didn't get much attention.

@rolandshoemaker
Copy link
Member Author

Oh thanks for the pointer, I completely missed that CL. Will take a closer look.

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Feb 20, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/651275 mentions this issue: crypto/x509,ecoding/asn1: better handling of weird encodings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants