Skip to content

Modify EMSA::config_for_x509 to no longer require a private key#3142

Merged
randombit merged 3 commits intomasterfrom
jack/config-for-x509-wtf
Jan 14, 2023
Merged

Modify EMSA::config_for_x509 to no longer require a private key#3142
randombit merged 3 commits intomasterfrom
jack/config-for-x509-wtf

Conversation

@randombit
Copy link
Copy Markdown
Owner

@randombit randombit commented Dec 29, 2022

In EMSA1 we always now set parameters to empty since that is the convention for DSA, ECDSA, etc. Previously if some other algorithm (eg RSA) was used, the parameters would be set. However the sig_algo_and_padding_ok check already prevented one from using EMSA1 padding with RSA, so there is no change to functionality as a result.

@randombit randombit requested a review from reneme December 29, 2022 15:33
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 29, 2022 •
edited
Loading

Codecov Report

Base: 88.02% // Head: 88.03% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (bf77fda) compared to base (c61f78b).
Patch coverage: 63.15% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3142      +/-   ##
==========================================
+ Coverage   88.02%   88.03%   +0.01%     
==========================================
  Files         599      600       +1     
  Lines       66361    67077     +716     
  Branches     6611     6703      +92     
==========================================
+ Hits        58411    59050     +639     
- Misses       5170     5198      +28     
- Partials     2780     2829      +49
Impacted Files Coverage Δ
src/lib/pk_pad/emsa.cpp 71.83% <0.00%> (ø)
src/lib/pk_pad/emsa_pssr/pssr.cpp 81.60% <33.33%> (ø)
src/lib/pk_pad/emsa_pkcs1/emsa_pkcs1.cpp 77.41% <50.00%> (ø)
src/lib/pk_pad/emsa1/emsa1.cpp 84.44% <80.00%> (+1.11%) ⬆️
src/lib/x509/x509_obj.cpp 83.01% <83.33%> (-0.84%) ⬇️
src/lib/tls/tls13/msg_encrypted_extensions.cpp 70.00% <0.00%> (-20.91%) ⬇️
src/lib/tls/tls_session.cpp 85.71% <0.00%> (-6.81%) ⬇️
src/lib/tls/msg_client_hello.cpp 78.52% <0.00%> (-4.45%) ⬇️
src/fuzzer/mode_padding.cpp 82.69% <0.00%> (-4.35%) ⬇️
src/lib/tls/tls_extensions_cert_status_req.cpp 79.62% <0.00%> (-3.71%) ⬇️
... and 41 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@randombit randombit changed the title Modfiy EMSA::config_for_x509 to no longer require a private key Modify EMSA::config_for_x509 to no longer require a private key Dec 29, 2022
@randombit
Copy link
Copy Markdown
Owner Author

randombit commented Jan 6, 2023

@reneme Ping. Do you think this would help with #3125?

@reneme
Copy link
Copy Markdown
Collaborator

reneme commented Jan 6, 2023

Do you think this would help with #3125?

Yes, I'm pretty sure it will help. Thanks. I'll look deeper into it next week. Frankly, this got swamped by the sessions.

reneme
reneme approved these changes Jan 9, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@reneme reneme left a comment

Choose a reason for hiding this comment

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

This works well for #3125. The only small caveat: The TLS code uses "SHA-1" as a hash function name. And the sanity check in EMSA::config_for_x509() compare it to the result of HashFunction::name() which comes back as "SHA-160". For now, I just mapped the string accordingly.

Let's look into that in #3125 once this is merged.

@randombit randombit merged commit f216fe1 into master Jan 14, 2023
@randombit randombit deleted the jack/config-for-x509-wtf branch January 14, 2023 20:05
@reneme reneme mentioned this pull request Jan 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reneme reneme reneme approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@randombit @codecov-commenter @reneme