fix verification of signatures produced with pki11

examples/cosign/verify/main.rs

@@ -111,7 +111,7 @@ async fn run_app(
     let mut client_builder = sigstore::cosign::ClientBuilder::default();
 
     if let Some(key) = frd.rekor_pub_key.as_ref() {
-        client_builder = client_builder.with_rekor_pub_key(&key);
+        client_builder = client_builder.with_rekor_pub_key(key);
     }
 
     if !frd.fulcio_certs.is_empty() {

examples/openidflow/openidconnect/main.rs

@@ -30,7 +30,7 @@ fn main() -> Result<(), anyhow::Error> {
             open::that(url.0.to_string())?;
             println!(
                 "Open this URL in a browser if it does not automatically open for you:\n{}\n",
-                url.0.to_string()
+                url.0
             );
         }
         Err(e) => println!("{}", e),

src/cosign/mod.rs

@@ -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)

src/cosign/signature_layers.rs

@@ -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>,
@@ -173,8 +190,6 @@ impl SignatureLayer {
     ///     entries
     ///   * `fulcio_pub_key`: the public key provided by Fulcio's certificate.
     ///     Used to verify the `certificate` entries
-    ///   * `cert_email`: optional, the SAN to look for inside of trusted
-    ///     certificates issued by Fulcio
     ///
     /// **Note well:** the certificate and bundle added to the final SignatureLayer
     /// object are to be considered **trusted** and **verified**, according to
@@ -220,7 +235,7 @@ impl SignatureLayer {
             &annotations,
             fulcio_cert_pool,
             bundle.as_ref(),
-        )?;
+        );
 
         Ok(SignatureLayer {
             oci_digest: descriptor.digest.clone(),
@@ -261,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
@@ -719,7 +747,7 @@ JsB89BPhZYch0U0hKANx5TY+ncrm0s8bfJxxHoenAEFhwhuXeb4PqIrtoQ==
             None,
         );
 
-        assert!(actual.unwrap().is_none());
+        assert!(actual.is_none());
     }
 
     #[test]
@@ -731,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]
@@ -753,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]

src/crypto/mod.rs

@@ -238,7 +238,7 @@ OSWS1X9vPavpiQOoTTGC0xX57OojUadxF1cdQmrsiReWg2Wn4FneJfa8xw==
         let public_key = private_key.public_key();
 
         let ec_pub_key =
-            EcKey::from_public_key(&group, &public_key).expect("Cannot create ec pub key");
+            EcKey::from_public_key(&group, public_key).expect("Cannot create ec pub key");
         let pkey = pkey::PKey::from_ec_key(ec_pub_key).expect("Cannot create pkey");
 
         let mut x509_name_builder = X509NameBuilder::new()?;
@@ -369,7 +369,7 @@ OSWS1X9vPavpiQOoTTGC0xX57OojUadxF1cdQmrsiReWg2Wn4FneJfa8xw==
         // set issuer
         if let Some(issuer_data) = issuer {
             let issuer_name = issuer_data.cert.subject_name();
-            x509_builder.set_issuer_name(&issuer_name)?;
+            x509_builder.set_issuer_name(issuer_name)?;
         } else {
             // self signed cert
             x509_builder.set_issuer_name(&x509_name)?;

src/crypto/signing_key/mod.rs

@@ -340,10 +340,9 @@ mod tests {
     #[case(SigningScheme::ECDSA_P384_SHA384_ASN1)]
     #[case(SigningScheme::ED25519)]
     fn sigstore_signing(#[case] signing_scheme: SigningScheme) {
-        let signer = signing_scheme.create_signer().expect(&format!(
-            "create SigStoreSigner with {:?} failed",
-            signing_scheme
-        ));
+        let signer = signing_scheme
+            .create_signer()
+            .unwrap_or_else(|_| panic!("create SigStoreSigner with {:?} failed", signing_scheme));
         let key_pair = signer
             .to_sigstore_keypair()
             .expect("convert SigStoreSigner to SigStoreKeypair failed.");

src/crypto/verification_key.rs

@@ -288,7 +288,7 @@ mod tests {
                 .expect("Cannot create CosignVerificationKey");
         let msg = r#"{"critical":{"identity":{"docker-reference":"registry-testing.svc.lan/busybox"},"image":{"docker-manifest-digest":"sha256:f3cfc9d0dbf931d3db4685ec659b7ac68e2a578219da4aae65427886e649b06b"},"type":"cosign container image signature"},"optional":null}"#;
 
-        let outcome = verification_key.verify_signature(signature, &msg.as_bytes());
+        let outcome = verification_key.verify_signature(signature, msg.as_bytes());
         assert!(outcome.is_ok());
     }
 
@@ -301,7 +301,7 @@ mod tests {
         let msg = "hello world";
 
         let err = verification_key
-            .verify_signature(signature, &msg.as_bytes())
+            .verify_signature(signature, msg.as_bytes())
             .expect_err("Was expecting an error");
         let found = match err {
             SigstoreError::PublicKeyVerificationError => true,
@@ -319,7 +319,7 @@ mod tests {
         let msg = r#"{"critical":{"identity":{"docker-reference":"registry-testing.svc.lan/busybox"},"image":{"docker-manifest-digest":"sha256:f3cfc9d0dbf931d3db4685ec659b7ac68e2a578219da4aae65427886e649b06b"},"type":"cosign container image signature"},"optional":null}"#;
 
         let err = verification_key
-            .verify_signature(signature, &msg.as_bytes())
+            .verify_signature(signature, msg.as_bytes())
             .expect_err("Was expecting an error");
         let found = match err {
             SigstoreError::Base64DecodeError(_) => true,
@@ -344,7 +344,7 @@ JsB89BPhZYch0U0hKANx5TY+ncrm0s8bfJxxHoenAEFhwhuXeb4PqIrtoQ==
         let msg = r#"{"critical":{"identity":{"docker-reference":"registry-testing.svc.lan/busybox"},"image":{"docker-manifest-digest":"sha256:f3cfc9d0dbf931d3db4685ec659b7ac68e2a578219da4aae65427886e649b06b"},"type":"cosign container image signature"},"optional":null}"#;
 
         let err = verification_key
-            .verify_signature(signature, &msg.as_bytes())
+            .verify_signature(signature, msg.as_bytes())
             .expect_err("Was expecting an error");
         let found = match err {
             SigstoreError::PublicKeyVerificationError => true,
@@ -374,7 +374,7 @@ DwIDAQAB
         let msg = r#"{"critical":{"identity":{"docker-reference":"registry.suse.com/suse/sle-micro/5.0/toolbox"},"image":{"docker-manifest-digest":"sha256:356631f7603526a0af827741f5fe005acf19b7ef7705a34241a91c2d47a6db5e"},"type":"cosign container image signature"},"optional":{"creator":"OBS"}}"#;
 
         assert!(verification_key
-            .verify_signature(signature, &msg.as_bytes())
+            .verify_signature(signature, msg.as_bytes())
             .is_ok());
     }
 }

src/rekor/apis/entries_api.rs

@@ -52,7 +52,7 @@ pub enum SearchLogQueryError {
     UnknownValue(serde_json::Value),
 }
 
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct LogEntries {
     entries: Vec<LogEntry>,

src/rekor/models/alpine.rs

@@ -11,7 +11,7 @@
 use serde::{Deserialize, Serialize};
 
 /// Alpine : Alpine package
-#[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize)]
+#[derive(Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize)]
 pub struct Alpine {
     #[serde(rename = "kind")]
     pub kind: String,

src/rekor/models/alpine_all_of.rs

@@ -10,7 +10,7 @@
 
 use serde::{Deserialize, Serialize};
 
-#[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize)]
+#[derive(Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize)]
 pub struct AlpineAllOf {
     #[serde(rename = "apiVersion")]
     pub api_version: String,

src/rekor/models/consistency_proof.rs

@@ -10,7 +10,7 @@
 
 use serde::{Deserialize, Serialize};
 
-#[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize)]
+#[derive(Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize)]
 pub struct ConsistencyProof {
     /// The hash value stored at the root of the merkle tree at the time the proof was generated
     #[serde(rename = "rootHash")]

src/rekor/models/error.rs

@@ -10,7 +10,7 @@
 
 use serde::{Deserialize, Serialize};
 
-#[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize)]
+#[derive(Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize)]
 pub struct Error {
     #[serde(rename = "code", skip_serializing_if = "Option::is_none")]
     pub code: Option<i32>,

src/rekor/models/hashedrekord.rs

@@ -13,7 +13,7 @@ use url::Url;
 
 /// Hashedrekord : Hashed Rekord object
 
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
 pub struct Hashedrekord {
     #[serde(rename = "kind")]
     pub kind: String,
@@ -35,7 +35,7 @@ impl Hashedrekord {
 }
 
 /// Stores the Signature and Data struct
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct Spec {
     signature: Signature,
@@ -50,7 +50,7 @@ impl Spec {
 }
 
 /// Stores the signature format, signature of the artifact and the PublicKey struct
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct Signature {
     format: String,
@@ -69,7 +69,7 @@ impl Signature {
 }
 
 /// Stores the public key used to sign the artifact
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct PublicKey {
     content: String,
@@ -81,7 +81,7 @@ impl PublicKey {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct Data {
     hash: Hash,
@@ -94,15 +94,15 @@ impl Data {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[allow(non_camel_case_types)]
 pub enum AlgorithmKind {
     sha256,
     sha1,
 }
 
 /// Stores the algorithm used to hash the artifact and the value of the hash
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct Hash {
     algorithm: AlgorithmKind,

src/rekor/models/hashedrekord_all_of.rs

@@ -10,7 +10,7 @@
 
 use serde::{Deserialize, Serialize};
 
-#[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize)]
+#[derive(Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize)]
 pub struct HashedrekordAllOf {
     #[serde(rename = "apiVersion")]
     pub api_version: String,