acl-agent: pass Nebraska-reported hash to Trident as sha384

Cargo.toml

@@ -18,6 +18,7 @@ members = [
 [workspace.dependencies]
 anyhow = { version = "1.0.94", features = ["backtrace"] }
 backoff = "0.4.0"
+base64 = "0.22.1"
 bitflags = { version = "2.6.0", features = ["serde"] }
 chrono = { version = "0.4.38", features = ["serde"] }
 clap = { version = "4.4.18", features = ["derive", "wrap_help"] }

crates/trident-acl-agent/Cargo.toml

@@ -7,6 +7,7 @@ publish = false
 [dependencies]
 anyhow = { workspace = true, features = ["backtrace"] }
 backoff = { workspace = true }
+base64 = { workspace = true }
 clap = { workspace = true, features = ["derive"] }
 chrono = { workspace = true }
 const_format = { workspace = true }
@@ -15,6 +16,7 @@ envy = { workspace = true }
 futures = { workspace = true }
 hostname = { workspace = true }
 humantime = { workspace = true }
+hex = { workspace = true }
 k8s-openapi = { workspace = true }
 kube = { workspace = true }
 log = { workspace = true }

crates/trident-acl-agent/src/annotations/orchestrator.rs

@@ -536,6 +536,43 @@ impl Orchestrator {
             return Ok(());
         }
 
+        // Nebraska's reported hash is required, not optional: it is Trident's
+        // only signal to verify the downloaded image's identity before
+        // installing it, so a missing/unusable hash is a hard failure here
+        // rather than a silently-skipped check. See
+        // `PackageHash::to_cosi_sha384` for why this needs converting (our
+        // Nebraska deployment stores a base64 SHA-384 in the field Omaha
+        // calls `sha1`) rather than being forwarded as-is. Checked before the
+        // DownloadStarted event below so a bad hash fails the request instead
+        // of reporting a download that will never usefully start.
+        let hash = match offered
+            .primary
+            .hash
+            .as_ref()
+            .ok_or_else(|| "Nebraska offered no package hash".to_string())
+            .and_then(|h| h.to_cosi_sha384().map_err(|err| err.to_string()))
+        {
+            Ok(hash) => hash,
+            Err(err) => {
+                let status = UpdateStatus::new(
+                    &request,
+                    Operation::Stage,
+                    request.operation_id.clone(),
+                    StatusCode::OperationFailed,
+                    format!(
+                        "Nebraska did not report a usable package hash for '{}': {err}",
+                        offered.primary.name
+                    ),
+                    from_version,
+                    to_version,
+                    started,
+                    Some(Utc::now()),
+                );
+                self.record_and_publish(status).await?;
+                return Ok(());
+            }
+        };
+
         let current_ver = parse_nebraska_version(&from_version, "stage");
         if let Some(ref v) = current_ver {
             self.report_nebraska_event(
@@ -549,15 +586,12 @@ impl Orchestrator {
         }
 
         let mut client = TridentClient::connect(&self.config.trident.socket).await?;
-        // Integrity of the downloaded image is verified by Trident itself
-        // via the image's own COSI metadata, so the Nebraska-reported hash
-        // (offered.primary.hash) is not passed here.
         let result = self
             .run_with_status_heartbeat(
                 in_progress,
                 client.update_stage(
                     &offered.primary.url,
-                    None,
+                    Some(&hash),
                     self.config.orchestration.stage_timeout,
                 ),
             )

crates/trident-acl-agent/src/core/nebraska/client.rs

@@ -2,6 +2,7 @@
 
 use std::{thread, time::Duration};
 
+use base64::{engine::general_purpose::STANDARD, Engine as _};
 use log::{debug, trace, warn};
 use semver::Version;
 use url::Url;
@@ -83,18 +84,75 @@ pub struct PackageFile {
 
 /// The hash(es) of a file, as reported by Nebraska.
 ///
-/// Both values are base64-encoded and hash the *file* (not its contents).
-/// Nebraska reports a SHA-1; `sha256` is present only when the file was
+/// Both values are base64-encoded content hashes of the file: upstream
+/// Nebraska reports a SHA-1, and `sha256` is present only when the file was
 /// registered with one.
+///
+/// **Our Nebraska deployment does not follow that naming.** By internal
+/// convention, our server puts a base64-encoded **SHA-384** of the COSI
+/// image's metadata section into the `sha1` field instead of a real SHA-1 -
+/// the same value Trident itself computes and validates as `image.sha384`
+/// (see `crates/trident/src/osimage/cosi/mod.rs`). So despite the field's
+/// Omaha-inherited name, treat [`sha1`](PackageHash::sha1) as "the value our
+/// Nebraska calls `hash`", not as an actual SHA-1 digest - use
+/// [`to_cosi_sha384`](PackageHash::to_cosi_sha384) to get the value in the
+/// form Trident's gRPC API expects, rather than forwarding this field as-is.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct PackageHash {
-    /// Base64-encoded SHA-1 of the file.
+    /// Base64-encoded SHA-1 of the file, per the Omaha wire format - but see
+    /// this struct's docs: our Nebraska deployment actually puts a
+    /// base64-encoded SHA-384 here, not a SHA-1.
     pub sha1: String,
 
-    /// Base64-encoded SHA-256 of the file, when Nebraska provides it.
+    /// Base64-encoded SHA-256 of the file, when Nebraska provides it. Not
+    /// used by [`to_cosi_sha384`](PackageHash::to_cosi_sha384); our
+    /// deployment does not repurpose this field, so it carries a real SHA-256
+    /// (or is absent) same as upstream Omaha/Nebraska.
     pub sha256: Option<String>,
 }
 
+impl PackageHash {
+    /// Converts this hash into the hex-encoded SHA-384 checksum string that
+    /// Trident's gRPC `image.sha384` Host Configuration field expects.
+    ///
+    /// This is a straight re-encode (base64 -> raw bytes -> hex), not a real
+    /// hash-family conversion: per this struct's docs, our Nebraska
+    /// deployment already stores a SHA-384 digest in
+    /// [`sha1`](PackageHash::sha1), just base64-encoded instead of hex, so
+    /// there is no cryptographic conversion between algorithms happening
+    /// here - only a change of text encoding.
+    ///
+    /// Returns an error if the field does not decode to valid base64, or
+    /// decodes to something other than 48 bytes (the fixed size of a SHA-384
+    /// digest) - either means our "this field holds a SHA-384" assumption
+    /// does not hold for this response, and forwarding a wrong-sized/garbage
+    /// value on to Trident as an integrity check would be worse than failing
+    /// loudly here.
+    pub fn to_cosi_sha384(&self) -> Result<String, NebraskaError> {
+        const SHA384_LEN_BYTES: usize = 48;
+
+        // Decode straight into a fixed-size stack buffer instead of
+        // `Engine::decode`'s allocating `Vec<u8>` output: a malicious or
+        // buggy Nebraska response could otherwise send an arbitrarily large
+        // base64 string, forcing a large allocation before we ever get to
+        // checking the decoded length below. `decode_slice` rejects any
+        // input that would decode to more than `SHA384_LEN_BYTES` without
+        // allocating proportionally to the (untrusted) input size.
+        let mut raw = [0u8; SHA384_LEN_BYTES];
+        let decoded_len = STANDARD.decode_slice(&self.sha1, &mut raw).map_err(|err| {
+            NebraskaError::UnexpectedResponse(format!(
+                "Nebraska-reported hash is not valid base64, or does not fit in {SHA384_LEN_BYTES} bytes (SHA-384): {err}"
+            ))
+        })?;
+        if decoded_len != SHA384_LEN_BYTES {
+            return Err(NebraskaError::UnexpectedResponse(format!(
+                "Nebraska-reported hash decodes to {decoded_len} bytes, expected {SHA384_LEN_BYTES} (SHA-384)"
+            )));
+        }
+        Ok(hex::encode(raw))
+    }
+}
+
 /// The bounded exponential-backoff policy used by
 /// [`Client::complete_after_reboot`] when retrying transient failures.
 ///
@@ -1327,4 +1385,55 @@ mod tests {
         assert!(err.is_retryable());
         assert_eq!(client.transport.calls.get(), 1);
     }
+
+    #[test]
+    fn to_cosi_sha384_converts_base64_to_hex() {
+        // 48 zero bytes, base64-encoded - a well-formed (if not realistic)
+        // SHA-384 digest.
+        let hash = PackageHash {
+            sha1: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA".to_string(),
+            sha256: None,
+        };
+        assert_eq!(hash.to_cosi_sha384().unwrap(), "0".repeat(96));
+    }
+
+    #[test]
+    fn to_cosi_sha384_rejects_invalid_base64() {
+        let hash = PackageHash {
+            sha1: "not-valid-base64!!".to_string(),
+            sha256: None,
+        };
+        let err = hash.to_cosi_sha384().unwrap_err();
+        assert!(matches!(err, NebraskaError::UnexpectedResponse(_)));
+    }
+
+    #[test]
+    fn to_cosi_sha384_rejects_wrong_length() {
+        // Valid base64, but decodes to far fewer than the 48 bytes a SHA-384
+        // digest requires - e.g. a real SHA-1 (20 bytes), confirming this
+        // check would catch a Nebraska deployment that (unlike ours) puts an
+        // actual SHA-1 in this field.
+        let hash = PackageHash {
+            sha1: "AAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_string(),
+            sha256: None,
+        };
+        let err = hash.to_cosi_sha384().unwrap_err();
+        assert!(matches!(err, NebraskaError::UnexpectedResponse(_)));
+    }
+
+    #[test]
+    fn to_cosi_sha384_rejects_oversized_input() {
+        // Valid base64 that decodes to more than the 48 bytes a SHA-384
+        // digest requires. This must be rejected via the fixed-size decode
+        // buffer rather than by first allocating a `Vec` sized to the
+        // (potentially attacker-controlled) input.
+        // 68 base64 chars (no padding needed, 68 % 4 == 0) decode to 51
+        // bytes - more than the 48 a SHA-384 digest requires.
+        let hash = PackageHash {
+            sha1: "A".repeat(68),
+            sha256: None,
+        };
+        let err = hash.to_cosi_sha384().unwrap_err();
+        assert!(matches!(err, NebraskaError::UnexpectedResponse(_)));
+    }
 }

crates/trident-acl-agent/src/omahaonly/mod.rs

@@ -57,14 +57,31 @@ pub async fn run_omaha_only(config: &AgentConfig) -> Result<(), Error> {
         }
         CheckOutcome::UpdateAvailable(offer) => {
             info!("Triggering one-shot Omaha update to {}", offer.version);
+            // Nebraska's reported hash is required, not optional: it is
+            // Trident's only signal to verify the downloaded image's identity
+            // before installing it, so a missing hash is a hard failure here
+            // rather than a silently-skipped check. See
+            // `PackageHash::to_cosi_sha384` for why this needs converting
+            // (our Nebraska deployment stores a base64 SHA-384 in the field
+            // Omaha calls `sha1`) rather than being forwarded as-is.
+            let hash = offer
+                .primary
+                .hash
+                .as_ref()
+                .ok_or_else(|| {
+                    anyhow!(
+                        "Nebraska offered {} with no package hash for '{}'; refusing to update without an integrity check",
+                        offer.version,
+                        offer.primary.name
+                    )
+                })?
+                .to_cosi_sha384()
+                .context("Nebraska-reported hash is not a usable SHA-384")?;
             let mut client = TridentClient::connect(&config.trident.socket).await?;
             let combined_timeout =
                 config.orchestration.stage_timeout + config.orchestration.finalize_timeout;
-            // Integrity of the downloaded image is verified by Trident itself
-            // via the image's own COSI metadata, so the Nebraska-reported hash
-            // (offer.primary.hash) is not passed here.
             client
-                .update(&offer.primary.url, None, combined_timeout)
+                .update(&offer.primary.url, Some(&hash), combined_timeout)
                 .await?;
             Ok(())
         }