Define Bundle Verification Result

[bdehamer]

We already have message types defined that describe the inputs to the verification process (Bundle, TrustedRoot, ArtifactVerificationOptions) so it seems reasonable to also define a standardized verification response. This will give clients a standard way to convey more specific verification errors (and make it easier to compare the verification results across the different client implementations).

I'm not married to this particular design, but offering it as a starting point for refinement:

enum VerificationStep {
        // Verify the signature w/ the leaf certificate in the chain
        SIGNATURE = 1;
        // Verify the certificate chain back to the root of the identity service
        CERTIFICATE_CHAIN = 2;
        // Verify the SET of the transparency log entry
        TRANSPARENCY_LOG = 3;
        // Verify the signed timestamp is valid and trusted
        TIMESTAMP_AUTHORITY = 4;
        // Verify the signature timestamp (either from tlog SET or timestamp authority) is within certificate's validity period
        SIGNATURE_TIMESTAMP = 5;
}

message VerificationResult {
        bool success = 1;
        optional string message = 2;
        optional VerificationStep failed_step = 3;
}

If we go with this idea of having some sort of enum to identity the specific verification step which failed we could go even more granular (e.g. CERTIFICATE_CHAIN_SCT, TRANSPARENCY_LOG_INCLUSION_PROOF, etc) . . . not clear to me what the most useful level of detail is gonna be.

Another thing which occurs to me is that it might be helpful for the VerificationResult to surface the Fulcio certificate extensions (in the case where the verification is successful). The extensions are probably the next things any client is gonna read after verifying the bundle and this could save them the trouble of fishing them out of the certificate.

CC @kommendorkapten @asraa

[asraa]

Another thing which occurs to me is that it might be helpful for the VerificationResult to surface the Fulcio certificate extensions (in the case where the verification is successful).

+1 @vaikas, any other things useful or interesting?

My own thoughts about the error struct:

• I think there should be different classes of errors. I think you've described something like "verification errors", but I think there are other "types" or classes, like "validation errors" or "parsing errors" to represent malformed content (or do we assume that these are already checked for?), "internal errors" (like if you request an online proof and it times out), and maybe others
• I think "Signature timestamp" is actually a class of "Certificate errors" or certificate validity. In the broader context of all types of PKI, it may not be necessary that a signature timestamp must be checked, as if you use a key.

But +1 on defining step related verification results - I image the message contains info like "the ith tlog entry was unable to verify"

We can always extend for the surfaced info when there is a success.

Maybe we do something like this:

message VerificationResult {
         oneof result {
           Success success = 1;
           MalformedContent malformed_content = 2;
           InternalError internal_error = 3;
           VerificationError verification_error = 4;
         } 
}

message Success {
   // leave for extending with more info
}

message VerificationError {
  optional VerificationStep failed_step = 1;
   optional string message = 2;
}

message InternalError {
   optional string message = 1;
}

message ValidationError {
   // Didn't want to use the same "ErrorWithMessage" type in case there's extensions.
   optional string message = 1;
}

[haydentherapper]

Another thing which occurs to me is that it might be helpful for the VerificationResult to surface the Fulcio certificate extensions (in the case where the verification is successful).

One concern for this is that if the verification result with parsed extensions gets persisted and later used for verification, it's susceptible to compromise/mutation. Can we be very clear that the verification result is meant to only be used at the time of verification?

+1 on having error classes.

One small question is do we want to distinguish between whether a key was used vs a certificate? Or is it sufficient to derive that from what was provided for verification?

[asraa]

One concern for this is that if the verification result with parsed extensions gets persisted and later used for verification, it's susceptible to compromise/mutation.

I would find it surprising that people would persist this and use it in an untrusted process. But yeah, I agree, it shouldn't be passed to other processes.

One small question is do we want to distinguish between whether a key was used vs a certificate? Or is it sufficient to derive that from what was provided for verification?

Another way to put it is that some VerificationSteps may only be relevant for some inputs (like a key being used will never trigger a CERTIFICATE_CHAIN error).

If we add verification output, then I image that the certificate OID extensions would be in an optional field that is only present when the input was a cert.

[bdehamer]

Big +1 on the error classes -- much more expressive than my initial proposal.

If we add verification output, then I image that the certificate OID extensions would be in an optional field that is only present when the input was a cert.

Maybe the OID extensions become an optional field in Success message (would we ever want to return these if the verification wasn't 100% successful)? Presumably, the caller will know whether a key or certificate is being used since they'll have to provide it -- so it should be obvious whether or not to look for the OID extensions in the verification output.

[asraa]

(would we ever want to return these if the verification wasn't 100% successful)?

No :D because no one should use it. But actually I'm just kidding:

I believe ArtifactVerificationProperties did have properties on identity / extension stuff? If so, maybe you fail on policy but have a cryptographically valid sig. In which case the error class that describes cert invalidity can have an error that allows describing expected/retrieved extensions. I'd say that's somewhat different than a sig verification error, and more of a policy error?

[kommendorkapten]

I believe ArtifactVerificationProperties did have properties on identity / extension stuff? If so, maybe you fail on policy but have a cryptographically valid sig

Correct, and that is an important info to be returned back to the client too.

[woodruffw]

FWIW, we currently do a similar thing in sigstore-python:

https://github.com/sigstore/sigstore-python/blob/7fcad7d55b27c492ca5d1c443b5e7b8d918229ee/sigstore/verify/models.py#L66-L90

So I'm a big fan of more formally enumerating the error states here 🙂

[kommendorkapten]

Inspiration from the sigstore-go client: https://github.com/sigstore/sigstore-go/blob/main/pkg/verify/signed_entity.go#L156