Skip to content

Comments

Fix PEM_write_bio_PrivateKey_traditional() to not output PKCS#8#12728

Closed
levitte wants to merge 3 commits intoopenssl:masterfrom
levitte:fix-PEM_write_bio_PrivateKey_traditional-1
Closed

Fix PEM_write_bio_PrivateKey_traditional() to not output PKCS#8#12728
levitte wants to merge 3 commits intoopenssl:masterfrom
levitte:fix-PEM_write_bio_PrivateKey_traditional-1

Conversation

@levitte
Copy link
Member

@levitte levitte commented Aug 27, 2020

PEM_write_bio_PrivateKey_traditional() uses i2d_PrivateKey() to do the
actual encoding to DER. However, i2d_PrivateKey() is a generic
function that will do what it can to produce output according to what
the associated EVP_PKEY_ASN1_METHOD offers. If that method offers a
function 'old_priv_encode', which is expected to produce the
"traditional" encoded form, then i2d_PrivateKey() uses that. If not,
i2d_PrivateKey() will go on and used more modern methods, which are
all expected to produce PKCS#8.

To ensure that PEM_write_bio_PrivateKey_traditional() never produces
more modern encoded forms, an extra check that 'old_priv_encode' is
non-NULL is added. If it is NULL, an error is returned.

PEM_write_bio_PrivateKey_traditional() uses i2d_PrivateKey() to do the
actual encoding to DER.  However, i2d_PrivateKey() is a generic
function that will do what it can to produce output according to what
the associated EVP_PKEY_ASN1_METHOD offers.  If that method offers a
function 'old_priv_encode', which is expected to produce the
"traditional" encoded form, then i2d_PrivateKey() uses that.  If not,
i2d_PrivateKey() will go on and used more modern methods, which are
all expected to produce PKCS#8.

To ensure that PEM_write_bio_PrivateKey_traditional() never produces
more modern encoded forms, an extra check that 'old_priv_encode' is
non-NULL is added.  If it is NULL, an error is returned.
@levitte levitte added branch: master Applies to master branch approval: review pending This pull request needs review by a committer branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) labels Aug 27, 2020
@levitte
Copy link
Member Author

levitte commented Aug 27, 2020

A backport to 1.1.1 will come in a separate PR

@levitte
Copy link
Member Author

levitte commented Aug 27, 2020

Backport to 1.1.1 in #12729

@levitte
Copy link
Member Author

levitte commented Aug 27, 2020

... and with this, we discover that not all of our EVP_PKEY_ASN1_METHOD instances support "traditional" PEM or DER output... One example: DH

Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved if CI passes.

@t8m t8m added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) labels Aug 27, 2020
@levitte levitte modified the milestone: 3.0.0 Aug 28, 2020
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Aug 28, 2020
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Aug 28, 2020
@levitte
Copy link
Member Author

levitte commented Aug 28, 2020

Merged

87d91d2 Fix PEM_write_bio_PrivateKey_traditional() to not output PKCS#8
bddfea0 TEST: Adapt some tests for a stricter PEM_write_bio_PrivateKey_traditional()

@levitte levitte closed this Aug 28, 2020
openssl-machine pushed a commit that referenced this pull request Aug 28, 2020
PEM_write_bio_PrivateKey_traditional() uses i2d_PrivateKey() to do the
actual encoding to DER.  However, i2d_PrivateKey() is a generic
function that will do what it can to produce output according to what
the associated EVP_PKEY_ASN1_METHOD offers.  If that method offers a
function 'old_priv_encode', which is expected to produce the
"traditional" encoded form, then i2d_PrivateKey() uses that.  If not,
i2d_PrivateKey() will go on and used more modern methods, which are
all expected to produce PKCS#8.

To ensure that PEM_write_bio_PrivateKey_traditional() never produces
more modern encoded forms, an extra check that 'old_priv_encode' is
non-NULL is added.  If it is NULL, an error is returned.

Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #12728)
openssl-machine pushed a commit that referenced this pull request Aug 28, 2020
…ional()

- test/endecode_test.c

Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #12728)
@levitte levitte deleted the fix-PEM_write_bio_PrivateKey_traditional-1 branch August 28, 2020 18:49
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
PEM_write_bio_PrivateKey_traditional() uses i2d_PrivateKey() to do the
actual encoding to DER.  However, i2d_PrivateKey() is a generic
function that will do what it can to produce output according to what
the associated EVP_PKEY_ASN1_METHOD offers.  If that method offers a
function 'old_priv_encode', which is expected to produce the
"traditional" encoded form, then i2d_PrivateKey() uses that.  If not,
i2d_PrivateKey() will go on and used more modern methods, which are
all expected to produce PKCS#8.

To ensure that PEM_write_bio_PrivateKey_traditional() never produces
more modern encoded forms, an extra check that 'old_priv_encode' is
non-NULL is added.  If it is NULL, an error is returned.

Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#12728)
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
…ional()

- test/endecode_test.c

Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#12728)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants