-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
x/crypto/cryptobyte: "x509: invalid key usage" error caused by strict asn1 bit string parsing #70242
Comments
Also, running openssl asn1parse doesn't show any warning or error: |
EDIT: actually it is fine "It is an error if the BIT STRING is not a whole number of bytes" |
@FiloSottile @golang/security PTAL, the problem is nicely explained. |
Actualy, the spec authors decided that. DER requires unused bits to be set to zero, only BER allows that. X.690:
|
My manual bisection suggests that, if this is indeed a bug, it is present since at least go1.17. |
@zpavlinovic ParseCertificate was rewritten then, mainly a change from
But interestingly, the Lines 202 to 207 in 5123f38
https://github.com/golang/crypto/blob/71ed71b4faf97caafd1863fed003e9ac311f10ee/cryptobyte/asn1.go#L557-L563 |
Go version
go version go1.23.2 darwin/arm64
Output of
go env
in your module/workspace:What did you do?
I received from my bank a x509 certificate in the PEM format to be used to connect to one of their services.
I passed the certificate to the
openssl x509
command-line program to print a textual representation of the certificate: it worked.Then I tried to parse the certificate in go using
x509.ParseCertificate
as shown in the following example program:https://go.dev/play/p/jtIyu3C6fL1
What did you see happen?
The go example program fails with this error:
What did you expect to see?
Because the
openssl
command succeeded, I expected the go program to work as well, and to print the parsed certificate.Here is the output of the
openssl
command, it doesn't have any warning or error.I've tracked down the problem by comparing the execution of my go program in the delve debugger with the execution of the openssl in the lldb debugger.
The error in go happens in the asn1 parser, specifically in the
ReadASN1BitString
function, at this very specific line:https://cs.opensource.google/go/x/crypto/+/71ed71b4faf97caafd1863fed003e9ac311f10ee:cryptobyte/asn1.go;l=561
The result of the expression
bytes[len(bytes)-1]&(1<<paddingBits-1)
is not equal to0
, so theReadASN1BitString
function returnsfalse
(ie: an error occured).(In my specific example,
paddingBits
is equal to5
and bytes is equal to[136]
)My understanding is that the go authors decided that if the padded part of the bit string is not equal to zero, then the function should fail.
In openssl, the equivalent line is this one:
https://github.com/openssl/openssl/blob/e54526413d5ef7c665e25f552f2f01d4352bd33d/crypto/asn1/a_bitstr.c#L121
We see that the openssl library doesn't behave like the go library: instead of failing if the padded part of the bit string is not equal to zero, it sets the value of the padded part to zero.
(In my specific example,
136 & (0xff << 5) == 128
).The rust authors seem to have made the same choice as openssl. See the comment that says
The "unused bits" are set to 0.
in this file:https://docs.rs/der/latest/src/der/asn1/bit_string.rs.html#56
If my analysis is correct, I suggest to relax the ASN1 bit string parser by allowing non-zero padding and then setting it to zero like openssl and rust does.
The text was updated successfully, but these errors were encountered: