Conversation
Codecov Report
@@ Coverage Diff @@
## main #144 +/- ##
==========================================
+ Coverage 91.05% 93.84% +2.79%
==========================================
Files 12 12
Lines 1542 1900 +358
==========================================
+ Hits 1404 1783 +379
+ Misses 101 82 -19
+ Partials 37 35 -2
... and 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| Sign(rand io.Reader, content []byte) ([]byte, error) | ||
| } | ||
|
|
||
| // DigestSigner is an interface for private keys to sign digested COSE signatures. |
There was a problem hiding this comment.
this is interface also an integration point with remote KMS?
any time you have hardware isolated keys, you need a signer interface to request signatures from them.
OR13
left a comment
There was a problem hiding this comment.
looks good to me, but better to get a review from a better go dev
|
@qmuntal as mentioned in the meeting, please add the README explaining the new API |
…nterface Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
Done. |
|
@yogeshbdeshpande, @shizhMSFT, @ivarprudnikov, @setrofim can you PTAL |
|
Unit tests would be necessary to prove it works.
Otherwise it looks OK |
|
You might want your test strings to include some nasty unicode, unless you use some standard test strings from some the RFCs |
Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
@ivarprudnikov @OR13 done. I initially didn't add any test case because the built-in signers/verifiers are internally implemented in terms of |
| } | ||
|
|
||
| // DigestSigner is an interface for private keys to sign digested COSE signatures. | ||
| type DigestSigner interface { |
There was a problem hiding this comment.
Question: Does this interface support signing digest from any Digest Algorithm ? Meaning should go-cose not restrict the digest signing to ONLY a known set of digest algorithms? This interface relies on caller to just pass digest as a substitute to content[] ?
There was a problem hiding this comment.
Question: Does this interface support signing digest from any Digest Algorithm ? Meaning should go-cose not restrict the digest signing to ONLY a known set of digest algorithms
How could we restrict that? Everyone is free to implement the interface as needed, Go doesn't allow intercepting interface implementations to do custom validations. Also, in this PR we don't use this interface anywhere, users would have to construct the cose.Sign1Message themselves, so it's an experts-only feature.
This interface relies on caller to just pass digest as a substitute to content[] ?
Yes
Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
yogeshbdeshpande
left a comment
There was a problem hiding this comment.
LGTM!
Thanks, I suggested some minor nits, but in general look ok!
Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
Fixes #120