Skip to content

Commit

Permalink
fix: support signatures produced with PKCS11 token
Browse files Browse the repository at this point in the history
The `cosign` tool can produce signatures using a PKCS11 token. These
signatures feature a `certificate` annotation inside of their OCI layer.

However, when `COSIGN_EXPERIMENTAL` is not enabled, the layer will not
feature a Rekor bundle.

Prior to this commit, the code assumed signature layers could have a
`certificate` annotation only when using the Fulcio integration. Because
of that, layers with a `certificate` but without a Rekor bundle were
discarded. That was done to ensure the robustness of keyless
verification.

This commit changes the code that creates `SignatureLayer` objects to not
raise errors when an embedded certificate cannot be verified. Be it
because it has been forged/invalid/etc or because the Rekor bundle is
not found inside of the layer.

The resulting `SignatureLayer` will not be discarded, but it will have
its `certificate_signature` attribute set to `None`.

> **Note:** `SignatureLayer::certificate_signature` was already a `Option`
> before of this commit.

The verification constraints implementing keyless verification will
discard these kind of layers because they do not have a
`certificate_signature`.

However, the public key based verifier will be able to verify the
signature stored inside of the layer.

This solves the following scenario:

> As a user,
> Given Alice signed a container image using a PKCS11 token but without having cosign's Rekor integration enabled
> When verifying the container image signature using the public key associated with the certificate stored on her PKCS11 token
> Then the container image will be reported as successfully verified

Fixes #135

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
  • Loading branch information
flavio committed Oct 12, 2022
1 parent 6b4ff41 commit 523650e
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 33 deletions.
31 changes: 24 additions & 7 deletions src/cosign/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,31 @@ pub trait CosignCapabilities {
/// Returns the list of [`SignatureLayer`](crate::cosign::signature_layers::SignatureLayer)
/// objects that are associated with the given signature object.
///
/// When Fulcio's integration has been enabled, the returned `SignatureLayer`
/// objects have been verified using the certificates bundled inside of the
/// signature image. All these certificates have been issues by Fulcio's CA.
/// Each layer is verified, to ensure it contains legitimate data.
///
/// When Rekor's integration is enabled, the [`SignatureLayer`] objects have
/// been successfully verified using the Bundle object found inside of the
/// signature image. All the Bundled objects have been verified using Rekor's
/// signature.
/// ## Layers with embedded certificate
///
/// A signature can contain a certificate, this happens when signatures
/// are produced in keyless mode or when a PKCS11 tokens are used.
///
/// The certificate is added to [`SignatureLayer::certificate_signature`]
/// only when it can be trusted.
///
/// In order to trust an embedded certificate, the following prerequisites
/// must be satisfied:
///
/// * The [`sigstore::cosign::Client`](crate::cosign::client::Client) must
/// have been created with Rekor integration enabled (see
/// [`sigstore::cosign::ClientBuilder::with_rekor_pub_key`](crate::cosign::ClientBuilder::with_rekor_pub_key))
/// * The [`sigstore::cosign::Client`](crate::cosign::client::Client) must
/// have been created with Fulcio integration enabled (see
/// [`sigstore::cosign::ClientBuilder::with_fulcio_certs`](crate::cosign::ClientBuilder::with_fulcio_certs))
/// * The layer must include a bundle produced by Rekor
///
/// When the embedded certificate cannot be verified, [`SignatureLayer::certificate_signature`]
/// is going to be `None`.
///
/// ## Usage
///
/// These returned objects can then be verified against
/// [`VerificationConstraints`](crate::cosign::verification_constraint::VerificationConstraint)
Expand Down
72 changes: 46 additions & 26 deletions src/cosign/signature_layers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,24 @@ pub struct SignatureLayer {
/// The digest of the layer
pub oci_digest: String,
/// The certificate holding the identity of the signer, plus his
/// verification key. This exists for signature done with keyless mode.
/// verification key. This exists for signature done with keyless mode or
/// when a PKCS11 token was used.
///
/// The value of `CertificateSignature` is `None`
/// when no certificate was embedded into the
/// layer, or when the embedded certificate could not be verified.
///
/// Having a `None` value will rightfully cause the
/// keyless verifiers like
/// [`CertSubjectEmailVerifier`](crate::cosign::verification_constraint::CertSubjectEmailVerifier)
/// or
/// [`CertSubjectUrlVerifier`](crate::cosign::verification_constraint::CertSubjectUrlVerifier)
/// to fail verification.
/// However, it will still be possible to use the
/// [`PublicKeyVerifier`](crate::cosign::verification_constraint::PublicKeyVerifier)
/// to verify the layer. This can be useful to verify signatures produced
/// with a PKCS11 token, but with Rekor's integration disabled at
/// signature time.
pub certificate_signature: Option<CertificateSignature>,
/// The bundle produced by Rekor.
pub bundle: Option<Bundle>,
Expand Down Expand Up @@ -218,7 +235,7 @@ impl SignatureLayer {
&annotations,
fulcio_cert_pool,
bundle.as_ref(),
)?;
);

Ok(SignatureLayer {
oci_digest: descriptor.digest.clone(),
Expand Down Expand Up @@ -259,29 +276,42 @@ impl SignatureLayer {
annotations: &HashMap<String, String>,
fulcio_cert_pool: Option<&CertificatePool>,
bundle: Option<&Bundle>,
) -> Result<Option<CertificateSignature>> {
) -> Option<CertificateSignature> {
let cert_raw = match annotations.get(SIGSTORE_CERT_ANNOTATION) {
Some(value) => value,
None => return Ok(None),
None => return None,
};

let fulcio_cert_pool = match fulcio_cert_pool {
Some(cp) => cp,
None => {
return Err(SigstoreError::SigstoreFulcioCertificatesNotProvidedError);
info!(
reason = "fulcio certificates not provided",
"Ignoring certificate annotation"
);
return None;
}
};

let bundle = match bundle {
Some(b) => b,
None => {
return Err(SigstoreError::SigstoreRekorBundleNotFoundError);
info!(
reason = "rekor bundle not found",
"Ignoring certificate annotation"
);
return None;
}
};

let certificate_signature =
CertificateSignature::from_certificate(cert_raw.as_bytes(), fulcio_cert_pool, bundle)?;
Ok(Some(certificate_signature))
match CertificateSignature::from_certificate(cert_raw.as_bytes(), fulcio_cert_pool, bundle)
{
Ok(certificate_signature) => Some(certificate_signature),
Err(e) => {
info!(reason=?e, "Ignoring certificate annotation");
None
}
}
}

/// Given a Cosign public key, check whether this Signature Layer has been
Expand Down Expand Up @@ -717,7 +747,7 @@ JsB89BPhZYch0U0hKANx5TY+ncrm0s8bfJxxHoenAEFhwhuXeb4PqIrtoQ==
None,
);

assert!(actual.unwrap().is_none());
assert!(actual.is_none());
}

#[test]
Expand All @@ -729,17 +759,12 @@ JsB89BPhZYch0U0hKANx5TY+ncrm0s8bfJxxHoenAEFhwhuXeb4PqIrtoQ==

let fulcio_cert_pool = get_fulcio_cert_pool();

let error = SignatureLayer::get_certificate_signature_from_annotations(
let cert = SignatureLayer::get_certificate_signature_from_annotations(
&annotations,
Some(&fulcio_cert_pool),
None,
)
.expect_err("Didn't get an error");

assert!(matches!(
error,
SigstoreError::SigstoreRekorBundleNotFoundError
));
);
assert!(cert.is_none());
}

#[test]
Expand All @@ -751,17 +776,12 @@ JsB89BPhZYch0U0hKANx5TY+ncrm0s8bfJxxHoenAEFhwhuXeb4PqIrtoQ==

let bundle = build_bundle();

let error = SignatureLayer::get_certificate_signature_from_annotations(
let cert = SignatureLayer::get_certificate_signature_from_annotations(
&annotations,
None,
Some(&bundle),
)
.expect_err("Didn't get an error");

assert!(matches!(
error,
SigstoreError::SigstoreFulcioCertificatesNotProvidedError
));
);
assert!(cert.is_none());
}

#[test]
Expand Down

0 comments on commit 523650e

Please sign in to comment.