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

Add support for user-defined signed attributes #133

Closed
wants to merge 1 commit into from
Closed

Add support for user-defined signed attributes #133

wants to merge 1 commit into from

Conversation

priteshbandi
Copy link
Contributor

This change allows user to add custom signed metadata to signature.

Signed-off-by: Pritesh Bandi [email protected]

"size": 16724
"size": 16724,
"annotations": {
"wabbit-networks.io.buildId": "123", // user defined signed attribute.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's mark this code block using jsonc instead of json for better syntax highlighting for comments.

@@ -16,7 +16,7 @@ The user wants to sign an OCI artifact and push the signature to a repository.
1. Verify that the signing certificate is valid and satisfies [certificate requirements](./signature-specification.md#certificate-requirements).
1. Verify that the signing algorithm satisfies [algorithm requirements](./signature-specification.md#signature-algorithm-requirements).
1. Generate signature.
1. Generate signature using signature formats specified in [supported signature envelopes](./signature-specification.md#supported-signature-envelopes).
1. Generate signature using signature formats specified in [supported signature envelopes](./signature-specification.md#supported-signature-envelopes). Also, as part of this step, the user-defined/supplied custom attributes should be added to the signature's signed attributes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term signature's signed attributes is ambiguous. Current solution for user-defined attributes is to store them in the annotations field of the content descriptor where the descriptor is the signing payload. Meanwhile, modern signature synax / envelopes like JWS, PKCS#7, COSE support protected headers besides the payload. The protected headers are the signature's signed attributes. They are signed with the signing payload but not the part of the signing payload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have defined signed attributes in signature-specification.md#signature-envelope. The signed attribute matters in systems that support multiple signatures in a single signature envelope, in such cases every signature shares a common payload but have different signed attributes. As in the case of notary we support 1 signature per envelope it doesn't matters if it goes into payload or in dedicated signed attributes filed.

@@ -72,16 +72,19 @@ Notary v2 supports [JWS JSON Serialization](https://datatracker.ietf.org/doc/htm
Notary v2 requires Payload to be the content **descriptor** of the subject manifest that is being signed.

1. Descriptor MUST contain `mediaType`, `digest`, `size` fields.
2. Descriptor MAY contain `annotations` and if present it MUST follow the [annotation rules](https://github.com/opencontainers/image-spec/blob/main/annotations.md#rules). In Notary v2 annotations are being used to store signed attributes. The annotations key prefix for Notary v2 use is not yet finalized. See [issues-106](https://github.com/notaryproject/notaryproject/issues/106).
2. Descriptor MAY contain `annotations` and if present it MUST follow the [annotation rules](https://github.com/opencontainers/image-spec/blob/main/annotations.md#rules). Notary v2 uses annotations for storing both Notary specific and user defined signed attributes. The prefix `org.cncf.notary` in annotation keys is reserved for use in Notary v2 and MUST NOT be used outside this specification.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you want cncf.io, not cncf.org.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

annotation namespaces is pending, per #106
Suggest we leave this for consistency in this PR and resolve as we finalize 106.

@SteveLasker
Copy link
Contributor

Back to @priteshbandi for merge conflicts

@priteshbandi
Copy link
Contributor Author

Tracking this in #136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants