Skip to content

Add support for deterministic signatures #1104

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

Merged
merged 9 commits into from
Jul 24, 2024

Conversation

jschlyter
Copy link
Contributor

First cut at support for deterministic signatures, feedback requested.

@pspacek
Copy link
Collaborator

pspacek commented Jul 22, 2024

FTR Knot DNS does that by default, and we plan on doing so in BIND if OpenSSL version is new enough: https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/9128

@rthalley
Copy link
Owner

I also just consulted some cryptographers and so far everyone prefers deterministic due to risks from problems and even biases in random number generators, so I'm willing to go with "default on" at this point.

@jschlyter
Copy link
Contributor Author

Should we call this option reproducible_signing (as in Knot), deterministic or deterministic_signing?

@jschlyter jschlyter requested review from rthalley and bwelling July 22, 2024 19:51
@rthalley
Copy link
Owner

Should we call this option reproducible_signing (as in Knot), deterministic or deterministic_signing?

I don't feel super strongly, but my preference is "deterministic" as that's what the RFC and crypto experts seem to call it, and omitting "_signing" because it's a parameter to the sign() function

@jschlyter jschlyter requested a review from bwelling July 23, 2024 07:08
@jschlyter
Copy link
Contributor Author

Type check error fixed in #1105

@rthalley
Copy link
Owner

Can you rebase off of the current main and then re-run tests? In addition to the thing which made the CI fail, which may be fixed by 1105, I still saw an instance of deterministic_signing = signing that was making checking unhappy.

@jschlyter
Copy link
Contributor Author

Can you rebase off of the current main and then re-run tests? In addition to the thing which made the CI fail, which may be fixed by 1105, I still saw an instance of deterministic_signing = signing that was making checking unhappy.

All tests succeeds here now.

@codecov-commenter
Copy link

codecov-commenter commented Jul 24, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.86%. Comparing base (1532bff) to head (9a361d9).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1104   +/-   ##
=======================================
  Coverage   93.86%   93.86%           
=======================================
  Files         144      144           
  Lines       13404    13408    +4     
  Branches     2611     2611           
=======================================
+ Hits        12581    12586    +5     
  Misses        486      486           
+ Partials      337      336    -1     
Flag Coverage Δ
unittests 93.82% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rthalley rthalley merged commit 83ad949 into rthalley:main Jul 24, 2024
9 checks passed
@rthalley
Copy link
Owner

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants