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

cosigned should support verifying Fulcio signatures #866

Closed
mattmoor opened this issue Oct 10, 2021 · 3 comments
Closed

cosigned should support verifying Fulcio signatures #866

mattmoor opened this issue Oct 10, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@mattmoor
Copy link
Member

Description

Today the cosigned webhook requires folks to setup k8s://cosign-system/verification-key with the public key that should be used for verification. The webhook passes the following CheckOpts:

RootCerts: fulcioroots.Get(),
SigVerifier: ecdsaVerifier,

... which includes a SigVerifier based on the verification key, which sends us through this path in Verify:

cosign/pkg/cosign/verify.go

Lines 143 to 150 in 278ad7d

case co.SigVerifier != nil:
signature, err := base64.StdEncoding.DecodeString(b64sig)
if err != nil {
return err
}
if err := co.SigVerifier.VerifySignature(bytes.NewReader(signature), bytes.NewReader(payload), options.WithContext(ctx)); err != nil {
return err
}

... but for Fulcio we want things to follow this path:

cosign/pkg/cosign/verify.go

Lines 151 to 184 in 278ad7d

// If we don't have a public key to check against, we can try a root cert.
case co.RootCerts != nil:
// There might be signatures with a public key instead of a cert, though
if cert == nil {
return errors.New("no certificate found on signature")
}
pub, err := signature.LoadECDSAVerifier(cert.PublicKey.(*ecdsa.PublicKey), crypto.SHA256)
if err != nil {
return errors.Wrap(err, "invalid certificate found on signature")
}
// Now verify the cert, then the signature.
if err := TrustedCert(cert, co.RootCerts); err != nil {
return err
}
signature, err := base64.StdEncoding.DecodeString(b64sig)
if err != nil {
return err
}
if err := pub.VerifySignature(bytes.NewReader(signature), bytes.NewReader(payload), options.WithContext(ctx)); err != nil {
return err
}
if co.CertEmail != "" {
emailVerified := false
for _, em := range cert.EmailAddresses {
if co.CertEmail == em {
emailVerified = true
break
}
}
if !emailVerified {
return errors.New("expected email not found in certificate")
}
}

@mattmoor mattmoor added the enhancement New feature or request label Oct 10, 2021
@mattmoor
Copy link
Member Author

Here's sort of what I'm thinking:

  1. Switch from using a secret for verification-key to using a ConfigMap (since it's a public key, when specified).
  2. When the key isn't specified, verify against the Fulcio roots.

Over time, we can use this configmap to pass additional configuration, including:

  • Alternate Fulcio root certs (e.g. if a custom Fulcio root is in use),
  • A Rekor URL to verify transparency logs.

... and likely a bunch more (e.g. stuff like CertEmail in CheckOpts).

cc @dlorenc @dekkagaijin @n3wscott for thoughts

@mattmoor
Copy link
Member Author

I guess we could start verifying against Fulcio when no key is specified without the ConfigMap switch too, but I do think a ConfigMap switch is in our not-to-distant future 🤔

mattmoor added a commit to mattmoor/cosign that referenced this issue Oct 10, 2021
This change makes it so that when `cosigned` is enabled on a namespace, and no public key is provided the webhook will verify things against the Fulcio root.

The basic idea here is that this roughly matches the CLI where omitting the key will verify things against the Fulcio root instead.

Related: sigstore#866

Signed-off-by: Matt Moore <mattomata@gmail.com>
mattmoor added a commit to mattmoor/cosign that referenced this issue Oct 10, 2021
This change makes it so that when `cosigned` is enabled on a namespace, and no public key is provided the webhook will verify things against the Fulcio root.

The basic idea here is that this roughly matches the CLI where omitting the key will verify things against the Fulcio root instead.

Related: sigstore#866

Signed-off-by: Matt Moore <mattomata@gmail.com>
mattmoor added a commit to mattmoor/cosign that referenced this issue Oct 10, 2021
This change makes it so that when `cosigned` is enabled on a namespace, and no public key is provided the webhook will verify things against the Fulcio root.

The basic idea here is that this roughly matches the CLI where omitting the key will verify things against the Fulcio root instead.

Related: sigstore#866

Signed-off-by: Matt Moore <mattomata@gmail.com>
mattmoor added a commit to mattmoor/cosign that referenced this issue Oct 11, 2021
This change makes it so that when `cosigned` is enabled on a namespace, and no public key is provided the webhook will verify things against the Fulcio root.

The basic idea here is that this roughly matches the CLI where omitting the key will verify things against the Fulcio root instead.

Related: sigstore#866

Signed-off-by: Matt Moore <mattomata@gmail.com>
dlorenc pushed a commit that referenced this issue Oct 11, 2021
This change makes it so that when `cosigned` is enabled on a namespace, and no public key is provided the webhook will verify things against the Fulcio root.

The basic idea here is that this roughly matches the CLI where omitting the key will verify things against the Fulcio root instead.

Related: #866

Signed-off-by: Matt Moore <mattomata@gmail.com>
@mattmoor
Copy link
Member Author

I'm going to close this, and we can open a new issue for greater configurability.

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

No branches or pull requests

1 participant