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

New DSA signing API for #1648 #2642

Closed
wants to merge 1 commit into from

Conversation

joernheissler
Copy link

No description provided.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @reaperhulk, @public and @alex to be potential reviewers

>>> digest = digester.finalize()
>>> signature = private_key.sign(digest, already_hashed=True)

Specifying the per-message key k:
Copy link
Member

Choose a reason for hiding this comment

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

Under what scenario do you want the ability to set k manually? I realize there's a warning below, but this API is incredibly dangerous.

Copy link
Member

Choose a reason for hiding this comment

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

I would say setting k is out of scope for this API concern (and separately,
we should never support it)

On Wed, Jan 6, 2016 at 12:49 PM, Paul Kehrer notifications@github.com
wrote:

In docs/hazmat/primitives/asymmetric/dsa.rst
#2642 (comment):

+Signing a precomputed digest:
+
+.. warning::
+

  • Only use this for digests, do NOT sign raw data. The data will get truncated
  • to the size of DSA's "q" parameter, which is just a few bytes.

+.. doctest::
+

  • digester = hashes.Hash(hashes.SHA256(), backend=default_backend())

  • digester.update(data)

  • digest = digester.finalize()

  • signature = private_key.sign(digest, already_hashed=True)

+Specifying the per-message key k:

Under what scenario do you want the ability to set k manually? I realize
there's a warning below, but this API is incredibly dangerous.


Reply to this email directly or view it on GitHub
https://github.com/pyca/cryptography/pull/2642/files#r48986337.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

Copy link
Author

Choose a reason for hiding this comment

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

For debugging and educational purposes. And perhaps to allow users to implement stuff like rfc6979. Openssl implemented something similar which uses the private key + message digest do seed a PRNG. But it's non-deterministic. I'm not saying that this approach is bad, but it's not rfc6979.

What's wrong with supporting a dangerous API? For one, it's all in "hazmat". And with the big fat warning even a stupid user shouldn't accidentally use this function.
Perhaps change "warning" to "dangerous"?

Copy link
Member

Choose a reason for hiding this comment

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

There's no reason for it. pyca/cryptography, we shouldn't be giving people
more tools to shoot themselves in the foot than is absolutely necessary.

On Wed, Jan 6, 2016 at 1:33 PM, joernheissler notifications@github.com
wrote:

In docs/hazmat/primitives/asymmetric/dsa.rst
#2642 (comment):

+Signing a precomputed digest:
+
+.. warning::
+

  • Only use this for digests, do NOT sign raw data. The data will get truncated
  • to the size of DSA's "q" parameter, which is just a few bytes.

+.. doctest::
+

  • digester = hashes.Hash(hashes.SHA256(), backend=default_backend())

  • digester.update(data)

  • digest = digester.finalize()

  • signature = private_key.sign(digest, already_hashed=True)

+Specifying the per-message key k:

For debugging and educational purposes. And perhaps to allow users to
implement stuff like rfc6979. Openssl implemented something similar which
uses the private key + message digest do seed a PRNG. But it's
non-deterministic. I'm not saying that this approach is bad, but it's not
rfc6979.

What's wrong with supporting a dangerous API? For one, it's all in
"hazmat". And with the big fat warning even a stupid user shouldn't
accidentally use this function.
Perhaps change "warning" to "dangerous"?


Reply to this email directly or view it on GitHub
https://github.com/pyca/cryptography/pull/2642/files#r48991323.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

@reaperhulk
Copy link
Member

This has been partially addressed by #2945 (in terms of API design, although that only implements RSA and does not allow prehashing at this time). I'm going to go ahead and close this PR and if we want to pursue this further we can discuss it on a new and more narrowly focused PR.

@reaperhulk reaperhulk closed this Jun 4, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants