-
Notifications
You must be signed in to change notification settings - Fork 6
Expose Hash, Sign, VerifySignature and VerifyHashSignature #4
Conversation
This patch refactors some of the internal logic to expose public functions that can be used outside of a MAR file.
Pull Request Test Coverage Report for Build 61
💛 - Coveralls |
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.
r+ w/ nits
sign.go
Outdated
@@ -64,44 +66,16 @@ func (file *File) FinalizeSignatures() error { | |||
if err != nil { | |||
return err | |||
} | |||
if len(file.Signatures) == 0 { | |||
return fmt.Errorf("there are no signature to finalize") |
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.
nit: s/signature/signatures/
sign.go
Outdated
sigData, err := file.Signatures[i].privateKey.(crypto.Signer).Sign( | ||
rand.Reader, md.Sum(nil), h) | ||
hashed, hashAlg, err := Hash(signableBlock, file.Signatures[i].AlgorithmID) | ||
sigData, err := Sign(file.Signatures[i].privateKey, rand.Reader, hashed, hashAlg) |
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.
👍
@@ -112,6 +86,46 @@ func (file *File) MarshalForSignature() ([]byte, error) { | |||
return file.Marshal() | |||
} | |||
|
|||
// Hash takes an input and a signature algorithm and returns its hashed value | |||
func Hash(input []byte, sigalg uint32) (output []byte, h crypto.Hash, err error) { |
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.
might want a type alias for the sig algs
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 that buy us anything? we'll need to type assert to uint32 for marshalling
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.
it's more explicit
sign.go
Outdated
// Sign signs digest with the private key, possibly using entropy from rand | ||
func Sign(key crypto.PrivateKey, rand io.Reader, digest []byte, h crypto.Hash) (sigData []byte, err error) { | ||
// call the signer interface of the private key to sign the hash | ||
sigData, err = key.(crypto.Signer).Sign(rand, digest, h) |
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.
can the cast to crypto.Signer fail and if so do we need to handle that?
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 added an assertion that will return an error if Signer isn't implemented
verify.go
Outdated
} | ||
|
||
// VerifyHashSignature takes a signature, the digest of a signed MAR block, a hash algorithm and a public | ||
// key and return nil if a valid signature is found, or an error if it isn't |
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.
nit: s/return/returns/
verify.go
Outdated
|
||
// VerifyHashSignature takes a signature, the digest of a signed MAR block, a hash algorithm and a public | ||
// key and return nil if a valid signature is found, or an error if it isn't | ||
func VerifyHashSignature(signature []byte, digest []byte, hashAlg crypto.Hash, key crypto.PublicKey) error { |
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.
👍 for splitting this out of VerifySignature
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.
r+
This patch refactors some of the internal logic to expose public functions that can be used outside of a MAR file.