Skip to content

Commit d6a01e8

Browse files
fix: align remote config digest signature (#1704)
* fix(workaround) * feat: encode msg into b64 * style: move digest generation to upper layers * test: fix * style: ignore ascii case --------- Co-authored-by: David Sánchez <davidslt+git@pm.me>
1 parent 77803ac commit d6a01e8

File tree

6 files changed

+40
-13
lines changed

6 files changed

+40
-13
lines changed

agent-control/src/opamp/remote_config/validators/signature/public_key.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use crate::opamp::remote_config::signature::SigningAlgorithm;
22
use crate::opamp::remote_config::validators::signature::public_key_fetcher::KeyData;
33
use crate::opamp::remote_config::validators::signature::verifier::Verifier;
4-
use base64::Engine;
5-
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
4+
use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD};
65
use ring::signature::{ED25519, UnparsedPublicKey};
76
use thiserror::Error;
87

agent-control/src/opamp/remote_config/validators/signature/public_key_fetcher.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,13 @@ pub mod tests {
131131
}
132132
}
133133
pub fn sign(&self, msg: &[u8]) -> String {
134-
BASE64_STANDARD.encode(self.key_pair.sign(msg).as_ref())
134+
// Actual implementation from FC side signs the Base64 representation of the SHA256 digest
135+
// of the message (i.e. the remote configs). Hence, to verify the signature, we need to
136+
// compute the SHA256 digest of the message, then Base64 encode it, and finally verify
137+
// the signature against that.
138+
let digest = ring::digest::digest(&ring::digest::SHA256, msg);
139+
let msg = BASE64_STANDARD.encode(digest);
140+
BASE64_STANDARD.encode(self.key_pair.sign(msg.as_bytes()).as_ref())
135141
}
136142
}
137143

agent-control/src/opamp/remote_config/validators/signature/validator.rs

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ use crate::opamp::remote_config::validators::signature::public_key::PublicKey;
1111
use crate::opamp::remote_config::validators::signature::public_key_fetcher::PublicKeyFetcher;
1212
use crate::opamp::remote_config::validators::signature::verifier::VerifierStore;
1313
use crate::sub_agent::identity::AgentIdentity;
14+
use base64::Engine as _;
15+
use base64::prelude::BASE64_STANDARD;
1416
use serde::Deserialize;
1517
use std::path::PathBuf;
1618
use std::time::Duration;
@@ -212,8 +214,15 @@ impl RemoteConfigValidator for CompositeSignatureValidator {
212214

213215
let config_content = opamp_remote_config
214216
.get_unique()
215-
.map_err(|e| SignatureValidatorError::VerifySignature(e.to_string()))?
216-
.as_bytes();
217+
.map_err(|e| SignatureValidatorError::VerifySignature(e.to_string()))?;
218+
219+
// Actual implementation from FC side signs the Base64 representation of the SHA256 digest
220+
// of the message (i.e. the remote configs). Hence, to verify the signature, we need to
221+
// compute the SHA256 digest of the message, then Base64 encode it, and finally verify
222+
// the signature against that.
223+
let digest = ring::digest::digest(&ring::digest::SHA256, config_content.as_ref());
224+
let digest_b64 = BASE64_STANDARD.encode(digest);
225+
let msg = digest_b64.as_bytes();
217226

218227
// Until backend migrates to new signature platform, the validation starts with the public key based,
219228
// and falls back to cert based in case of failure.
@@ -223,7 +232,7 @@ impl RemoteConfigValidator for CompositeSignatureValidator {
223232
match public_key_store.verify_signature(
224233
signature.signature_algorithm(),
225234
signature.key_id(),
226-
config_content,
235+
msg,
227236
signature.signature(),
228237
) {
229238
Ok(()) => return Ok(()),
@@ -242,7 +251,7 @@ impl RemoteConfigValidator for CompositeSignatureValidator {
242251
.verify_signature(
243252
signature.signature_algorithm(),
244253
signature.key_id(),
245-
config_content,
254+
msg,
246255
signature.signature(),
247256
)
248257
.map_err(|e| SignatureValidatorError::VerifySignature(e.to_string()))
@@ -668,8 +677,14 @@ pub mod tests {
668677
);
669678

670679
let config = "value";
671-
672-
let encoded_signature = test_signer.encoded_signature(config);
680+
// Actual implementation from FC side signs the Base64 representation of the SHA256 digest
681+
// of the message (i.e. the remote configs). Hence, to verify the signature, we need to
682+
// compute the SHA256 digest of the message, then Base64 encode it, and finally verify
683+
// the signature against that.
684+
let digest = ring::digest::digest(&ring::digest::SHA256, config.as_bytes());
685+
let msg = BASE64_STANDARD.encode(digest);
686+
687+
let encoded_signature = test_signer.encoded_signature(&msg);
673688
let remote_config = OpampRemoteConfig::new(
674689
AgentID::AgentControl,
675690
Hash::from("test"),

agent-control/src/opamp/remote_config/validators/signature/verifier.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ where
9494
.fetch()
9595
.map_err(|err| VerifierStoreError::Fetch(err.to_string()))?;
9696

97-
if !verifier.key_id().eq(key_id) {
97+
if !verifier.key_id().eq_ignore_ascii_case(key_id) {
9898
return Err(VerifierStoreError::KeyMismatch {
9999
signature_key_id: key_id.to_string(),
100100
certificate_key_id: verifier.key_id().to_string(),

agent-control/tests/common/opamp.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,14 @@ fn build_response(
326326
}),
327327
});
328328

329-
let signature = key_pair.sign(config.raw_body.as_bytes());
329+
// Actual implementation from FC side signs the Base64 representation of the SHA256 digest
330+
// of the message (i.e. the remote configs). Hence, to verify the signature, we need to
331+
// compute the SHA256 digest of the message, then Base64 encode it, and finally verify
332+
// the signature against that.
333+
let digest = ring::digest::digest(&ring::digest::SHA256, config.raw_body.as_bytes());
334+
let msg = BASE64_STANDARD.encode(digest);
335+
336+
let signature = key_pair.sign(msg.as_bytes());
330337

331338
let custom_message_data = HashMap::from([(
332339
"fakeCRC".to_string(), //AC is not using the CRC.

agent-control/tests/on_host/tools/custom_agent_type.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,14 +117,14 @@ impl CustomAgentType {
117117

118118
pub fn with_health(self, health: Option<&str>) -> Self {
119119
Self {
120-
health: health.map(|h| (serde_yaml::from_str(h).unwrap())),
120+
health: health.map(|h| serde_yaml::from_str(h).unwrap()),
121121
..self
122122
}
123123
}
124124

125125
pub fn with_version(self, version: Option<&str>) -> Self {
126126
Self {
127-
version: version.map(|v| (serde_yaml::from_str(v).unwrap())),
127+
version: version.map(|v| serde_yaml::from_str(v).unwrap()),
128128
..self
129129
}
130130
}

0 commit comments

Comments
 (0)