feat: add support for signing blob#379
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #379 +/- ##
==========================================
+ Coverage 73.43% 75.33% +1.89%
==========================================
Files 24 24
Lines 2507 2019 -488
==========================================
- Hits 1841 1521 -320
+ Misses 526 353 -173
- Partials 140 145 +5 ☔ View full report in Codecov by Sentry. |
shizhMSFT
left a comment
There was a problem hiding this comment.
Blob signing introduced by notaryproject/specifications#283 is slightly different from the OCI artifact signing. In summary, blob signing has the same signing / verification core as OCI artifact signing where the digest algorithm in the notary-project specific descriptor is restricted to the signing / verification key.
It is natural to share the same Signer interface with both OCI artifact and blob signing and add a new option IsBlob in SignerSignOptions (and VerifierVerifyOptions for Verifier) since the signing core is the notary-project specific descriptor. However, this solution is vulnerable for those existing implementations (if any) don't support blob signing / verification. Precisely, the IsBlob option may be dropped by those implementations unintentionally where a blob may be verified as an artifact although IsBlob is set to true, which produces unexpected results. Therefore, it is better to split it to two different interfaces as this PR does. In notation-go/v2, maybe we can combine the signing cores into a single one.
To elaborate a bit, we need the following new stuffs for signing
type BlobSigner interface {
SignBlob(ctx context.Context, desc ocispec.Descriptor, opts SignerSignOptions) ([]byte, *signature.SignerInfo, error)
BlobDigestAlgorithm(ctx context.Context) (digest.Algorithm, error)
}
type SignBlobOptions struct {
SignerSignOptions
ContentMediaType string
UserMetadata map[string]string
}
func SignBlob(ctx context.Context, signer BlobSigner, reader io.Reader, signOpts SignBlobOptions) ([]byte, error)The BlobSigner.SignBlob does not take a reader since notation-go, as a library, it should provide flexibility where the digest of a huge blob is generated by other component efficiently. The BlobSigner.SignBlob implementation can later check if the digest algorithm matches the signing key or not and reject if not.
The SignBlob is more like a utility function of BlobSigner.SignBlob where it takes a reader to compute the digest using the algorithm specified by BlobSigner.BlobDigestAlgorithm() backed by the signing key.
/cc @Two-Hearts
|
As discussed over slack with Shiwei, I will be updating notation blob signing functions to following: Reasoning: This approach reduces the number of calls to the plugin's DescribeKey to just one, compared to the two calls required in the previous approach. Additionally, in earlier approach we did explore caching the DescribeKey response based on keyId, but it limits the seamless migration of signing keys behind the KeyId. |
f58ed61 to
0a4c92c
Compare
6532bba to
ad012dc
Compare
c701253 to
80b0268
Compare
Signed-off-by: Pritesh Bandi <[email protected]>
Signed-off-by: Pritesh Bandi <[email protected]>
Signed-off-by: Pritesh Bandi <[email protected]>
Signed-off-by: Pritesh Bandi <[email protected]>
Signed-off-by: Pritesh Bandi <[email protected]>
04f5d9b to
446557c
Compare
Signed-off-by: Pritesh Bandi <[email protected]>
04a9ae9 to
0b7be6c
Compare
Signed-off-by: Pritesh Bandi <[email protected]>
Signed-off-by: Pritesh Bandi <[email protected]>
Co-authored-by: Shiwei Zhang <[email protected]> Signed-off-by: Pritesh Bandi <[email protected]>
Signed-off-by: Pritesh Bandi <[email protected]>
This reverts commit ec42378.
This reverts commit ec42378. Signed-off-by: Junjie Gao [email protected]
This reverts commit ec42378. Signed-off-by: Junjie Gao [email protected] Signed-off-by: Junjie Gao <[email protected]>
Revert: - reverted #379 - reverted test cases: `TestSignOptsMissingSignatureMediaType` and `TestSignOptsUnknownMediaType`, introduced in #405 Test: - added test for `log` package to pass 80% coverage target This reverts commit ec42378. Resolves part of #412 Signed-off-by: Junjie Gao <[email protected]> --------- Signed-off-by: Junjie Gao [email protected] Signed-off-by: Junjie Gao <[email protected]>
This PR adds support for signing blobs.