Fix d2i_AutoPrivateKey_ex so that is uses the new decoder (and produces non legacy keys). by slontis · Pull Request #13591 · openssl/openssl

slontis · 2020-12-02T08:00:46Z

Checklist

documentation is added or updated
tests are added or updated

slontis · 2020-12-02T08:04:27Z

@levitte - this produces an interesting failure in the fuzz tests..

dumpasn1 fuzz/corpora/asn1/2b5bdcbf1810066fcc04831b9b60365150e5340c
0 18: SEQUENCE {
2 9: SEQUENCE {
4 7: OBJECT IDENTIFIER dsaWithSha1 (1 2 840 10040 4 3)
: }
13 5: BIT STRING
: '11000100000000001000000001000000'B
: }

Basically there are no domain params in this data so when it goes to cache it tries to access dsa->param and falls over..
I am not sure where we need to check for bad data during the decoding process, but this is obviously a bit of a problem.

crypto/asn1/d2i_pr.c

kroeckx · 2020-12-02T10:03:27Z

Why doesn't the ci fuzz target complain about the issue?

slontis · 2020-12-02T10:29:05Z

Why doesn't the ci fuzz target complain about the issue?

Not sure. It was failing locally..

kroeckx · 2020-12-02T10:37:35Z

I've opened an issue at google/oss-fuzz#4767

crypto/evp/p_lib.c

I approved too soon... CIs are unhappy

slontis · 2020-12-04T06:48:35Z

They are unhappy because of the issue I just reported :)

slontis · 2020-12-07T22:43:39Z

CI loop is happy now that the DSA_size() NULL checks have been added.

openssl-machine · 2020-12-09T07:00:14Z

This pull request is ready to merge

jon-oracle · 2021-01-15T01:43:19Z

Ping

slontis · 2021-01-20T05:32:13Z

ping

slontis · 2021-02-08T06:53:26Z

ping

slontis · 2021-02-08T06:59:24Z

looks like this doesnt build now :)

slontis · 2021-02-08T23:11:08Z

Rebase and removed duplicate function evp_pkey_type2name.

non legacy keys). Fixes openssl#13522

slontis · 2021-02-12T05:59:10Z

Rebased again..
ping.

slontis · 2021-02-16T04:25:11Z

ping

openssl-machine · 2021-02-19T07:00:16Z

This pull request is ready to merge

non legacy keys). Fixes #13522 Reviewed-by: Paul Dale <[email protected]> (Merged from #13591)

slontis · 2021-02-19T09:26:09Z

Thanks for reviewing. Merged to master.

slontis added branch: master Applies to master branch triaged: OTC evaluated labels Dec 2, 2020

slontis added this to the 3.0.0 beta1 milestone Dec 2, 2020

slontis commented Dec 2, 2020

View reviewed changes

crypto/asn1/d2i_pr.c Outdated Show resolved Hide resolved

slontis commented Dec 2, 2020

View reviewed changes

crypto/asn1/d2i_pr.c Outdated Show resolved Hide resolved

slontis changed the title ~~Fix d2i_AutoPrivateKey_ex so that is uses the new decoder (and produces non legay keys).~~ Fix d2i_AutoPrivateKey_ex so that is uses the new decoder (and produces non legacy keys). Dec 2, 2020

levitte requested changes Dec 2, 2020

View reviewed changes

crypto/asn1/d2i_pr.c Outdated Show resolved Hide resolved

crypto/asn1/d2i_pr.c Outdated Show resolved Hide resolved

levitte reviewed Dec 2, 2020

View reviewed changes

crypto/asn1/d2i_pr.c Outdated Show resolved Hide resolved

levitte reviewed Dec 2, 2020

View reviewed changes

crypto/asn1/d2i_pr.c Outdated Show resolved Hide resolved

kroeckx mentioned this pull request Dec 2, 2020

CIFuzz doesn't detect a problem it should google/oss-fuzz#4767

Closed

slontis mentioned this pull request Dec 4, 2020

OSSL_DECODER segfaults if passed a pkcs8 (x509) public key. #13610

Closed

levitte reviewed Dec 4, 2020

View reviewed changes

crypto/evp/p_lib.c Outdated Show resolved Hide resolved

levitte previously approved these changes Dec 4, 2020

View reviewed changes

levitte added approval: done This pull request has the required number of approvals and removed approval: done This pull request has the required number of approvals labels Dec 4, 2020

slontis closed this Dec 7, 2020

slontis reopened this Dec 7, 2020

levitte approved these changes Dec 8, 2020

View reviewed changes

levitte added the approval: done This pull request has the required number of approvals label Dec 8, 2020

openssl-machine removed the approval: done This pull request has the required number of approvals label Dec 9, 2020

openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Dec 9, 2020

t8m added the hold: discussion The community needs to establish a consensus how to move forward with the issue or PR label Dec 9, 2020

t8m added approval: review pending This pull request needs review by a committer and removed approval: ready to merge The 24 hour grace period has passed, ready to merge labels Jan 18, 2021

levitte mentioned this pull request Jan 25, 2021

removal of internal OSSL dependencies open-quantum-safe/openssl#267

Merged

slontis closed this Feb 1, 2021

slontis reopened this Feb 1, 2021

paulnelsontx self-assigned this Feb 4, 2021

slontis closed this Feb 8, 2021

slontis reopened this Feb 8, 2021

slontis force-pushed the d2i_autoprivatekey_prov branch from 73d58d1 to b3a0fc1 Compare February 8, 2021 23:09

Fix d2i_AutoPrivateKey_ex so that is uses the new decoder (and produces

22ed598

non legacy keys). Fixes openssl#13522

slontis force-pushed the d2i_autoprivatekey_prov branch from b3a0fc1 to 22ed598 Compare February 12, 2021 00:41

slontis closed this Feb 12, 2021

slontis reopened this Feb 12, 2021

jon-oracle mentioned this pull request Feb 16, 2021

Make d2i_PUBKEY_ex generate new style EVP_PKEYs #14197

Closed

paulidale approved these changes Feb 18, 2021

View reviewed changes

paulidale added the approval: done This pull request has the required number of approvals label Feb 18, 2021

openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Feb 19, 2021

slontis removed the approval: review pending This pull request needs review by a committer label Feb 19, 2021

openssl-machine pushed a commit that referenced this pull request Feb 19, 2021

Fix d2i_AutoPrivateKey_ex so that is uses the new decoder (and produces

576892d

non legacy keys). Fixes #13522 Reviewed-by: Paul Dale <[email protected]> (Merged from #13591)

slontis closed this Feb 19, 2021

Uh oh!

Conversation

slontis commented Dec 2, 2020

Checklist

Uh oh!

slontis commented Dec 2, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kroeckx commented Dec 2, 2020

Uh oh!

slontis commented Dec 2, 2020

Uh oh!

kroeckx commented Dec 2, 2020

Uh oh!

Uh oh!

slontis commented Dec 4, 2020

Uh oh!

slontis commented Dec 7, 2020

Uh oh!

openssl-machine commented Dec 9, 2020

Uh oh!

jon-oracle commented Jan 15, 2021

Uh oh!

slontis commented Jan 20, 2021

Uh oh!

slontis commented Feb 8, 2021

Uh oh!

slontis commented Feb 8, 2021

Uh oh!

slontis commented Feb 8, 2021

Uh oh!

slontis commented Feb 12, 2021

Uh oh!

slontis commented Feb 16, 2021

Uh oh!

openssl-machine commented Feb 19, 2021

Uh oh!

slontis commented Feb 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants