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

Openssl 1.1.1-pre9 no longer accepts RSA private key accepted by openssl-1.1.0i #7134

Closed
DirtYiCE opened this Issue Sep 6, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@DirtYiCE

DirtYiCE commented Sep 6, 2018

We have a few RSA private keys where integer 0 was serialized as 02 00 instead of 02 01 00. As far as I know, only the later is correct, but openssl 1.1.0 accepted these private keys, while in 1.1.1 they fail with illegal zero content.

Example private key:

-----BEGIN RSA PRIVATE KEY-----
MBoCAAIBDwIBAgIBCAIBAwIBBQIBAAIBAAIBAg==
-----END RSA PRIVATE KEY-----

Run openssl pkey with the above key. With openssl 1.1.0i, it successfuly parses it and re-serializes it:

-----BEGIN PRIVATE KEY-----                                                                                                                                                                                                                                                                                                   
MDECAQAwDQYJKoZIhvcNAQEBBQAEHTAbAgEAAgEPAgECAgEIAgEDAgEFAgEAAgEA
AgEC
-----END PRIVATE KEY-----

With openssl-1.1.1-pre9 or git master, I get:

140017262183424:error:0D0E20DE:asn1 encoding routines:c2i_ibuf:illegal zero content:crypto/asn1/a_int.c:154:
140017262183424:error:0D08303A:asn1 encoding routines:asn1_template_noexp_d2i:nested asn1 error:crypto/asn1/tasn_dec.c:627:Field=version, Type=RSAPrivateKey
140017262183424:error:04093004:rsa routines:old_rsa_priv_decode:RSA lib:crypto/rsa/rsa_ameth.c:130:
140017262183424:error:0D0E20DE:asn1 encoding routines:c2i_ibuf:illegal zero content:crypto/asn1/a_int.c:154:
140017262183424:error:0D08303A:asn1 encoding routines:asn1_template_noexp_d2i:nested asn1 error:crypto/asn1/tasn_dec.c:627:Field=version, Type=PKCS8_PRIV_KEY_INFO
140017262183424:error:0907B00D:PEM routines:PEM_read_bio_PrivateKey:ASN1 lib:crypto/pem/pem_pkey.c:88:

It looks like this change was introduced in 6a32a3c, the LONG parsing functions didn't check for zero length and accepted them while the INT32 parsing functions fails with zero length.

@mattcaswell mattcaswell added this to the 1.1.1 milestone Sep 6, 2018

@levitte levitte self-assigned this Sep 6, 2018

@levitte

This comment has been minimized.

Member

levitte commented Sep 6, 2018

Ok, checking this out.

@levitte

This comment has been minimized.

Member

levitte commented Sep 6, 2018

Your analysis is correct, that is the causing commit. I think that the easiest (and best) way to fix this is to make it so uint64_c2i accepts zero length integers, even though that's not a permitted encoding, but for general backward compatibility. uint64_i2c should remain untouched, so new numbers get correctly encoded.

levitte added a commit to levitte/openssl that referenced this issue Sep 6, 2018

ASN.1 DER: Make INT32 / INT64 types read badly encoded LONG zeroes
The deprecated ASN.1 type LONG / ZLONG (incorrectly) produced zero
length INTEGER encoding for zeroes.  For the sake of backward
compatibility, we allow those to be read without fault when using the
replacement types INT32 / UINT32 / INT64 / UINT64.

Fixes openssl#7134
@levitte

This comment has been minimized.

Member

levitte commented Sep 6, 2018

I think #7144 fixes this.

@mattcaswell mattcaswell modified the milestones: 1.1.1, Assessed Sep 6, 2018

levitte added a commit to levitte/openssl that referenced this issue Sep 8, 2018

levitte added a commit to levitte/openssl that referenced this issue Sep 8, 2018

ASN.1 DER: Make INT32 / INT64 types read badly encoded LONG zeroes
The deprecated ASN.1 type LONG / ZLONG (incorrectly) produced zero
length INTEGER encoding for zeroes.  For the sake of backward
compatibility, we allow those to be read without fault when using the
replacement types INT32 / UINT32 / INT64 / UINT64.

Fixes openssl#7134

levitte added a commit to levitte/openssl that referenced this issue Sep 8, 2018

ASN.1 DER: Make INT32 / INT64 types read badly encoded LONG zeroes
The deprecated ASN.1 type LONG / ZLONG (incorrectly) produced zero
length INTEGER encoding for zeroes.  For the sake of backward
compatibility, we allow those to be read without fault when using the
replacement types INT32 / UINT32 / INT64 / UINT64.

Fixes openssl#7134

levitte added a commit to levitte/openssl that referenced this issue Sep 8, 2018

ASN.1 DER: Make INT32 / INT64 types read badly encoded LONG zeroes
The deprecated ASN.1 type LONG / ZLONG (incorrectly) produced zero
length INTEGER encoding for zeroes.  For the sake of backward
compatibility, we allow those to be read without fault when using the
replacement types INT32 / UINT32 / INT64 / UINT64.

Fixes openssl#7134

levitte added a commit that referenced this issue Sep 9, 2018

TESTS: add test of decoding of invalid zero length ASN.1 INTEGER zero
Confirms #7134

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #7153)

@levitte levitte closed this in ca89174 Sep 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment