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.

Sign up for GitHub

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

Jump to bottom

kdf: add PIN verification key KDF to providers #15968

Closed
wants to merge 10 commits into from
Closed

kdf: add PIN verification key KDF to providers #15968

wants to merge 10 commits into from

Conversation

paulidale
Copy link
Contributor

@paulidale paulidale commented Jul 1, 2021 •
edited

This is in pem/pvkfmt.c currently. Since it is a KDF, it is better located in a provider -- in this case legacy since it is only used if RC4 is available.

This PR is based on #15967, will need to be rebased after that is merged.

The existing test cases exercise this code sufficiently.

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

@paulidale paulidale added branch: master Merge to master branch approval: review pending This pull request needs review by a committer hold: merge after release This pull request must not be merged until after the release from the merge branch is done labels Jul 1, 2021
@paulidale paulidale added this to the Post 3.0.0 milestone Jul 1, 2021
@paulidale paulidale self-assigned this Jul 1, 2021
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Jul 1, 2021
@paulidale paulidale force-pushed the pvkkdf branch 2 times, most recently from a3dd7ec to db76dfd Compare July 5, 2021 01:52
@t8m t8m added the triaged: feature The issue/pr requests/adds a feature label Aug 9, 2021
@paulidale
Copy link
Contributor Author

Rebased to current master

@paulidale paulidale removed the hold: merge after release This pull request must not be merged until after the release from the merge branch is done label Sep 3, 2021
@paulidale
Copy link
Contributor Author

Review required.

@paulidale paulidale added the hold: need otc decision The OTC needs to make a decision label Sep 13, 2021
@paulidale
Copy link
Contributor Author

Was kind of hoping for a 3.0.1/3.1 decisions. Master feels fine for this change.

@paulidale paulidale removed the hold: need otc decision The OTC needs to make a decision label Sep 15, 2021
@paulidale
Copy link
Contributor Author

Ping for review

@paulidale
Copy link
Contributor Author

Rebased to master, still awaiting review.

t8m
t8m requested changes Sep 24, 2021
View reviewed changes
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.

Some things to consider. Also should there be a CHANGES.md entry? We have it for other KDFs.
Also, should we have some test cases in evp tests?

doc/man7/EVP_KDF-PVKKDF.pod Show resolved Hide resolved
providers/implementations/kdfs/pvkkdf.c Show resolved Hide resolved
@paulidale
Copy link
Contributor Author

Good feedback @t8m, I'll address net week.

@paulidale
test: add some PVK KDF unit test cases
e86d7e1 
These cases were generated using OpenSSL.
@paulidale
Copy link
Contributor Author

Feedback addressed, test cases added.

@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 27, 2021
@openssl-machine openssl-machine removed the approval: done This pull request has the required number of approvals label Sep 28, 2021
@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 Sep 28, 2021
openssl-machine pushed a commit that referenced this pull request Sep 28, 2021
@paulidale
kdf: Add PVK KDF to providers.
722fe8e 
Add PIN Verification Key key derevation function to providers.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #15968)
openssl-machine pushed a commit that referenced this pull request Sep 28, 2021
@paulidale
pvk: use PVK KDF
1ffac6c 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #15968)
openssl-machine pushed a commit that referenced this pull request Sep 28, 2021
@paulidale
doc: add page for PVK KDF
fc9eda5 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #15968)
openssl-machine pushed a commit that referenced this pull request Sep 28, 2021
@paulidale
include PVK KDF in legacy provider algorithm list
4667b0f 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #15968)
openssl-machine pushed a commit that referenced this pull request Sep 28, 2021
@paulidale
doc: include PVK KDFdocumentation in build.info
4eb2714 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #15968)
openssl-machine pushed a commit that referenced this pull request Sep 28, 2021
@paulidale
doc: note that these KDFs require the legacy provider to be available
5fae7b4 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #15968)
openssl-machine pushed a commit that referenced this pull request Sep 28, 2021
@paulidale
changes: note that PVK KDF has moved to the legacy provider
c8ffd22 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #15968)
openssl-machine pushed a commit that referenced this pull request Sep 28, 2021
@paulidale
test: add some PVK KDF unit test cases
2d34e5b 
These cases were generated using OpenSSL.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #15968)
@paulidale
Copy link
Contributor Author

Merged to master, thanks for the reviews.

@paulidale paulidale closed this Sep 28, 2021
@paulidale paulidale deleted the pvkkdf branch September 28, 2021 08:10
levitte pushed a commit to openssl/otc that referenced this pull request Feb 23, 2022
@paulidale
remove PVK KDF item, it's done by openssl/openssl#15968
a798216
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@levitte levitte levitte left review comments

@t8m t8m t8m approved these changes

Assignees

@paulidale paulidale

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 triaged: feature The issue/pr requests/adds a feature
Projects
None yet
Milestone
Post 3.0.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@paulidale @openssl-machine @levitte @t8m