Add containers_image_sequoia build tag to do simple signing verification using Sequoia-PGP, and add a signature/simplesequoia implementation

[mtrmac]

• In signature/internal/sequoia, add Go bindings for https://github.com/ueno/podman-sequoia . (Parts of the middle C code are auto-generated by that project.)
• With the containers_image_sequoia build tag:
  • Implement the previously-included but stub-only signature/simplesequoia API for creating simple-signing signatures using Sequoia-PGP
  • Do all simple signing verification using the Sequoia-PGP backend, instead of GnuPG via GPGME. Hopefully that will be transparent to users as of now.
  • Keep the traditional simple-signing API as is, backed by GnuPG via GPGME. That way, existing workflows that generate/import keys to GnuPG, or rely on GNUPGHOME, are unaffected.
• Add a new set of CI tests, which run with the containers_image_sequoia build tag; currently that requires Rawhide.

Based on original contribution in #2569, with many thanks to @ueno .

DO NOT MERGE: Includes UNMERGED #2905 .

[ueno]

Just a matter of taste, but instead of storing errors as a string, I would keep the error types and let the caller format them:

diff --git a/signature/internal/sequoia/rust/src/signature.rs b/signature/internal/sequoia/rust/src/signature.rs
index 014d4f5f..d0ba6cd3 100644
--- a/signature/internal/sequoia/rust/src/signature.rs
+++ b/signature/internal/sequoia/rust/src/signature.rs
@@ -169,6 +169,7 @@ impl<'a> SequoiaMechanism<'a> {
         let h = Helper {
             certstore: self.certstore.clone(),
             signer: Default::default(),
+            errors: Default::default(),
         };
 
         let mut v = VerifierBuilder::from_bytes(signature)?.with_policy(&self.policy, None, h)?;
@@ -190,6 +191,7 @@ impl<'a> SequoiaMechanism<'a> {
 struct Helper<'a> {
     certstore: Arc<sequoia_cert_store::CertStore<'a>>,
     signer: Option<openpgp::Cert>,
+    errors: Vec<openpgp::Error>,
 }
 
 impl<'a> VerificationHelper for Helper<'a> {
@@ -205,7 +207,6 @@ impl<'a> VerificationHelper for Helper<'a> {
     }
 
     fn check(&mut self, structure: MessageStructure) -> openpgp::Result<()> {
-        let mut signature_errors: Vec<String> = Vec::new();
         for layer in structure {
             match layer {
                 MessageLayer::Compression { algo } => log::info!("Compressed using {}", algo),
@@ -219,7 +220,7 @@ impl<'a> VerificationHelper for Helper<'a> {
                         log::info!("Encrypted using {}", sym_algo);
                     }
                 }
-                MessageLayer::SignatureGroup { ref results } => {
+                MessageLayer::SignatureGroup { results } => {
                     for result in results {
                         match result {
                             Ok(good_checksum) => {
@@ -227,19 +228,14 @@ impl<'a> VerificationHelper for Helper<'a> {
                                 return Ok(());
                             }
                             Err(verification_error) => {
-                                signature_errors.push(verification_error.to_string());
+                                self.errors.push(openpgp::Error::from(verification_error));
                             }
                         }
                     }
                 }
             }
         }
-        let err = match signature_errors.len() {
-        0 => anyhow::anyhow!("No valid signature"),
-        1 => anyhow::anyhow!("{}", &signature_errors[0]),
-        _ => anyhow::anyhow!("Multiple signature errors: [{}]", signature_errors.join(", ")),
-        };
-        Err(err)
+        Err(anyhow::anyhow!("No valid signature"))
     }
 }

I also wonder if we might want to record all signers if there are multiple signatures.

[mtrmac]

I would keep the error types and let the caller format them:

Thanks, that might very well be cleaner. It would require a somewhat tight coupling with the caller — the caller would specifically need to know that it should ignore the returned “no valid signature” error.

---

I also wonder if we might want to record all signers if there are multiple signatures.

For myself, I think I need to study / understand what is the precise semantics of MessageStructure, whether it is possible to have multiple signatures, and what does that mean.

… and whether it is relevant to designing a stable C API, which is the ~most urgent work right now.

[ueno]

I would keep the error types and let the caller format them:

Thanks, that might very well be cleaner. It would require a somewhat tight coupling with the caller — the caller would specifically need to know that it should ignore the returned “no valid signature” error.

Actually that's the intention behind this suggestion: if the errors are converted into a string at the callee, useful information that could be retrieved through VerificationError may be lost, or it would require parsing of the error strings later.

I also wonder if we might want to record all signers if there are multiple signatures.

For myself, I think I need to study / understand what is the precise semantics of MessageStructure, whether it is possible to have multiple signatures, and what does that mean.

… and whether it is relevant to designing a stable C API, which is the ~most urgent work right now.

MessageStructure can certainly have multiple signatures, though the current c/image API seems to only expect a single signature to be valid. For future extension with PQ/T multiple signatures, it might make sense to provide a control on which signature has to be valid and which can be ignored.

As for the C API, GPGME provides a similar abstraction with gpgme_verify_result_t, which also supports multiple signatures.

[mtrmac]

Branch sequoia-refactor-SignDockerManifest, but that can wait.

[ueno]

Yes, if needed we could reorganize the Rust API to allow that.

[Luap99]

I mention it because we have been burned by this many times before and I haven't looked into the sequoia code to know for sure but is the sequoia library fully thread safe. And by that I mean is it using any thread local memory or other things that depend on the current thread? Because the go runtime can schedule us around in any thread at any time between these C calls which means go_sequoia_sign() and go_sequoia_signature_get_data() may get called from different threads so the underlying library must be able to handle this correctly

i.e. a simple example on how things can go wrong easily:
coreos/go-systemd@d1df97f

[mtrmac]

On the Go side, this PR now uses a sync.Once to load the library. And afterwards, everything is supposed to go through a Go *SigningMechanism / Rust SequoiaMechanism, where serialization of operations on a single mechanism is up to the caller.

I have no idea whether Sequoia is internally consistently thread-safe; I do see some uses of OnceLock on static variables, so it is at least not naively unaware.

[Luap99]

serialization of operations on a single mechanism is up to the caller.

My concern was not so much about serialization but more about the use of stuff like thread local storage or other per thread things like a namespace for example. Between each C.somefunc call the go runtime can schedule us the code onto another thread. But because that is super unlikely normal testing will not uncover this most likely. It could end up being a very hard to find race condition later on. See the linked commit where the use of dlerror() causes some bugs because it stores the error in thread local memory.

I am not saying there is a problem here but just that we should check. I am sure @ueno or the other sequoia folks would know this

[mtrmac]

signature/internal/sequoia/*.[ch] are auto-generated in https://github.com/ueno/podman-sequoia/tree/main/go/sequoia , only copied into this repo.

[mtrmac]

Ready for review, PTAL. I have preserved the original work from #2569 and this is structured as a series of commits on top, but in this case, it’s probably easier to just review the final diff as a single unit.

Cc: @giuseppe @Luap99 @lsm5

[lsm5]

Ready for review, PTAL. I have preserved the original work from #2569 and this is structured as a series of commits on top, but in this case, it’s probably easier to just review the final diff as a single unit.

Cc: @giuseppe @Luap99 @lsm5

Should this PR be moved to the monorepo?

[mtrmac]

Should this PR be moved to the monorepo?

Sure, thanks for the reminder. I’ll move things after the dust settles.

[Luap99]

LGTM, I rather get this merged by EOD before the monorepo move to avoid having to redo all commits against the new repo structure.

Automation wise it means we must port the rawhide tests to the new repo but we can do this after the initial repo creation.
cc @jankaluza

[lsm5]

LGTM, I rather get this merged by EOD before the monorepo move to avoid having to redo all commits against the new repo structure.

LGTM in that case