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

Change PBES2 PBKDF2 default salt length to 16 bytes. #21858

Closed
wants to merge 6 commits into from

Conversation

slontis
Copy link
Member

@slontis slontis commented Aug 28, 2023

The PKCS5 (RFC 8018) standard uses a 64 bit salt length for PBE, and recommends a minimum of 64 bits for PBES2. For FIPS compliance PBKDF2 requires a salt length of 128 bits.
This affects OpenSSL command line applications such as "genrsa" and "pkcs8" and API's such as PEM_write_bio_PrivateKey() that are reliant on the default value.

The "pkcs8" and "enc" applications now have an extra "saltlen" option which allows the user to set the lenght of the salt.
Note that "genrsa" can only use defaults since it has no way of passing the saltlen down thru the API's.. If you wanted to change the saltlen you could always use "genrsa" followed by a "pkcs8" (with the salt length set to non default).

Checklist
  • documentation is added or updated
  • tests are added or updated

@slontis
Copy link
Member Author

slontis commented Aug 28, 2023

Replaces PR #20783

@slontis slontis added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member labels Aug 28, 2023
@slontis
Copy link
Member Author

slontis commented Aug 28, 2023

Backport to 3.0 and 3.1?
OTC answer was no..

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.

I would be OK with backporting this to 3.1 and 3.0 as well.

@t8m t8m added triaged: bug The issue/pr is/fixes a bug tests: present The PR has suitable tests present labels Aug 28, 2023
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Aug 31, 2023
@slontis slontis requested a review from t8m August 31, 2023 08:04
@slontis
Copy link
Member Author

slontis commented Aug 31, 2023

Rather than have merge issues, I moved the code from the PR that dealt with app options into this PR. These have been added as new commits.. They should address all the requirements requested by the OTC.

@t8m
Copy link
Member

t8m commented Aug 31, 2023

Hmm... interesting. So with enc we are no longer compatible by default with older releases if -pbkdf2 is used? I.e., you would have to know to use -saltlen 8 if you encrypted before? Perhaps the default should be kept as 8?

apps/enc.c Show resolved Hide resolved
The PKCS5 (RFC 8018) standard uses a 64 bit salt length for PBE, and
recommends a minimum of 64 bits for PBES2. For FIPS compliance PBKDF2
requires a salt length of 128 bits.
This affects OpenSSL command line applications such as "genrsa" and "pkcs8"
and API's such as PEM_write_bio_PrivateKey() that are reliant on the
default salt length.
This allows PBKDF2 to change the saltlen to something other than the
new default value of 16. Previously this app hardwired the salt length
to a maximum of 8 bytes. Non PBKDF2 mode uses EVP_BytesToKey()
internally, which is documented to only allow 8 bytes.
@slontis
Copy link
Member Author

slontis commented Aug 31, 2023

Had to rebase, nothing changed other than CHANGES.md

@paulidale paulidale removed the approval: otc review pending This pull request needs review by an OTC member label Aug 31, 2023
@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 labels Sep 1, 2023
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Sep 2, 2023
@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Sep 2, 2023
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@paulidale
Copy link
Contributor

Merged to master. Thanks.

@paulidale paulidale closed this Sep 4, 2023
openssl-machine pushed a commit that referenced this pull request Sep 4, 2023
The PKCS5 (RFC 8018) standard uses a 64 bit salt length for PBE, and
recommends a minimum of 64 bits for PBES2. For FIPS compliance PBKDF2
requires a salt length of 128 bits.
This affects OpenSSL command line applications such as "genrsa" and "pkcs8"
and API's such as PEM_write_bio_PrivateKey() that are reliant on the
default salt length.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #21858)
openssl-machine pushed a commit that referenced this pull request Sep 4, 2023
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #21858)
openssl-machine pushed a commit that referenced this pull request Sep 4, 2023
This allows PBKDF2 to change the saltlen to something other than the
new default value of 16. Previously this app hardwired the salt length
to a maximum of 8 bytes. Non PBKDF2 mode uses EVP_BytesToKey()
internally, which is documented to only allow 8 bytes.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #21858)
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 Merge to master branch severity: fips change The pull request changes FIPS provider sources tests: present The PR has suitable tests present triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants