From 83e6357186be11fb8f2a6b66fab3730c44ee59dd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Jul 2022 14:42:19 -0400 Subject: [PATCH 1/2] sign/ed25519: Verify signatures are minimum length The ed25519 signature verification code does not check that the signature is a minimum/correct length. As a result, if the signature is too short, libsodium will end up reading a few bytes out of bounds. Reported-by: Demi Marie Obenour Co-authored-by: Demi Marie Obenour Closes: https://github.com/ostreedev/ostree/security/advisories/GHSA-gqf4-p3gv-g8vw --- src/libostree/ostree-sign-ed25519.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libostree/ostree-sign-ed25519.c b/src/libostree/ostree-sign-ed25519.c index 809ffe87..f271fd49 100644 --- a/src/libostree/ostree-sign-ed25519.c +++ b/src/libostree/ostree-sign-ed25519.c @@ -209,6 +209,9 @@ gboolean ostree_sign_ed25519_data_verify (OstreeSign *self, g_autoptr (GVariant) child = g_variant_get_child_value (signatures, i); g_autoptr (GBytes) signature = g_variant_get_data_as_bytes(child); + if (g_bytes_get_size (signature) != crypto_sign_BYTES) + return glnx_throw (error, "Invalid signature length of %" G_GSIZE_FORMAT " bytes, expected %" G_GSIZE_FORMAT, (gsize) g_bytes_get_size (signature), (gsize) crypto_sign_BYTES); + g_autofree char * hex = g_malloc0 (crypto_sign_PUBLICKEYBYTES*2 + 1); g_debug("Read signature %d: %s", (gint)i, g_variant_print(child, TRUE)); From e0417957ea1578032da505947ec9cac299015446 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Jul 2022 16:48:12 -0400 Subject: [PATCH 2/2] rust: Add a test case for ed25519 Specifically, I verified that *before* the previous patch to the ed25519 C code, the last bit of code would fail with a SIGSEGV when trying to read the empty signature. --- rust-bindings/tests/sign/mod.rs | 64 +++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/rust-bindings/tests/sign/mod.rs b/rust-bindings/tests/sign/mod.rs index 5df49d63..f3c817a3 100644 --- a/rust-bindings/tests/sign/mod.rs +++ b/rust-bindings/tests/sign/mod.rs @@ -1,4 +1,8 @@ +use std::process::Command; + +use ostree::prelude::SignExt; use ostree::prelude::*; +use ostree::Sign; use ostree::{gio, glib}; #[test] @@ -19,3 +23,63 @@ fn sign_api_should_work() { let result = ostree::Sign::by_name("NOPE"); assert!(result.is_err()); } + +fn inner_sign_ed25519(signer: T) { + assert_eq!(signer.name().unwrap(), "ed25519"); + + let td = tempfile::tempdir().unwrap(); + let path = td.path(); + + // Horrible bits to reuse libtest shell script code to generate keys + let pwd = std::env::current_dir().unwrap(); + let cmd = format!( + r#". {:?}/tests/libtest.sh +gen_ed25519_keys +echo $ED25519PUBLIC > ed25519.public +echo $ED25519SEED > ed25519.seed +echo $ED25519SECRET > ed25519.secret +"#, + pwd + ); + let s = Command::new("bash") + .env("G_TEST_SRCDIR", pwd) + .current_dir(path) + .args(["-euo", "pipefail"]) + .args(["-c", cmd.as_str()]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .unwrap(); + assert!(s.success()); + + let seckey = std::fs::read_to_string(path.join("ed25519.secret")).unwrap(); + let seckey = seckey.to_variant(); + signer.set_sk(&seckey).unwrap(); + let pubkey = std::fs::read_to_string(path.join("ed25519.public")).unwrap(); + let pubkey = pubkey.to_variant(); + signer.add_pk(&pubkey).unwrap(); + + let payload = &glib::Bytes::from_static(b"1234"); + + let signature = signer.data(payload, gio::NONE_CANCELLABLE).unwrap(); + let signatures = [&*signature].to_variant(); + + let msg = signer.data_verify(payload, &signatures).unwrap().unwrap(); + assert!(msg.starts_with("ed25519: Signature verified successfully")); + + assert!(signer + .data_verify(&glib::Bytes::from_static(b""), &signatures) + .is_err()); + + let badsigs = [b"".as_slice()].to_variant(); + + let e = signer.data_verify(payload, &badsigs).err().unwrap(); + assert!(e.to_string().contains("Invalid signature length of 0 bytes")) +} + +#[test] +fn sign_ed25519() { + if let Some(signer) = Sign::by_name("ed25519").ok() { + inner_sign_ed25519(signer) + } +}