-
Notifications
You must be signed in to change notification settings - Fork 21
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
WIP: Defining an interface for allowing signing and verifying with different envelope types #109
Changes from all commits
5d16222
23519fa
3a873b2
5cd7d16
6042363
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,10 +15,11 @@ | |
package cryptoutil | ||
|
||
import ( | ||
"context" | ||
"crypto" | ||
"crypto/rand" | ||
"crypto/rsa" | ||
"io" | ||
"crypto/x509" | ||
) | ||
|
||
type RSASigner struct { | ||
|
@@ -34,8 +35,16 @@ func (s *RSASigner) KeyID() (string, error) { | |
return GeneratePublicKeyID(&s.priv.PublicKey, s.hash) | ||
} | ||
|
||
func (s *RSASigner) Sign(r io.Reader) ([]byte, error) { | ||
digest, err := Digest(r, s.hash) | ||
func (s *RSASigner) Algorithm() (x509.PublicKeyAlgorithm, crypto.Hash) { | ||
return x509.RSA, s.hash | ||
} | ||
|
||
func (s *RSASigner) Signer() (crypto.Signer, error) { | ||
return s.priv, nil | ||
} | ||
|
||
func (s *RSASigner) Sign(ctx context.Context, data []byte) ([]byte, error) { | ||
digest, err := DigestBytes(data, s.hash) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -65,8 +74,8 @@ func (v *RSAVerifier) KeyID() (string, error) { | |
return GeneratePublicKeyID(v.pub, v.hash) | ||
} | ||
|
||
func (v *RSAVerifier) Verify(data io.Reader, sig []byte) error { | ||
digest, err := Digest(data, v.hash) | ||
func (v *RSAVerifier) Verify(ctx context.Context, data []byte, sig []byte) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how will using []byte vs io.Reader effect memory usage? Verify is likely to be used by a service There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good question - one that I haven't thought of. The main motivation was to streamline it with the secure systems lab dsse implementation. |
||
digest, err := DigestBytes(data, v.hash) | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -79,6 +88,10 @@ func (v *RSAVerifier) Verify(data io.Reader, sig []byte) error { | |
return rsa.VerifyPSS(v.pub, v.hash, digest, sig, pssOpts) | ||
} | ||
|
||
func (v *RSAVerifier) Public() crypto.PublicKey { | ||
return v.pub | ||
} | ||
|
||
func (v *RSAVerifier) Bytes() ([]byte, error) { | ||
return PublicPemBytes(v.pub) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this need to be deprecated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I presume you mean the switch from taking in an
io.Reader
vsContext
and[]byte
?So this hasn't been documented at all yet, and probably needs doing so, but switching to this is to align closer to the secure systems lab lib, so moving over to it is less hassle when we do.
However, passing in an
io.Reader
might be more beneficial for making all of this test driven, so it probably warrants a wider discussion. Also there might be other perfectly good reasons for this being passed as an argument.To answer the original question, I am not sure about the need for deprecation inside this
cryptoutil
library more generally. Are we intending for it to be used directly? I have also been thinking about embedding it within thesignature
package, and only exposing it in the form ofNewSignerFromReader
/NewSignerFromFile
. Not sure what your thoughts are on this?