fix: transform payload to sha256 digest only for public key validator (#1711)

DavSanchez · web-flow · commit 773b3768ed37 · 2025-09-23T10:34:09.000Z
diff --git a/agent-control/src/opamp/remote_config/validators/signature/public_key.rs b/agent-control/src/opamp/remote_config/validators/signature/public_key.rs
@@ -1,7 +1,9 @@
 use crate::opamp::remote_config::signature::SigningAlgorithm;
 use crate::opamp::remote_config::validators::signature::public_key_fetcher::KeyData;
 use crate::opamp::remote_config::validators::signature::verifier::Verifier;
-use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD};
+use base64::engine::general_purpose::URL_SAFE_NO_PAD;
+use base64::{Engine, prelude::BASE64_STANDARD};
+use ring::digest;
 use ring::signature::{ED25519, UnparsedPublicKey};
 use thiserror::Error;
 
@@ -68,9 +70,18 @@ impl Verifier for PublicKey {
             ));
         }
 
-        self.public_key.verify(msg, signature).map_err(|_| {
-            PubKeyError::ValidatingSignature("signature verification failed".to_string())
-        })
+        // Actual implementation from FC side signs the Base64 representation of the SHA256 digest
+        // of the message (i.e. the remote configs). Hence, to verify the signature, we need to
+        // compute the SHA256 digest of the message, then Base64 encode it, and finally verify
+        // the signature against that.
+        let msg = digest::digest(&digest::SHA256, msg);
+        let msg = BASE64_STANDARD.encode(msg);
+
+        self.public_key
+            .verify(msg.as_bytes(), signature)
+            .map_err(|_| {
+                PubKeyError::ValidatingSignature("signature verification failed".to_string())
+            })
     }
 
     fn key_id(&self) -> &str {
@@ -86,6 +97,7 @@ mod tests {
     use crate::opamp::remote_config::validators::signature::verifier::Verifier;
     use base64::Engine;
     use base64::prelude::BASE64_STANDARD;
+    use ring::digest;
     use ring::rand::SystemRandom;
     use ring::signature::{Ed25519KeyPair, KeyPair, UnparsedPublicKey};
     use serde_json::json;
@@ -103,8 +115,11 @@ mod tests {
         let first_key = payload.keys.first().unwrap();
 
         let pub_key = PublicKey::try_new(first_key).unwrap();
+        // This is a direct call to the verify function. It isn't concerned with the digest
+        // and base64 transformation
         pub_key
-            .verify_signature(&ED25519, message.as_bytes(), signature.as_bytes())
+            .public_key
+            .verify(message.as_bytes(), signature.as_bytes())
             .unwrap();
     }
 
@@ -114,7 +129,14 @@ mod tests {
         let key_pair = Ed25519KeyPair::from_pkcs8(pkcs8_bytes.as_ref()).unwrap();
 
         const MESSAGE: &[u8] = b"hello, world";
-        let signature = key_pair.sign(MESSAGE);
+        // Actual implementation from FC side signs the Base64 representation of the SHA256 digest
+        // of the message (i.e. the remote configs). Hence, to verify the signature, we need to
+        // compute the SHA256 digest of the message, then Base64 encode it, and finally verify
+        // the signature against that.
+        let digest = digest::digest(&digest::SHA256, MESSAGE);
+        let msg = BASE64_STANDARD.encode(digest);
+
+        let signature = key_pair.sign(msg.as_bytes());
 
         let pub_key = PublicKey {
             key_id: "my-signing-key-test/0".to_string(),
diff --git a/agent-control/src/opamp/remote_config/validators/signature/public_key_fetcher.rs b/agent-control/src/opamp/remote_config/validators/signature/public_key_fetcher.rs
@@ -73,6 +73,7 @@ pub mod tests {
     use base64::Engine;
     use base64::prelude::{BASE64_STANDARD, BASE64_URL_SAFE_NO_PAD};
     use httpmock::prelude::*;
+    use ring::digest;
     use ring::rand::SystemRandom;
     use ring::signature::{Ed25519KeyPair, KeyPair};
     use serde_json::json;
@@ -135,7 +136,7 @@ pub mod tests {
             // of the message (i.e. the remote configs). Hence, to verify the signature, we need to
             // compute the SHA256 digest of the message, then Base64 encode it, and finally verify
             // the signature against that.
-            let digest = ring::digest::digest(&ring::digest::SHA256, msg);
+            let digest = digest::digest(&digest::SHA256, msg);
             let msg = BASE64_STANDARD.encode(digest);
             BASE64_STANDARD.encode(self.key_pair.sign(msg.as_bytes()).as_ref())
         }
diff --git a/agent-control/src/opamp/remote_config/validators/signature/validator.rs b/agent-control/src/opamp/remote_config/validators/signature/validator.rs
@@ -11,8 +11,6 @@ use crate::opamp::remote_config::validators::signature::public_key::PublicKey;
 use crate::opamp::remote_config::validators::signature::public_key_fetcher::PublicKeyFetcher;
 use crate::opamp::remote_config::validators::signature::verifier::VerifierStore;
 use crate::sub_agent::identity::AgentIdentity;
-use base64::Engine as _;
-use base64::prelude::BASE64_STANDARD;
 use serde::Deserialize;
 use std::path::PathBuf;
 use std::time::Duration;
@@ -214,15 +212,8 @@ impl RemoteConfigValidator for CompositeSignatureValidator {
 
         let config_content = opamp_remote_config
             .get_unique()
-            .map_err(|e| SignatureValidatorError::VerifySignature(e.to_string()))?;
-
-        // Actual implementation from FC side signs the Base64 representation of the SHA256 digest
-        // of the message (i.e. the remote configs). Hence, to verify the signature, we need to
-        // compute the SHA256 digest of the message, then Base64 encode it, and finally verify
-        // the signature against that.
-        let digest = ring::digest::digest(&ring::digest::SHA256, config_content.as_ref());
-        let digest_b64 = BASE64_STANDARD.encode(digest);
-        let msg = digest_b64.as_bytes();
+            .map_err(|e| SignatureValidatorError::VerifySignature(e.to_string()))?
+            .as_bytes();
 
         // Until backend migrates to new signature platform, the validation starts with the public key based,
         // and falls back to cert based in case of failure.
@@ -232,7 +223,7 @@ impl RemoteConfigValidator for CompositeSignatureValidator {
             match public_key_store.verify_signature(
                 signature.signature_algorithm(),
                 signature.key_id(),
-                msg,
+                config_content,
                 signature.signature(),
             ) {
                 Ok(()) => return Ok(()),
@@ -251,7 +242,7 @@ impl RemoteConfigValidator for CompositeSignatureValidator {
             .verify_signature(
                 signature.signature_algorithm(),
                 signature.key_id(),
-                msg,
+                config_content,
                 signature.signature(),
             )
             .map_err(|e| SignatureValidatorError::VerifySignature(e.to_string()))
@@ -677,14 +668,8 @@ pub mod tests {
         );
 
         let config = "value";
-        // Actual implementation from FC side signs the Base64 representation of the SHA256 digest
-        // of the message (i.e. the remote configs). Hence, to verify the signature, we need to
-        // compute the SHA256 digest of the message, then Base64 encode it, and finally verify
-        // the signature against that.
-        let digest = ring::digest::digest(&ring::digest::SHA256, config.as_bytes());
-        let msg = BASE64_STANDARD.encode(digest);
-
-        let encoded_signature = test_signer.encoded_signature(&msg);
+
+        let encoded_signature = test_signer.encoded_signature(config);
         let remote_config = OpampRemoteConfig::new(
             AgentID::AgentControl,
             Hash::from("test"),
diff --git a/agent-control/tests/common/opamp.rs b/agent-control/tests/common/opamp.rs
@@ -15,6 +15,7 @@ use opamp_client::opamp::proto::{
 use opamp_client::operation::instance_uid::InstanceUid;
 use prost::Message;
 use rcgen::{CertificateParams, KeyPair, PKCS_ED25519, PublicKeyData};
+use ring::digest;
 use ring::rand::SystemRandom;
 use ring::signature::{Ed25519KeyPair, KeyPair as _};
 use serde_json::json;
@@ -277,7 +278,16 @@ async fn opamp_handler(state: web::Data<Arc<Mutex<ServerState>>>, req: web::Byte
         (&server_state.key_pair, public_key_fingerprint(&public_key))
     };
 
-    let server_to_agent = build_response(identifier, remote_config, key_pair, key_id, flags);
+    let use_legacy_signature = server_state.use_legacy_signatures;
+
+    let server_to_agent = build_response(
+        identifier,
+        remote_config,
+        key_pair,
+        key_id,
+        flags,
+        use_legacy_signature,
+    );
     HttpResponse::Ok().body(server_to_agent)
 }
 
@@ -308,6 +318,7 @@ fn build_response(
     key_pair: &Ed25519KeyPair,
     key_id: String,
     flags: u64,
+    use_legacy_signature: bool,
 ) -> Vec<u8> {
     let mut remote_config = None;
     let mut custom_message = None;
@@ -326,19 +337,23 @@ fn build_response(
             }),
         });
 
-        // Actual implementation from FC side signs the Base64 representation of the SHA256 digest
-        // of the message (i.e. the remote configs). Hence, to verify the signature, we need to
-        // compute the SHA256 digest of the message, then Base64 encode it, and finally verify
-        // the signature against that.
-        let digest = ring::digest::digest(&ring::digest::SHA256, config.raw_body.as_bytes());
-        let msg = BASE64_STANDARD.encode(digest);
+        let msg = if use_legacy_signature {
+            config.raw_body
+        } else {
+            // Actual implementation from FC side signs the Base64 representation of the SHA256 digest
+            // of the message (i.e. the remote configs). Hence, to verify the signature, we need to
+            // compute the SHA256 digest of the message, then Base64 encode it, and finally verify
+            // the signature against that.
+            let digest = digest::digest(&digest::SHA256, config.raw_body.as_bytes());
+            BASE64_STANDARD.encode(digest)
+        };
 
         let signature = key_pair.sign(msg.as_bytes());
 
         let custom_message_data = HashMap::from([(
             "fakeCRC".to_string(), //AC is not using the CRC.
             vec![SignatureFields {
-                signature: BASE64_STANDARD.encode(signature.as_ref()),
+                signature: BASE64_STANDARD.encode(signature),
                 signing_algorithm: ED25519,
                 key_id,
             }],
