WIP: Sequoia signature experiments #2876
Conversation
This adds a new OpenPGP signing mechanism based Sequoia-PGP[1]. As Sequoia-PGP is written in Rust and doesn't provide a stable C FFI, this integration includes a minimal shared library as a "point solution". To build, first follow the instruction in signature/sequoia/README.md and install `libimage_sequoia.so*` into the library path, and then build with the following command from the top-level directory: $ make BUILDTAGS="btrfs_noversion containers_image_sequoia" Note also that, for testing on Fedora, crypto-policies settings might need to be modified to explictly allow SHA-1 and 1024 bit RSA, as the testing keys in signature/fixtures are using those legacy algorithms. 1. https://sequoia-pgp.org/ Signed-off-by: Daiki Ueno <dueno@redhat.com>
8c6899b to
97955fb
Compare
| return Ok(()); | ||
| } | ||
| Err(verification_error) => { | ||
| signature_errors.push(verification_error.to_string()); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
23e7645 to
7e16175
Compare
…iners-image into signature-sequoia
- Use the correct library name - Allow specifying a library path at compile time Example usage: > make BUILDTAGS="btrfs_noversion containers_image_sequoia" SEQUOIA_SONAME_DIR=$(pwd)/signature/sequoia/rust/target/release Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... to ensure it is consistent, and to allow updating it in one place. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
It is a generated file. This should be removed if the Rust code is moved to another repo. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Can we, instead, add a callback to go back to Go? Now? Later? Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Don't entirely discard the underlying error message. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…Mechanism Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…bpackage We will reuse them. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…w Signer API WIP: - Most importantly, does not allow using the default sq directory. - FIXMEs all over the place - Missing test coverage Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change test behavior, merely to be more similar to the rest of the codebase. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
efcf53d to
461beea
Compare
| func WithPassphrase(passphrase string) Option { | ||
| return func(s *simpleSequoiaSigner) error { | ||
| // The gpgme implementation can’t use passphrase with \n; reject it here for consistent behavior. | ||
| // FIXME: We don’t need it in this API at all, but the "\n" check exists in the current call stack. That should go away. |
There was a problem hiding this comment.
Branch sequoia-refactor-SignDockerManifest, but that can wait.
| // WithSequoiaHome returns an Option for NewSigner, specifying a Sequoia home directory to use. | ||
| func WithSequoiaHome(sequoiaHome string) Option { | ||
| return func(s *simpleSequoiaSigner) error { | ||
| if sequoiaHome == "none" || sequoiaHome == "default" { // FIXME: Do we need to special-case this? |
There was a problem hiding this comment.
I guess the answer is probably not, as these keywords are specific to the sq command, not the sequoia library.
| return nil, fmt.Errorf("Signing not supported: %w", err) | ||
| } | ||
|
|
||
| // Ideally, we should look up (and unlock?) the key at this point already. FIXME: is that possible? Anyway, low-priority. |
There was a problem hiding this comment.
Yes, if needed we could reorganize the Rust API to allow that.
- Allow using c/image without loading sequoia if GO_SEQUOIA_ENABLE_DLOPEN is used, and nothing uses the Sequoia code at runtime. - Make the two callers ~independent users of signature/internal/sequoia, without having to coordinate to call at least once. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Should not change behavior, but we will add support for the default home-directory paths. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Sharing this to possibly help collaboration; unlikely to be useful for any real-world use, anything finished will probably be split into separate PRs.
Based on #2569 , many thanks to @ueno .