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

crypto/ecdsa: the sign method on ecdsa.PrivateKey does not hash #13938

Closed
danielchatfield opened this issue Jan 13, 2016 · 6 comments
Closed

Comments

@danielchatfield
Copy link

The sign function in ecdsa states the following:

Sign signs an arbitrary length hash (which should be the result of hashing a
larger message) using the private key, priv. It returns the signature as a
pair of integers. The security of the private key depends on the entropy of
rand.

However the sign method on ecdsa.PrivateKey uses the msg directly rather than hashing it (and thus ignores the crypto.SignerOpts).

Either the description of the sign function should be changed, or the sign method should perform hashing. I'm not sure what the motivation was for the particular type signature of the sign method but taking a crypto.SignerOpts that is ignored is misleading.

@bradfitz
Copy link
Contributor

/cc @agl

@agl
Copy link
Contributor

agl commented Jan 13, 2016

The argument is called hash and the description says that it "should be the result of hashing a
larger message". Can you suggest a description that that's clearer that the input should already be hashed?

@danielchatfield
Copy link
Author

@agl

The problem is that the sign method on the PrivateKey type doesn't pass in a hash.

@agl
Copy link
Contributor

agl commented Jan 13, 2016

Oh, I see. Fair point; that's generally a bit unclear for Signer as a whole and could be improved.

@agl agl self-assigned this Jan 13, 2016
@rakyll rakyll changed the title The sign method on ecdsa.PrivateKey does not hash crypto/ecdsa: the sign method on ecdsa.PrivateKey does not hash Jan 14, 2016
@bradfitz
Copy link
Contributor

@agl, want to send a documentation CL, or do you want to use or pass opts?

@bradfitz bradfitz added this to the Go1.6Maybe milestone Jan 21, 2016
@gopherbot
Copy link

CL https://golang.org/cl/18856 mentions this issue.

@golang golang locked and limited conversation to collaborators Feb 3, 2017
@rsc rsc unassigned agl Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants