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

Added signing and verification workflows #122

Merged
merged 3 commits into from
Feb 3, 2022
Merged

Added signing and verification workflows #122

merged 3 commits into from
Feb 3, 2022

Conversation

priteshbandi
Copy link
Contributor

@priteshbandi priteshbandi commented Jan 11, 2022

Summary

  • Added signing and verification workflows
  • Added detailed steps for signature verification
    • Added certificate requirements
    • Added certificate revocation

Important call-outs

  • In verification, we are evaluating timestamp(if present) irrespective of whether the signing certificate is expired or not.
    • IMO if a user has added a timestamp to the signature then they want the verification system to use that timestamp.
    • Also, always evaluating timestamp(if present) avoid modes. e.g., if the timestamp is present but it's corrupted then the verification will continue to pass until the signing certificate hasn't expired but when the signing certificate expires, verification will start failing.
  • During signature verification, we don’t support multiple certificate chains leading to multiple roots. If multiple certificate chains are leading to trusted roots, Notary v2 randomly picks one(usually, the one which is encountered first).
  • Notary v2 doesn’t support building certificate chain by retrieving certificates from locations specified in the authority information access extension (AIA).

HackmdDoc: https://hackmd.io/@pritesh/signing-verfication-workflow

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


1. **Generate signature:** Using notation CLI or any other compliant signing tool, sign the OCI artifact. The signing tool should follow the following guideline.
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).
Copy link
Contributor

Choose a reason for hiding this comment

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

The signing algorithm MUST implement the Notary v2 algorithm requirements.

1. Generate signature.
1. Generate signature using signature formats specified in [supported signature envelopes](./signature-specification.md#supported-signature-envelopes).
1. Obtain an [RFC-3161](https://datatracker.ietf.org/doc/html/rfc3161.html) compliant timestamp for the signature generated in the previous step. This step is optional but recommended.
1. Verify that the timestamp signing certificate satisfies [certificate requirements](./signature-specification.md#certificate-requirements).
Copy link
Contributor

Choose a reason for hiding this comment

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

If provided, the timestamp signing certificate MUST satisfy certificate requirements.

1. Generate signature using signature formats specified in [supported signature envelopes](./signature-specification.md#supported-signature-envelopes).
1. Obtain an [RFC-3161](https://datatracker.ietf.org/doc/html/rfc3161.html) compliant timestamp for the signature generated in the previous step. This step is optional but recommended.
1. Verify that the timestamp signing certificate satisfies [certificate requirements](./signature-specification.md#certificate-requirements).
1. Verify that the timestamp signing algorithm satisfies [algorithm requirements](./signature-specification.md#signature-algorithm-requirements).
Copy link
Contributor

Choose a reason for hiding this comment

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

The timestamp signing algorithm MUST satisfy the Notary v2 algorithm requirements.


![uml diagram showing signature evaluation](./media/trust-store-trust-policy-evaluation.svg)
Copy link
Contributor

Choose a reason for hiding this comment

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

Visuals are always helpful. Perhaps a slightly simplified version, but it would be great to bring a simplified version back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, visuals are helpful but in this case it was becoming complicated so had to remove. I can try to create simplified version of it.

Copy link
Contributor

@SteveLasker SteveLasker left a comment

Choose a reason for hiding this comment

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

Thanks @priteshbandi for all the details. Mostly nits, with a bit more details in some places.

signing-and-verification-workflow.md Outdated Show resolved Hide resolved
signing-and-verification-workflow.md Outdated Show resolved Hide resolved
signing-and-verification-workflow.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
- **Delta CRLs:** Contains only certificates that have changed status since the last base CRL was published. Delta CRLs are signed by the certificate issuing CAs.
- **Indirect CRLs:** Special Case in which BaseCRLs and Delta CRLs are not signed by the certificate issuing CAs, instead they are signed with a completely different certificate.

Notary v2 MUST support BaseCRLs and Delta CRLs. Notary v2 MAY support Indirect CRLs. Notary v2 supports only HTTP CRL URLs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a note:
There are lots of ideas around how to improve the CRL models, but I'll leave those for later and we'll lean into the existing infra and work to improve it as incremental work.

trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signing-and-verification-workflow.md Outdated Show resolved Hide resolved
signing-and-verification-workflow.md Outdated Show resolved Hide resolved
signing-and-verification-workflow.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Show resolved Hide resolved
trust-store-trust-policy-specification.md Show resolved Hide resolved
1. Validate the signature envelope using trust-store and trust-policy as mentioned in [signature evaluation](./trust-store-trust-policy-specification.md#signature-evaluation) section.
1. If signature validation fails, skip the below steps and move to the next signature artifact descriptor(step 2.1). If all signature artifact manifests have already been processed, fail the signature verification and exit.
1. If signature validation succeeds, compare the digest derived from the given OCI artifact reference with the signed digest present in the signature envelope's payload. If digests are equal, signature verification is considered successful. Otherwise, move to the next signature artifact descriptor(step 2.1). If all signature artifact descriptors have already been processed, fail the signature verification and exit.
1. **Get OCI artifact:** Using the verified digest, download the OCI artifact. This step is not in the purview of Notary v2.
Copy link
Contributor

Choose a reason for hiding this comment

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

The signed payload of a signature contains the descriptor of the target OCI artifact, which includes digest, size, and media type.

Instead of using the verified digest, we should say using the verified descriptor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have mentioned digest instead of descriptor because we haven't verified the descriptor inside the signature envelope with the actual OCI artifact's descriptor (the one stored in the registry).

signing-and-verification-workflow.md Show resolved Hide resolved
signing-and-verification-workflow.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
trust-store-trust-policy-specification.md Outdated Show resolved Hide resolved
Copy link
Contributor

@sudo-bmitch sudo-bmitch left a comment

Choose a reason for hiding this comment

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

We may want to make a clear separation between the policies that will likely be implemented in notation, from the process to verify a signature what would also be implemented in other tools that want to verify a notary signature with their own policy checks.


If the certificate revocation trust-store setting is set to `skip`, skip the below steps. Otherwise, check for revocation status for certificate and certificate chain.

1. If the revocation status of any of the certificates cannot be determined (revocation unavailable) and `signingIdentityRevocation` is set to either `enforceWithFailClose` or `warn` then log a warning and skip the below steps. Otherwise, fail the signature validation and exit.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be enforceWithFailOpen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it should be enforceWithFailClose,

  • enforceWithFailOpen: This means implementation MUST perform validation and if validation fails because the endpoint is not reachable, the implementation MUST throw an error and MUST fail the validation.
  • enforceWithFailClose: This means implementation MUST perform validation and if validation fails because the endpoint is not reachable, the implementation MUST log an error and MUST NOT fail the validation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps I'm misunderstanding, or our usage of the terminology is confusing here. The traditional usage of "fail open" means that you only throw a warning but do not stop workflows from running when the security check is unable to be performed. It sounds like we're saying "fail open" means to fail the validation which would block images from being deployed in the common use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, you are correct our usage of terminology is incorrect. Recently I've been reading a lot about circuit breakers where open state represents failure scenario and close state represents business as usual. Perhaps I got confused and interpolated that with fail open and fail close of networking.
Have fixed it in the latest commit.

@gokarnm
Copy link
Contributor

gokarnm commented Feb 2, 2022

LGTM!

Pritesh Bandi and others added 2 commits February 1, 2022 22:22
  - Added detailed steps for signature verification
  - Added certificate requirements

Co-authored-by: Milind Gokarn <[email protected]>
Signed-off-by: Pritesh Bandi <[email protected]>
Copy link
Contributor

@SteveLasker SteveLasker left a comment

Choose a reason for hiding this comment

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

Overall LGTM, with a bit of redundant text on lines 237-238 and 245-247.

signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
Copy link
Contributor

@SteveLasker SteveLasker left a comment

Choose a reason for hiding this comment

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

A few more comments on trust-store-trust-policy-specification.md

If the certificate revocation trust-store setting is set to `skip`, skip the below steps. Otherwise, check for revocation status for certificate and certificate chain.

1. If the revocation status of any of the certificates cannot be determined (revocation unavailable) and `signingIdentityRevocation` is set to either `enforceWithFailOpen` or `warn` then log a warning and skip the below steps. Otherwise, fail the signature validation and exit.
1. If any of the certificates are revoked and `signingIdentityRevocation` is set to either `enforceWithFailOpen` or `enforceWithFailClose` then fail signature validation and exit else log a warning.
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I reading this as enforceWithFailOpen or enforceWithFailClose have the same result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, when the notary can identify the revocation status of a certificate enforceWithFailOpen or enforceWithFailClose will have the same result. The behavior differs when notary is unable to determine certificate revocation status.

trust-store-trust-policy-specification.md Show resolved Hide resolved
1. Verify the CRL signature.
1. Verify that the CRL is valid (not expired). A CRL is considered expired if the current date is after the `NextUpdate` field in the CRL.
1. Look up the certificate’s serial number in the CRL.
1. If the certificate’s serial number is listed in the CRL, look for `InvalidityDate`. If the invalidity date is present and timestamp signature is also present then if the invalidity date is before the timestamping date, the certificate is considered revoked. If the invalidity date is not present in CRL, the certificate is considered revoked.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this accurate, or complete?

If the invalidity date is present and timestamp signature is also present then if the invalidity date is before the timestamping date, the certificate is considered revoked.

Should this be:

If the invalidity date is present and timestamp signature is also present then if the invalidity date is before the current date and before the timestamping date, the certificate is considered revoked.

Copy link
Contributor Author

@priteshbandi priteshbandi Feb 3, 2022

Choose a reason for hiding this comment

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

I have paraphrased the sentence for better readability.
If CRL has invalidity date and artifact signature is timestamped then compare the invalidity date with the timestamping date. If the invalidity date is before the timestamping date, the certificate is considered revoked.

Basically we are comparing the signature's timestamping date(signing date) with the certificate's effective revocation date.

trust-store-trust-policy-specification.md Show resolved Hide resolved
Signed-off-by: Pritesh Bandi <[email protected]>
Copy link
Contributor

@SteveLasker SteveLasker left a comment

Choose a reason for hiding this comment

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

LGTM

@SteveLasker SteveLasker merged commit a93eec8 into notaryproject:main Feb 3, 2022
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.

6 participants