diff --git a/Cargo.lock b/Cargo.lock index 8035a71e5b..08d4fa460b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2631,7 +2631,6 @@ dependencies = [ "ring", "rstest", "rustls", - "rustls-pemfile", "schemars", "semver", "serde", @@ -2649,7 +2648,6 @@ dependencies = [ "tracing-subscriber", "tracing-test", "url", - "webpki", "wrapper_with_default", "x509-parser", ] @@ -3655,15 +3653,6 @@ dependencies = [ "security-framework", ] -[[package]] -name = "rustls-pemfile" -version = "2.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dce314e5fee3f39953d46bb63bb8a46d40c2f8fb7cc5a3b6cab2bde9721d6e50" -dependencies = [ - "rustls-pki-types", -] - [[package]] name = "rustls-pki-types" version = "1.12.0" @@ -4910,16 +4899,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "webpki" -version = "0.22.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed63aea5ce73d0ff405984102c42de94fc55a6b75765d621c65262469b3c9b53" -dependencies = [ - "ring", - "untrusted", -] - [[package]] name = "webpki-roots" version = "1.0.2" diff --git a/THIRD_PARTY_NOTICES.md b/THIRD_PARTY_NOTICES.md index 58e3ca0bd1..5ddcb316d8 100644 --- a/THIRD_PARTY_NOTICES.md +++ b/THIRD_PARTY_NOTICES.md @@ -2552,12 +2552,6 @@ Distributed under the following license(s): * MIT * Apache-2.0 -## webpki - -Distributed under the following license(s): - -* ISC - ## webpki-roots Distributed under the following license(s): diff --git a/agent-control/Cargo.toml b/agent-control/Cargo.toml index 5796168012..eb01b82b2a 100644 --- a/agent-control/Cargo.toml +++ b/agent-control/Cargo.toml @@ -62,7 +62,6 @@ opentelemetry-semantic-conventions = { version = "0.30.0", features = [ http-serde = "2.1.1" config = { version = "0.15.18", features = ["yaml"] } rustls = { version = "0.23.32", features = ["ring"] } -webpki = { version = "0.22.4", features = ["alloc"] } x509-parser = "0.18.0" ring = "0.17.14" bytes = "1.10.1" @@ -97,7 +96,6 @@ httpmock = { version = "0.8.0", features = ["proxy"] } serial_test = "3.2.0" futures = "0.3.31" rcgen = { version = "0.14.5", features = ["crypto"] } -rustls-pemfile = { version = "2.2.0" } rstest = "0.26.1" tokio-stream = { version = "0.1.17", features = ["net"] } diff --git a/agent-control/src/agent_control/config.rs b/agent-control/src/agent_control/config.rs index 28cf51b6e9..28b66507e8 100644 --- a/agent-control/src/agent_control/config.rs +++ b/agent-control/src/agent_control/config.rs @@ -181,7 +181,7 @@ impl<'de> Deserialize<'de> for OpAMPClientConfig { #[serde(default)] fleet_id: String, #[serde(default)] - pub signature_validation: SignatureValidatorConfig, + signature_validation: SignatureValidatorConfig, } let mut intermediate_spec = IntermediateOpAMPClientConfig::deserialize(deserializer)?; diff --git a/agent-control/src/agent_control/run.rs b/agent-control/src/agent_control/run.rs index b5dcd3a0cc..e05a5dcc1e 100644 --- a/agent-control/src/agent_control/run.rs +++ b/agent-control/src/agent_control/run.rs @@ -15,9 +15,7 @@ use crate::http::config::ProxyConfig; use crate::opamp::auth::token_retriever::TokenRetrieverImpl; use crate::opamp::client_builder::PollInterval; use crate::opamp::http::builder::OpAMPHttpClientBuilder; -use crate::opamp::remote_config::validators::signature::validator::{ - SignatureValidator, build_signature_validator, -}; +use crate::opamp::remote_config::validators::signature::validator::SignatureValidator; use std::error::Error; use std::fmt::{self, Display, Formatter}; use std::path::PathBuf; @@ -160,10 +158,10 @@ impl AgentControlRunner { let signature_validator = config .opamp .map(|fleet_config| { - build_signature_validator(fleet_config.signature_validation, config.proxy) + SignatureValidator::new(fleet_config.signature_validation, config.proxy) }) .transpose()? - .unwrap_or(SignatureValidator::Noop); + .unwrap_or(SignatureValidator::new_noop()); Ok(AgentControlRunner { http_server_runner, diff --git a/agent-control/src/opamp/remote_config/signature.rs b/agent-control/src/opamp/remote_config/signature.rs index 7f1be31c63..fa0fab42f3 100644 --- a/agent-control/src/opamp/remote_config/signature.rs +++ b/agent-control/src/opamp/remote_config/signature.rs @@ -1,7 +1,5 @@ use opamp_client::opamp::proto::CustomMessage; -use regex::bytes::Regex; use serde::{Deserialize, Serialize}; -use std::sync::OnceLock; use std::{collections::HashMap, fmt::Debug}; use thiserror::Error; @@ -10,35 +8,12 @@ pub const SIGNATURE_CUSTOM_CAPABILITY: &str = "com.newrelic.security.configSigna /// signature custom message type pub const SIGNATURE_CUSTOM_MESSAGE_TYPE: &str = "newrelicRemoteConfigSignature"; // Supported signature algorithms -// RSA regex matching supported RSA signature algorithms, length between 2048 and 8192 bits -pub const RSA_REGEX: &str = "RSA_PKCS1_([0-9]+)_SHA(256|384|512)"; -pub const RSA_PKCS1_2048_8192_SHA256: &str = "RSA_PKCS1_2048_8192_SHA256"; -pub const RSA_PKCS1_2048_8192_SHA384: &str = "RSA_PKCS1_2048_8192_SHA384"; -pub const RSA_PKCS1_2048_8192_SHA512: &str = "RSA_PKCS1_2048_8192_SHA512"; -pub const ECDSA_P256_SHA256: &str = "ECDSA_P256_SHA256"; -pub const ECDSA_P256_SHA384: &str = "ECDSA_P256_SHA384"; -pub const ECDSA_P384_SHA256: &str = "ECDSA_P384_SHA256"; -pub const ECDSA_P384_SHA384: &str = "ECDSA_P384_SHA384"; pub const ED25519: &str = "ED25519"; -fn rsa_regex() -> &'static Regex { - static RE_ONCE: OnceLock = OnceLock::new(); - RE_ONCE.get_or_init(|| Regex::new(RSA_REGEX).unwrap()) -} - -// TODO: simplify supported algorithms when removing support for certificate verification /// Defines the supported algorithms for signing #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(try_from = "&str")] -#[allow(non_camel_case_types)] pub enum SigningAlgorithm { - RSA_PKCS1_2048_8192_SHA256, - RSA_PKCS1_2048_8192_SHA384, - RSA_PKCS1_2048_8192_SHA512, - ECDSA_P256_SHA256, - ECDSA_P256_SHA384, - ECDSA_P384_SHA256, - ECDSA_P384_SHA384, ED25519, } @@ -46,14 +21,7 @@ impl TryFrom<&str> for SigningAlgorithm { type Error = SignatureError; fn try_from(s: &str) -> Result { - if let Some(rsa_algorithm) = parse_rsa_algorithm(s) { - return Ok(rsa_algorithm); - } match s.to_uppercase().as_str() { - ECDSA_P256_SHA256 => Ok(Self::ECDSA_P256_SHA256), - ECDSA_P256_SHA384 => Ok(Self::ECDSA_P256_SHA384), - ECDSA_P384_SHA256 => Ok(Self::ECDSA_P384_SHA256), - ECDSA_P384_SHA384 => Ok(Self::ECDSA_P384_SHA384), ED25519 => Ok(Self::ED25519), _unsupported_algorithm => Err(SignatureError::UnsupportedAlgorithm(s.to_string())), } @@ -63,65 +31,28 @@ impl TryFrom<&str> for SigningAlgorithm { impl AsRef for SigningAlgorithm { fn as_ref(&self) -> &str { match self { - SigningAlgorithm::RSA_PKCS1_2048_8192_SHA256 => RSA_PKCS1_2048_8192_SHA256, - SigningAlgorithm::RSA_PKCS1_2048_8192_SHA384 => RSA_PKCS1_2048_8192_SHA384, - SigningAlgorithm::RSA_PKCS1_2048_8192_SHA512 => RSA_PKCS1_2048_8192_SHA512, - SigningAlgorithm::ECDSA_P256_SHA256 => ECDSA_P256_SHA256, - SigningAlgorithm::ECDSA_P256_SHA384 => ECDSA_P256_SHA384, - SigningAlgorithm::ECDSA_P384_SHA256 => ECDSA_P384_SHA256, - SigningAlgorithm::ECDSA_P384_SHA384 => ECDSA_P384_SHA384, SigningAlgorithm::ED25519 => ED25519, } } } -impl TryFrom<&SigningAlgorithm> for &webpki::SignatureAlgorithm { - type Error = SignatureError; - fn try_from(value: &SigningAlgorithm) -> Result { - let algorithm = match value { - SigningAlgorithm::RSA_PKCS1_2048_8192_SHA256 => &webpki::RSA_PKCS1_2048_8192_SHA256, - SigningAlgorithm::RSA_PKCS1_2048_8192_SHA384 => &webpki::RSA_PKCS1_2048_8192_SHA384, - SigningAlgorithm::RSA_PKCS1_2048_8192_SHA512 => &webpki::RSA_PKCS1_2048_8192_SHA512, - SigningAlgorithm::ECDSA_P256_SHA256 => &webpki::ECDSA_P256_SHA256, - SigningAlgorithm::ECDSA_P256_SHA384 => &webpki::ECDSA_P256_SHA384, - SigningAlgorithm::ECDSA_P384_SHA256 => &webpki::ECDSA_P384_SHA256, - SigningAlgorithm::ECDSA_P384_SHA384 => &webpki::ECDSA_P384_SHA384, - SigningAlgorithm::ED25519 => &webpki::ED25519, - }; - Ok(algorithm) - } -} - -// parses the RSA algorithm string coming from the server into a supported signature algorithm -// example: "RSA_PKCS1_2048_SHA256" -> RSA_PKCS1_2048_8192_SHA256 -// example: "RSA_PKCS1_4444_SHA256" -> RSA_PKCS1_2048_8192_SHA256 -fn parse_rsa_algorithm(algo: &str) -> Option { - let m = rsa_regex().captures(algo.as_bytes())?; - let (_, [lenght_bytes, hash_bytes]) = m.extract(); - - // Validate the length is within the supported range - let length = std::str::from_utf8(lenght_bytes) - .ok() - .and_then(|s| s.parse::().ok())?; - if !(2048..=8192).contains(&length) { - return None; - } - - match hash_bytes { - b"256" => Some(SigningAlgorithm::RSA_PKCS1_2048_8192_SHA256), - b"384" => Some(SigningAlgorithm::RSA_PKCS1_2048_8192_SHA384), - b"512" => Some(SigningAlgorithm::RSA_PKCS1_2048_8192_SHA512), - _ => None, - } -} - +/// Holds the signature custom message data. It is coupled to a RemoteConfig message and +/// should be present in the same ServerToAgent message. +/// +/// Even if each config identifier may contain many signature details (it holds an array) it is deserialized +/// as a single structure of [SignatureData] containing the first signature with a supported algorithm. +/// /// In order to mitigate MITM attacks, the OpAMP server signs the remote configuration and sends the /// signature data as part of a CustomMessage in the same ServerToAgent message where the RemoteConfig is sent. /// Agent control will verify that the signature and the configuration data match. `SignatureValidator` is /// responsible for verifying the signature with the certificate fetched from the server. /// -/// The signature will consist in a encrypted hash of the configuration data, signed with a private key. -/// The public key is distributed to the agents in the form of a HTTPS server TLS certificate. +/// The signed message is consist in the remote config standard encoded base64 sha256 of the config body, which is signed +/// with the private key and algorithm specified in the custom_message. +/// Public key is distributed in JWKS format in the following endpoints: +/// https://staging-publickeys.newrelic.com/r/blob-management/global/agentconfiguration/jwks.json +/// https://publickeys.eu.newrelic.com/r/blob-management/global/agentconfiguration/jwks.json +/// https://publickeys.newrelic.com/r/blob-management/global/agentconfiguration/jwks.json /// /// Example: /// ```json @@ -130,64 +61,26 @@ fn parse_rsa_algorithm(algo: &str) -> Option { /// config: { /// config_map: { /// "agentConfig": { -/// body: "chart_version: 1.10.12\nchart_values:\n podLabels: \"192.168.5.0\"" +/// body: "chart_version: \"6.0.1\"" /// content_type: "" /// } /// } /// } -/// config_hash: "817982697f614312018935c351fdd71aca190f106fda2d7bc69da86e878ea5e4" +/// config_hash: "b5c6779371b3b1e608b55d0b0b4d970afa1d97f176d60cc8a7034b2b2d12da66" /// } /// custom_message:{ /// capability: "com.newrelic.security.configSignature" /// type: "newrelicRemoteConfigSignature" /// data: { /// "agentConfig": [{ -/// "checksum": "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", -/// "checksumAlgorithm": "SHA256", -/// "signature": "nppw2CuZg+YO5MsEoNOsHlgHxF7qAwWPli37NGXAr5isfP1jUTSJcLi0l7k9lNlpbq31GF9DZ0JQBZhoGS0j+sDjvirKSb7yXdqj6JcZ8sxax7KWAnk5QPiwLHFA1kGmszVJ/ccbwtVozG46FvKedcc3X5RME/HGdJupKBe3UzmJawL0xs9jNY+9519CL+CpbkBl/WgCvrIUhTNZv5TUHK23hMD+kz1Brf60pW7MQVtsyClOllsb6WhAsSXdhkpSCJ+96ZGyYywUlvx3/vkBM5a7q4IWqiPM4U0LPZDMQJQCCpxWV3T7cnIR1Ye2yYUqJHs9vfKmTWeBKH2Tb5FgpQ==", -/// "signingAlgorithm": "RSA_PKCS1_2048_SHA256", -/// "signatureSpecification": "PKCS #1 v2.2", -/// "signingDomain": "iast-csec-se.test-poised-pear.cell.us.nr-data.net", -/// "keyId": "778b223984d389ad6555bdbbbf118420290c53296b6511e1964309965ec5f710" +/// "signature": "YHUmpyXFyCw9LP4NbpGtBpY7u9iu0zWpkGv0ePw4WA2sCSJqdYK3G2RRVgIjHcWlFNwX8p4Yc+CQdGBDvr5RCw==", +/// "signingAlgorithm": "ED25519", +/// "keyId": "AgentConfiguration/0" /// }] /// } /// } /// } /// ``` -/// `Signatures` holds the signature custom message data. It is coupled to a RemoteConfig message and -/// should be present in the same ServerToAgent message. -/// -/// Even if each config identifier may contain many signature details (it holds an array) it is deserialized -/// as a single structure of [SignatureData] containing the first signature with a supported algorithm. -/// -/// Example: -/// ``` -/// use crate::newrelic_agent_control::opamp::remote_config::signature::Signatures; -/// -/// let data= r#"{ -/// "agentConfig": [ -/// { -/// "signature": "some signature", -/// "signingAlgorithm": "UNSUPPORTED", -/// "keyId": "some key id" -/// }, -/// { -/// "signature": "some signature", -/// "signingAlgorithm": "ED25519", -/// "keyId": "some key id" -/// }, -/// { -/// "signature": "some signature", -/// "signingAlgorithm": "RSA_PKCS1_2048_SHA256", -/// "keyId": "some key id" -/// } -/// ] -/// }"#.as_bytes().to_vec(); -/// -/// let signatures: Signatures = serde_json::from_slice(&data).unwrap(); -/// let (_, signature) = signatures.signatures.iter().next().unwrap(); -/// assert_eq!(signature.signing_algorithm.as_ref(), "ED25519"); -/// ``` #[derive(Debug, Serialize, PartialEq, Clone)] pub struct Signatures { #[serde(flatten)] @@ -250,9 +143,7 @@ pub struct SignatureFields { pub signature: String, /// Public key identifier. pub key_id: String, - /// Signing algorithm used the config: - /// [ECDSA_P256_SHA256,ECDSA_P256_SHA384,ECDSA_P384_SHA256,ECDSA_P384_SHA384,RSA_PKCS1_[2048-8192]_SHA256, - /// RSA_PKCS1_2048_8192_SHA384,RSA_PKCS1_2048_8192_SHA512,RSA_PKCS1_3072_8192_SHA384,ED25519] + /// Signing algorithm. pub signing_algorithm: A, } @@ -327,9 +218,6 @@ mod tests { use super::SignatureData; use super::Signatures; use crate::opamp::remote_config::DEFAULT_AGENT_CONFIG_IDENTIFIER; - use crate::opamp::remote_config::signature::ECDSA_P256_SHA256; - use crate::opamp::remote_config::signature::ECDSA_P256_SHA384; - use crate::opamp::remote_config::signature::ED25519; use crate::opamp::remote_config::signature::SigningAlgorithm; use opamp_client::opamp::proto::CustomMessage; use std::collections::HashMap; @@ -386,121 +274,14 @@ mod tests { capability: super::SIGNATURE_CUSTOM_CAPABILITY.to_string(), r#type: super::SIGNATURE_CUSTOM_MESSAGE_TYPE.to_string(), data: r#"{ - "3936250589": [{ - "checksum": "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", - "checksumAlgorithm": "SHA256", - "signature": "nppw2CuZg+YO5MsEoNOsHlgHxF7qAwWPli37NGXAr5isfP1jUTSJcLi0l7k9lNlpbq31GF9DZ0JQBZhoGS0j+sDjvirKSb7yXdqj6JcZ8sxax7KWAnk5QPiwLHFA1kGmszVJ/ccbwtVozG46FvKedcc3X5RME/HGdJupKBe3UzmJawL0xs9jNY+9519CL+CpbkBl/WgCvrIUhTNZv5TUHK23hMD+kz1Brf60pW7MQVtsyClOllsb6WhAsSXdhkpSCJ+96ZGyYywUlvx3/vkBM5a7q4IWqiPM4U0LPZDMQJQCCpxWV3T7cnIR1Ye2yYUqJHs9vfKmTWeBKH2Tb5FgpQ==", - "signingAlgorithm": "RSA_PKCS1_2048_SHA256", - "signatureSpecification": "PKCS #1 v2.2", - "signingDomain": "iast-csec-se.test-poised-pear.cell.us.nr-data.net", - "keyId": "778b223984d389ad6555bdbbbf118420290c53296b6511e1964309965ec5f710" - }] - }"#.as_bytes().to_vec(), - }, - algorithm: SigningAlgorithm::RSA_PKCS1_2048_8192_SHA256, - }, - TestCase { - name: "required fields only, RSA_PKCS1_2048_SHA256", - custom_message: CustomMessage { - capability: super::SIGNATURE_CUSTOM_CAPABILITY.to_string(), - r#type: super::SIGNATURE_CUSTOM_MESSAGE_TYPE.to_string(), - data: r#"{ - "3936250589": [{ - "signature": "fake", - "signingAlgorithm": "RSA_PKCS1_2048_SHA256", - "keyId": "fake" - }] - }"#.as_bytes().to_vec(), - }, - algorithm: SigningAlgorithm::RSA_PKCS1_2048_8192_SHA256, - }, - TestCase { - name: "RSA_PKCS1_2048_SHA512", - custom_message: CustomMessage { - capability: super::SIGNATURE_CUSTOM_CAPABILITY.to_string(), - r#type: super::SIGNATURE_CUSTOM_MESSAGE_TYPE.to_string(), - data: r#"{ - "3936250589": [{ - "signature": "fake", - "signingAlgorithm": "RSA_PKCS1_2048_SHA512", - "keyId": "fake" - }] - }"#.as_bytes().to_vec(), - }, - algorithm: SigningAlgorithm::RSA_PKCS1_2048_8192_SHA512, - }, - TestCase { - name: "RSA_PKCS1_2049_SHA512", - custom_message: CustomMessage { - capability: super::SIGNATURE_CUSTOM_CAPABILITY.to_string(), - r#type: super::SIGNATURE_CUSTOM_MESSAGE_TYPE.to_string(), - data: r#"{ - "3936250589": [{ - "signature": "fake", - "signingAlgorithm": "RSA_PKCS1_2049_SHA512", - "keyId": "fake" - }] - }"#.as_bytes().to_vec(), - }, - algorithm: SigningAlgorithm::RSA_PKCS1_2048_8192_SHA512, - }, - TestCase { - name: "RSA_PKCS1_3072_SHA384", - custom_message: CustomMessage { - capability: super::SIGNATURE_CUSTOM_CAPABILITY.to_string(), - r#type: super::SIGNATURE_CUSTOM_MESSAGE_TYPE.to_string(), - data: r#"{ - "3936250589": [{ - "signature": "fake", - "signingAlgorithm": "RSA_PKCS1_3072_SHA384", - "keyId": "fake" - }] - }"#.as_bytes().to_vec(), - }, - algorithm: SigningAlgorithm::RSA_PKCS1_2048_8192_SHA384, - }, - TestCase { - name: ECDSA_P256_SHA256, - custom_message: CustomMessage { - capability: super::SIGNATURE_CUSTOM_CAPABILITY.to_string(), - r#type: super::SIGNATURE_CUSTOM_MESSAGE_TYPE.to_string(), - data: r#"{ - "3936250589": [{ - "signature": "fake", - "signingAlgorithm": "ECDSA_P256_SHA256", - "keyId": "fake" - }] - }"#.as_bytes().to_vec(), - }, - algorithm: SigningAlgorithm::ECDSA_P256_SHA256, - }, - TestCase { - name: ECDSA_P256_SHA384, - custom_message: CustomMessage { - capability: super::SIGNATURE_CUSTOM_CAPABILITY.to_string(), - r#type: super::SIGNATURE_CUSTOM_MESSAGE_TYPE.to_string(), - data: r#"{ - "3936250589": [{ - "signature": "fake", - "signingAlgorithm": "ECDSA_P256_SHA384", - "keyId": "fake" - }] - }"#.as_bytes().to_vec(), - }, - algorithm: SigningAlgorithm::ECDSA_P256_SHA384, - }, - TestCase { - name: ED25519, - custom_message: CustomMessage { - capability: super::SIGNATURE_CUSTOM_CAPABILITY.to_string(), - r#type: super::SIGNATURE_CUSTOM_MESSAGE_TYPE.to_string(), - data: r#"{ - "3936250589": [{ + "someConfigKey": [{ "signature": "fake", "signingAlgorithm": "ED25519", - "keyId": "fake" + "keyId": "ac/0" }] - }"#.as_bytes().to_vec(), + }"# + .as_bytes() + .to_vec(), }, algorithm: SigningAlgorithm::ED25519, }, @@ -522,11 +303,12 @@ mod tests { "keyId": "fake" } ] - }"#.as_bytes().to_vec(), + }"# + .as_bytes() + .to_vec(), }, algorithm: SigningAlgorithm::ED25519, }, - ]; for test_case in test_cases { @@ -612,7 +394,7 @@ mod tests { ); assert_eq!( signatures.signatures.get("2").unwrap().signing_algorithm, - SigningAlgorithm::ECDSA_P256_SHA256 + SigningAlgorithm::ED25519 ); } diff --git a/agent-control/src/opamp/remote_config/validators/signature.rs b/agent-control/src/opamp/remote_config/validators/signature.rs index 78f2e87a71..e0063c9af8 100644 --- a/agent-control/src/opamp/remote_config/validators/signature.rs +++ b/agent-control/src/opamp/remote_config/validators/signature.rs @@ -1,8 +1,4 @@ -mod certificate; -mod certificate_fetcher; pub mod public_key; mod public_key_fetcher; pub mod validator; mod verifier; - -pub use certificate::public_key_fingerprint; diff --git a/agent-control/src/opamp/remote_config/validators/signature/certificate.rs b/agent-control/src/opamp/remote_config/validators/signature/certificate.rs deleted file mode 100644 index f9ade9fadf..0000000000 --- a/agent-control/src/opamp/remote_config/validators/signature/certificate.rs +++ /dev/null @@ -1,126 +0,0 @@ -use ring::digest; -use std::fmt::Write; -use thiserror::Error; -use tracing::debug; -use webpki::EndEntityCert; -use x509_parser::prelude::{FromDer, X509Certificate}; - -use crate::opamp::remote_config::{ - signature::{SignatureError, SigningAlgorithm}, - validators::signature::verifier::Verifier, -}; - -#[derive(Error, Debug)] -pub enum CertificateError { - #[error("parsing certificate from bytes: {0}")] - ParseCertificate(String), - #[error("verifying signature: {0}")] - VerifySignature(String), -} -#[derive(Debug, Clone)] -pub struct Certificate { - cert_der: Vec, - // sha256 digest of the public key - public_key_id: String, -} - -impl Verifier for Certificate { - type Error = CertificateError; - fn verify_signature( - &self, - algorithm: &SigningAlgorithm, - msg: &[u8], - signature: &[u8], - ) -> Result<(), CertificateError> { - let certificate = EndEntityCert::try_from(self.cert_der.as_slice()) - .map_err(|e| CertificateError::VerifySignature(e.to_string()))?; - - let signature_algorithm: &webpki::SignatureAlgorithm = algorithm - .try_into() - .map_err(|err: SignatureError| CertificateError::VerifySignature(err.to_string()))?; - - certificate - .verify_signature(signature_algorithm, msg, signature) - .map_err(|e| CertificateError::VerifySignature(e.to_string()))?; - debug!("signature verification succeeded"); - - Ok(()) - } - - fn key_id(&self) -> &str { - &self.public_key_id - } -} - -impl Certificate { - pub fn try_new(cert_der: Vec) -> Result { - let _ = EndEntityCert::try_from(cert_der.as_slice()) - .map_err(|e| CertificateError::ParseCertificate(e.to_string()))?; - - let (_, cer) = X509Certificate::from_der(&cert_der) - .map_err(|e| CertificateError::ParseCertificate(e.to_string()))?; - - Ok(Self { - public_key_id: public_key_fingerprint(cer.public_key().raw), - cert_der, - }) - } -} - -pub fn public_key_fingerprint(public_key: &[u8]) -> String { - let key_id_bytes = digest::digest(&digest::SHA256, public_key); - - // encode the digest as hex string - key_id_bytes - .as_ref() - .iter() - .fold(String::new(), |mut output, b| { - let _ = write!(output, "{b:02x}"); - output - }) -} - -#[cfg(test)] -mod tests { - use super::*; - use std::io::Cursor; - - #[test] - fn test_certificate_key_id() { - let mut cursor = Cursor::new(CERT_PEM.as_bytes()); - let cert = rustls_pemfile::certs(cursor.get_mut()) - .next() - .unwrap() - .unwrap(); - - let key_id = Certificate::try_new(cert.as_ref().to_vec()) - .unwrap() - .key_id() - .to_string(); - - assert_eq!(key_id, CERT_PUBLIC_KEY_ID); - } - - const CERT_PUBLIC_KEY_ID: &str = - "3c333a786b8f1e93f3a099a09cf591c5faac126ea48699e9e290e72b0b6bf06c"; - const CERT_PEM: &str = r#"-----BEGIN CERTIFICATE----- -MIIDLzCCAhegAwIBAgIUc5RF25ZGKeFSMlB8EK0EuDxZUFgwDQYJKoZIhvcNAQEL -BQAwHjELMAkGA1UEBhMCRkkxDzANBgNVBAMMBmNhbmFtZTAeFw0yNTAxMjAwNzU2 -NTBaFw0yNjAxMjAwNzU2NTBaMCAxCzAJBgNVBAYTAkZJMREwDwYDVQQDDAh0ZXN0 -bmFtZTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAL6pXsPX+0HpRdp+ -xD88Ut/SL26kmYSCaY9U1nCo45bARlTlhW62Bf5WMETJhGGi/Kq93MjPMkmNFNF/ -2qQx+XpxmKQR+B/iQzrg9bD1evRQPQvnSFBHKMh8cbqVpsLq/p6ee2iMoDpQ8C8p -Y1WjmGhcpp7EpDLUwx2x8NOu+uZp7NjT2rFBni7KMcWKJXEYh59EHkL/J/DeTUtQ -0Jxrq6k2hbEBOxRzO3XdwZ3w+LlurankJBOBljLpXn7Du9iA/0BicWczBhwJqv3T -96gyxoClmyGpXRiaiHyP+6t7/xfNfwJ6AEuifyVIUnxEyP+lgx6stWnV2j58a4kT -asRIASECAwEAAaNjMGEwHwYDVR0jBBgwFoAUwg0OUU2UnO8UnMGFAjUdIl2S5Jow -CQYDVR0TBAIwADAUBgNVHREEDTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFLoNRu6n -UepmUndgCwPr7tHQ84N0MA0GCSqGSIb3DQEBCwUAA4IBAQB4yKCYrdbz4FGxfA4K -GbgXe0ylio1OCA/4Db3Xo/UYJwKG+sG5YWKJiOTqJqdOPSczZE8ROA9BNLKpfUXj -hIffqUXca298j+8Ag+gFE5oOnUF1RUwE+xLWj94Fby4yFeadPcn1E7amSGoK1kE2 -ksQmmplpaVP9lOKnk6pX9NbMsAW2IeuDROuCYyTE9XOUxzdNnQp2Uk7rnxbGHHIl -ag5JWpNv/SRwijhGyVKiLFINYILDaNZc56RxxNWgfKj8mTiRvFV5OiM0MrIjBCUu -O0jhqIc+AEbSGU0jdfFxs4f9fJklHDphUxqE1MSvqzOMaFNrt/8jEupa2ujLCVId -XeFA ------END CERTIFICATE-----"#; -} diff --git a/agent-control/src/opamp/remote_config/validators/signature/certificate_fetcher.rs b/agent-control/src/opamp/remote_config/validators/signature/certificate_fetcher.rs deleted file mode 100644 index 4c56eae5f8..0000000000 --- a/agent-control/src/opamp/remote_config/validators/signature/certificate_fetcher.rs +++ /dev/null @@ -1,201 +0,0 @@ -use super::certificate::Certificate; -use crate::http::client::HttpClient; -use crate::opamp::remote_config::validators::signature::verifier::VerifierFetcher; -use reqwest::tls::TlsInfo; -use rustls::pki_types::CertificateDer; -use rustls::pki_types::pem::PemObject; -use std::path::PathBuf; -use thiserror::Error; -use tracing::log::error; -use url::Url; - -pub type DerCertificateBytes = Vec; - -#[derive(Error, Debug)] -pub enum CertificateFetcherError { - #[error("fetching certificate: {0}")] - CertificateFetch(String), -} -pub enum CertificateFetcher { - Https(Url, HttpClient), - PemFile(PathBuf), -} - -impl VerifierFetcher for CertificateFetcher { - type Error = CertificateFetcherError; - type Verifier = Certificate; - - fn fetch(&self) -> Result { - let cert = match self { - CertificateFetcher::Https(url, client) => CertificateFetcher::fetch_https(url, client)?, - CertificateFetcher::PemFile(pem_file_path) => { - CertificateFetcher::fetch_file(pem_file_path)? - } - }; - Certificate::try_new(cert) - .map_err(|e| CertificateFetcherError::CertificateFetch(e.to_string())) - } -} - -impl CertificateFetcher { - fn fetch_https( - url: &Url, - client: &HttpClient, - ) -> Result { - let request = http::Request::builder() - .uri(url.as_ref()) - .method("HEAD") - .body(Vec::default()) - .map_err(|err| { - CertificateFetcherError::CertificateFetch(format!("error building request: {err}")) - })?; - let response = client.send(request).map_err(|e| { - CertificateFetcherError::CertificateFetch(format!("fetching certificate: {e}")) - })?; - let tls_info = response.extensions().get::().ok_or( - CertificateFetcherError::CertificateFetch("missing tls information".to_string()), - )?; - - let leaf_cert_der = - tls_info - .peer_certificate() - .ok_or(CertificateFetcherError::CertificateFetch( - "missing leaf certificates".into(), - ))?; - - Ok(leaf_cert_der.to_vec()) - } - - fn fetch_file(pem_file_path: &PathBuf) -> Result { - let cert = CertificateDer::from_pem_file(pem_file_path).map_err(|e| { - CertificateFetcherError::CertificateFetch(format!("reading certificate from file: {e}")) - })?; - Ok(cert.as_ref().to_vec()) - } -} - -#[cfg(test)] -mod tests { - use std::time::Duration; - - use super::*; - use crate::http::config::HttpConfig; - use crate::http::config::ProxyConfig; - use crate::http::tls::install_rustls_default_crypto_provider; - use crate::opamp::remote_config::validators::signature::validator::tests::TestCertificateSigner; - use crate::utils::retry::retry; - use assert_matches::assert_matches; - - const DEFAULT_CLIENT_TIMEOUT: Duration = Duration::from_secs(10); - - #[test] - fn test_https_fetcher() { - install_rustls_default_crypto_provider(); - - struct TestCase { - name: &'static str, - url: &'static str, - } - impl TestCase { - fn run(self) { - let http_config = HttpConfig::new( - DEFAULT_CLIENT_TIMEOUT, - DEFAULT_CLIENT_TIMEOUT, - ProxyConfig::default(), - ) - .with_tls_info(); - let client = HttpClient::new(http_config).unwrap(); - - // We have seen issues connecting to badssl.com from CI making this test flaky. - retry(10, Duration::from_secs(1), || { - if let Err(e) = - CertificateFetcher::Https(Url::parse(self.url).unwrap(), client.clone()) - .fetch() - { - return Err(format!("fetching cert err '{}', case: '{}'", e, self.name)); - } - - Ok(()) - }) - .unwrap(); - } - } - let test_cases = vec![ - TestCase { - name: "rsa sha256", - url: "https://sha256.badssl.com/", - }, - TestCase { - name: "ecc sha256", - url: "https://ecc256.badssl.com/", - }, - ]; - - for test_case in test_cases { - test_case.run(); - } - } - #[test] - fn test_https_fetcher_fails() { - install_rustls_default_crypto_provider(); - - struct TestCase { - name: &'static str, - url: &'static str, - } - impl TestCase { - fn run(self) { - let http_config = HttpConfig::new( - DEFAULT_CLIENT_TIMEOUT, - DEFAULT_CLIENT_TIMEOUT, - ProxyConfig::default(), - ) - .with_tls_info(); - let client = HttpClient::new(http_config).unwrap(); - let err = CertificateFetcher::Https(Url::parse(self.url).unwrap(), client) - .fetch() - .expect_err(format!("error is expected, case: {}", self.name).as_str()); - - assert_matches!(err, CertificateFetcherError::CertificateFetch(_)); - } - } - let test_cases = vec![ - TestCase { - name: "missing endpoint", - url: "https://badssl.com:9999/", - }, - TestCase { - name: "http", - url: "http://http.badssl.com/", - }, - TestCase { - name: "expired certificate", - url: "https://expired.badssl.com/", - }, - TestCase { - name: "wrong host", - url: "https://wrong.host.badssl.com/", - }, - TestCase { - name: "untrusted root", - url: "https://untrusted-root.badssl.com/", - }, - TestCase { - name: "self signed", - url: "https://self-signed.badssl.com/", - }, - ]; - - for test_case in test_cases { - test_case.run(); - } - } - - #[test] - fn test_file_fetcher() { - let test_signer = TestCertificateSigner::new(); - CertificateFetcher::PemFile(test_signer.cert_pem_path()) - .fetch() - .expect("to fetch certificate"); - } -} diff --git a/agent-control/src/opamp/remote_config/validators/signature/test_data/self-singed-cert-server.sh b/agent-control/src/opamp/remote_config/validators/signature/test_data/self-singed-cert-server.sh deleted file mode 100755 index 0ee6e0ed54..0000000000 --- a/agent-control/src/opamp/remote_config/validators/signature/test_data/self-singed-cert-server.sh +++ /dev/null @@ -1,72 +0,0 @@ -#!/bin/sh - -# This script spawn a docker nginx server with a self-signed certificate -# and create a configuration file and sign it with the certificate private key -# for testing proposes. - -mkdir -p self-cert-server && cd self-cert-server - -# source: https://users.rust-lang.org/t/use-tokio-tungstenite-with-rustls-instead-of-native-tls-for-secure-websockets/90130 -# Create unencrypted private key and a CSR (certificate signing request) -openssl req -newkey rsa:2048 -nodes -subj "/C=FI/CN=testname" -keyout server.key -out key.csr - -# Create self-signed certificate (`server.crt`) with the private key and CSR -openssl x509 -signkey server.key -in key.csr -req -days 365 -out server.crt - -# Create a self-signed root CA -openssl req -x509 -sha256 -nodes -subj "/C=FI/CN=caname" -days 1825 -newkey rsa:2048 -keyout ca.key -out ca.crt - -# Create file localhost.ext with the following content: -cat <<'EOF' >> localhost.ext -authorityKeyIdentifier=keyid,issuer -basicConstraints=CA:FALSE -subjectAltName = @testname -[testname] -DNS.1 = localhost -EOF - -# Sign the CSR (`server.crt`) with the root CA certificate and private key -# => this overwrites `server.crt` because it gets signed -openssl x509 -req -CA ca.crt -CAkey ca.key -in key.csr -out server.crt -days 365 -CAcreateserial -extfile localhost.ext - -# Create nginx configuration file to start a tls server using the generated certificates -cat <<'EOF' >> nginx.conf -events { - # You can leave this block empty or configure worker connections - worker_connections 1024; -} - -http { - server { - listen 443 default_server ssl; - server_name test; - - ssl_certificate /etc/nginx/certs/server.crt; - ssl_certificate_key /etc/nginx/certs/server.key; - - location / { - root /usr/share/nginx/html; - index index.html; - } - } -} -EOF - -# Create a dummy configuration file and sign it with the private key and verify it just for -# testing proposes -cat <<'EOF' > config.yaml -chart_version: 1.10.12 -chart_values: - podLabels: "192.168.5.0" -EOF -openssl dgst -sha256 -sign server.key -out signature.sha256 config.yaml -openssl base64 -in signature.sha256 -out signature.base64 - -openssl x509 -inform pem -in server.crt -pubkey -noout > server.pub -openssl dgst -sha256 -verify server.pub -signature signature.sha256 config.yaml - -# Run nginx server with the generated certificates -docker run --rm -p 443:443 \ - -v ./:/etc/nginx/certs:ro \ - -v ./nginx.conf:/etc/nginx/nginx.conf:ro \ - nginx diff --git a/agent-control/src/opamp/remote_config/validators/signature/test_data/verify.sh b/agent-control/src/opamp/remote_config/validators/signature/test_data/verify.sh deleted file mode 100755 index 34430c2f78..0000000000 --- a/agent-control/src/opamp/remote_config/validators/signature/test_data/verify.sh +++ /dev/null @@ -1,49 +0,0 @@ -#!/bin/bash -# This script helps to manually verify the signature of a Remote config for testing proposes. -# example: -# ServerToAgent: { -# remote_config:{ -# config: { -# config_map: { -# "agentConfig": { -# body: "chart_version: 1.10.12\nchart_values:\n podLabels: \"192.168.5.0\"" -# content_type: "" -# } -# } -# } -# config_hash: "817982697f614312018935c351fdd71aca190f106fda2d7bc69da86e878ea5e4" -# } -# custom_message:{ -# capability: "com.newrelic.security.configSignature" -# type: "newrelicRemoteConfigSignature" -# data: { -# "3936250589": [{ -# "checksum": "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", -# "checksumAlgorithm": "SHA256", -# "signature": "nppw2CuZg+YO5MsEoNOsHlgHxF7qAwWPli37NGXAr5isfP1jUTSJcLi0l7k9lNlpbq31GF9DZ0JQBZhoGS0j+sDjvirKSb7yXdqj6JcZ8sxax7KWAnk5QPiwLHFA1kGmszVJ/ccbwtVozG46FvKedcc3X5RME/HGdJupKBe3UzmJawL0xs9jNY+9519CL+CpbkBl/WgCvrIUhTNZv5TUHK23hMD+kz1Brf60pW7MQVtsyClOllsb6WhAsSXdhkpSCJ+96ZGyYywUlvx3/vkBM5a7q4IWqiPM4U0LPZDMQJQCCpxWV3T7cnIR1Ye2yYUqJHs9vfKmTWeBKH2Tb5FgpQ==", -# "signingAlgorithm": "RSA_PKCS1_2048_SHA256", -# "signatureSpecification": "PKCS #1 v2.2", -# "signingDomain": "iast-csec-se.test-poised-pear.cell.us.nr-data.net", -# "keyId": "778b223984d389ad6555bdbbbf118420290c53296b6511e1964309965ec5f710" -# }] -# } -# } -# } - - -mkdir -p verify && cd verify - -cert_url="iast-csec-se.test-poised-pear.cell.us.nr-data.net:443" - -signature_base64="nppw2CuZg+YO5MsEoNOsHlgHxF7qAwWPli37NGXAr5isfP1jUTSJcLi0l7k9lNlpbq31GF9DZ0JQBZhoGS0j+sDjvirKSb7yXdqj6JcZ8sxax7KWAnk5QPiwLHFA1kGmszVJ/ccbwtVozG46FvKedcc3X5RME/HGdJupKBe3UzmJawL0xs9jNY+9519CL+CpbkBl/WgCvrIUhTNZv5TUHK23hMD+kz1Brf60pW7MQVtsyClOllsb6WhAsSXdhkpSCJ+96ZGyYywUlvx3/vkBM5a7q4IWqiPM4U0LPZDMQJQCCpxWV3T7cnIR1Ye2yYUqJHs9vfKmTWeBKH2Tb5FgpQ==" - -openssl s_client -connect $cert_url /dev/null | openssl x509 -inform pem -text | sed -n '/-----BEGIN CERTIFICATE-----/,/-----END CERTIFICATE-----/p' > public_cert.pem -openssl x509 -inform pem -in public_cert.pem -pubkey -noout > certificate_publickey.pem - -echo $signature_base64 > signature.base64 -openssl base64 -d -in signature.base64 -out signature.sha256 - -echo -en "chart_version: 1.10.12\nchart_values:\n podLabels: \"192.168.5.0\"" > msg - -openssl dgst -sha256 -verify certificate_publickey.pem -signature signature.sha256 msg - diff --git a/agent-control/src/opamp/remote_config/validators/signature/validator.rs b/agent-control/src/opamp/remote_config/validators/signature/validator.rs index 263bf293db..2e276c8f3c 100644 --- a/agent-control/src/opamp/remote_config/validators/signature/validator.rs +++ b/agent-control/src/opamp/remote_config/validators/signature/validator.rs @@ -1,4 +1,3 @@ -use super::certificate_fetcher::CertificateFetcher; use crate::agent_control::defaults::get_custom_capabilities; use crate::http::client::HttpClient; use crate::http::config::HttpConfig; @@ -6,194 +5,113 @@ use crate::http::config::ProxyConfig; use crate::opamp::remote_config::OpampRemoteConfig; use crate::opamp::remote_config::signature::SIGNATURE_CUSTOM_CAPABILITY; use crate::opamp::remote_config::validators::RemoteConfigValidator; -use crate::opamp::remote_config::validators::signature::certificate::Certificate; 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 serde::Deserialize; -use std::path::PathBuf; use std::time::Duration; use thiserror::Error; -use tracing::debug; +use tracing::info; use tracing::log::error; -use tracing::{info, warn}; +use tracing::warn; use url::Url; -const DEFAULT_CERTIFICATE_SERVER_URL: &str = "https://newrelic.com/"; const DEFAULT_HTTPS_CLIENT_TIMEOUT: Duration = Duration::from_secs(30); const DEFAULT_SIGNATURE_VALIDATOR_ENABLED: bool = true; type ErrorMessage = String; #[derive(Error, Debug)] pub enum SignatureValidatorError { - #[error("failed to fetch certificate: {0}")] - FetchCertificate(ErrorMessage), #[error("failed to build validator: {0}")] BuildingValidator(ErrorMessage), #[error("failed to verify signature: {0}")] VerifySignature(ErrorMessage), } -/// Returns a SignatureValidator wrapping a CertificateSignatureValidator if fleet_control and signature validation are -/// enabled and a no-op validator otherwise. -/// -/// Proxies configuration that intercept TLS traffic are not supported since the fetcher expects to connect directly to the server. -pub fn build_signature_validator( - config: SignatureValidatorConfig, - proxy_config: ProxyConfig, -) -> Result { - if !config.enabled { - warn!("Remote config signature validation is disabled"); - return Ok(SignatureValidator::Noop); - } - - // Certificate from file takes precedence over fetching from the server when it is set - let certificate_fetcher = if !config.certificate_pem_file_path.as_os_str().is_empty() { - warn!( - "Remote config signature validation is enabled, using certificate from file: {}. Certificate rotation is not supported", - config.certificate_pem_file_path.display() - ); - CertificateFetcher::PemFile(config.certificate_pem_file_path) - } else { - info!( - "Remote config signature validation is enabled (certificate), fetching certificate from: {}", - config.certificate_server_url - ); - - let http_config = HttpConfig::new( - DEFAULT_HTTPS_CLIENT_TIMEOUT, - DEFAULT_HTTPS_CLIENT_TIMEOUT, - proxy_config.clone(), - ) - .with_tls_info(); - - let client = HttpClient::new(http_config) - .map_err(|e| SignatureValidatorError::BuildingValidator(e.to_string()))?; - - CertificateFetcher::Https(config.certificate_server_url, client) - }; - - let cert_verifier_store = VerifierStore::try_new(certificate_fetcher) - .map_err(|err| SignatureValidatorError::BuildingValidator(err.to_string()))?; - - let maybe_pubkey_verifier_store = - if let Some(public_key_server_url) = config.public_key_server_url { - let http_config = HttpConfig::new( - DEFAULT_HTTPS_CLIENT_TIMEOUT, - DEFAULT_HTTPS_CLIENT_TIMEOUT, - proxy_config, - ); - let http_client = HttpClient::new(http_config) - .map_err(|e| SignatureValidatorError::BuildingValidator(e.to_string()))?; - - info!( - "Remote config signature validation is (public key), fetching jwks from: {}", - public_key_server_url - ); - let public_key_fetcher = PublicKeyFetcher::new(http_client, public_key_server_url); - - let pubkey_verifier_store = VerifierStore::try_new(public_key_fetcher) - .map_err(|err| SignatureValidatorError::BuildingValidator(err.to_string()))?; - - Some(pubkey_verifier_store) - } else { - None - }; - - Ok(SignatureValidator::Composite( - CompositeSignatureValidator::new(cert_verifier_store, maybe_pubkey_verifier_store), - )) -} - #[derive(Debug, Deserialize, PartialEq, Clone)] pub struct SignatureValidatorConfig { - #[serde(default = "default_certificate_server_url")] - pub certificate_server_url: Url, - #[serde(default)] - pub public_key_server_url: Option, - /// Path to the PEM file containing the certificate to validate the signature. - /// Takes precedence over fetching from the server when it is set - #[serde(default)] - pub certificate_pem_file_path: PathBuf, #[serde(default = "default_signature_validator_config_enabled")] pub enabled: bool, + #[serde(default)] + pub public_key_server_url: Option, } - impl Default for SignatureValidatorConfig { fn default() -> Self { Self { - enabled: default_signature_validator_config_enabled(), - certificate_server_url: default_certificate_server_url(), + enabled: DEFAULT_SIGNATURE_VALIDATOR_ENABLED, public_key_server_url: None, - certificate_pem_file_path: PathBuf::new(), } } } -fn default_certificate_server_url() -> Url { - Url::parse(DEFAULT_CERTIFICATE_SERVER_URL).unwrap_or_else(|err| { - panic!("Invalid DEFAULT_CERTIFICATE_SERVER_URL: '{DEFAULT_CERTIFICATE_SERVER_URL}': {err}") - }) -} - fn default_signature_validator_config_enabled() -> bool { DEFAULT_SIGNATURE_VALIDATOR_ENABLED } -// NOTE: if we updated the components using the validator to use a composite-like implementation to handle validation, -// the no-op validator wouldn't be necessary. -/// The SignatureValidator enum wraps [CertificateSignatureValidator] and adds support for No-op validator. -#[allow(clippy::large_enum_variant)] -pub enum SignatureValidator { - Composite(CompositeSignatureValidator), - Noop, +pub struct SignatureValidator { + public_key_store: Option>, } -impl RemoteConfigValidator for SignatureValidator { - type Err = SignatureValidatorError; - - fn validate( - &self, - agent_identity: &AgentIdentity, - opamp_remote_config: &OpampRemoteConfig, - ) -> Result<(), Self::Err> { - match self { - SignatureValidator::Composite(v) => v.validate(agent_identity, opamp_remote_config), - SignatureValidator::Noop => Ok(()), +impl SignatureValidator { + pub fn new( + config: SignatureValidatorConfig, + proxy_config: ProxyConfig, + ) -> Result { + if !config.enabled { + warn!("Remote config signature validation is disabled"); + return Ok(Self::new_noop()); } - } -} -/// Temporal signature validator that uses both certificate and public key validation in order -/// to support backward compatibility with existing signatures. -/// Once the migration to the new signature platform is complete, the certificate validation -/// can be removed. -pub struct CompositeSignatureValidator { - certificate_store: VerifierStore, - public_key_store: Option>, -} + let Some(public_key_server_url) = config.public_key_server_url else { + return Err(SignatureValidatorError::BuildingValidator( + "missing public_key_server_url configuration".to_string(), + )); + }; -impl CompositeSignatureValidator { - pub fn new( - certificate_store: VerifierStore, - public_key_store: Option>, - ) -> Self { + info!( + "Remote config signature validation is enabled, fetching jwks from: {}", + public_key_server_url + ); + + let http_config = HttpConfig::new( + DEFAULT_HTTPS_CLIENT_TIMEOUT, + DEFAULT_HTTPS_CLIENT_TIMEOUT, + proxy_config, + ); + let http_client = HttpClient::new(http_config) + .map_err(|e| SignatureValidatorError::BuildingValidator(e.to_string()))?; + + let public_key_fetcher = PublicKeyFetcher::new(http_client, public_key_server_url); + + let pubkey_verifier_store = VerifierStore::try_new(public_key_fetcher) + .map_err(|err| SignatureValidatorError::BuildingValidator(err.to_string()))?; + + Ok(Self { + public_key_store: Some(pubkey_verifier_store), + }) + } + + pub fn new_noop() -> Self { Self { - certificate_store, - public_key_store, + public_key_store: None, } } } -impl RemoteConfigValidator for CompositeSignatureValidator { +impl RemoteConfigValidator for SignatureValidator { type Err = SignatureValidatorError; fn validate( &self, agent_identity: &AgentIdentity, opamp_remote_config: &OpampRemoteConfig, - ) -> Result<(), SignatureValidatorError> { + ) -> Result<(), Self::Err> { + // Noop validation + let Some(public_key_store) = &self.public_key_store else { + return Ok(()); + }; + // custom capabilities are got from the agent-type (currently hard-coded) // If the capability is not set, no validation is performed if !get_custom_capabilities(&agent_identity.agent_type_id).is_some_and(|c| { @@ -215,30 +133,7 @@ impl RemoteConfigValidator for CompositeSignatureValidator { .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. - // This fallback mechanism makes errors misleading in case the platform is migrated and the validation fails - // since the showed error is from the cert validation. - if let Some(public_key_store) = &self.public_key_store { - match public_key_store.verify_signature( - signature.signature_algorithm(), - signature.key_id(), - config_content, - signature.signature(), - ) { - Ok(()) => return Ok(()), - Err(err) => { - debug!( - "Failed to verify signature using the public key in the configured JWKS: {}", - err - ); - } - } - } - - debug!("Falling back to signature verification using the configured Certificate"); - - self.certificate_store + public_key_store .verify_signature( signature.signature_algorithm(), signature.key_id(), @@ -253,260 +148,48 @@ impl RemoteConfigValidator for CompositeSignatureValidator { pub mod tests { use super::*; use crate::agent_control::agent_id::AgentID; - use crate::http::tls::install_rustls_default_crypto_provider; use crate::opamp::remote_config::hash::{ConfigState, Hash}; - use crate::opamp::remote_config::signature::{ - ECDSA_P256_SHA256, ED25519, SignatureData, Signatures, SigningAlgorithm, - }; + use crate::opamp::remote_config::signature::{ED25519, SignatureData, Signatures}; use crate::opamp::remote_config::validators::signature::public_key_fetcher::tests::FakePubKeyServer; - use crate::opamp::remote_config::validators::signature::verifier::{ - Verifier, VerifierStoreError, - }; use crate::opamp::remote_config::{ConfigurationMap, DEFAULT_AGENT_CONFIG_IDENTIFIER}; use crate::sub_agent::identity::AgentIdentity; use assert_matches::assert_matches; - use base64::Engine; - use base64::prelude::BASE64_STANDARD; - use rcgen::{CertificateParams, PKCS_ED25519}; use std::collections::HashMap; - use std::str::FromStr; - use tempfile::TempDir; - - // A test signer util that generates a key pair and a self-signed certificate, and can be used to sign messages, - // as the OpAmp server would do. - // The certificate is written to a temporary file which is cleaned up when the signer is dropped. - pub struct TestCertificateSigner { - key_pair: rcgen::KeyPair, - cert_temp_dir: TempDir, - cert: rcgen::Certificate, - key_id: String, - } - impl TestCertificateSigner { - const CERT_FILE_NAME: &'static str = "test.pem"; - pub fn new() -> Self { - let key_pair = rcgen::KeyPair::generate_for(&PKCS_ED25519).unwrap(); - let cert = CertificateParams::new(vec!["localhost".to_string()]) - .unwrap() - .self_signed(&key_pair) - .unwrap(); - - let key_id = Certificate::try_new(cert.der().as_ref().to_vec()) - .unwrap() - .key_id() - .to_string(); - - let cert_temp_dir = tempfile::tempdir().unwrap(); - std::fs::write(cert_temp_dir.path().join(Self::CERT_FILE_NAME), cert.pem()).unwrap(); - - Self { - key_pair, - key_id, - cert, - cert_temp_dir, - } - } - - pub fn cert_pem_path(&self) -> PathBuf { - self.cert_temp_dir.path().join(Self::CERT_FILE_NAME) - } - - pub fn key_id(&self) -> &str { - &self.key_id - } - - pub fn cert_pem(&self) -> String { - self.cert.pem() - } - - /// Sign a message and encode the signature in standard base64 encoding. - pub fn encoded_signature(&self, msg: &str) -> String { - let key_pair_ring = - ring::signature::Ed25519KeyPair::from_pkcs8(&self.key_pair.serialize_der()) - .unwrap(); - let signature = key_pair_ring.sign(msg.as_bytes()); - BASE64_STANDARD.encode(signature.as_ref()) - } - } - - impl Default for TestCertificateSigner { - fn default() -> Self { - Self::new() - } - } - - #[test] - fn test_certificate_verify_sucess() { - install_rustls_default_crypto_provider(); - let test_signer = TestCertificateSigner::new(); - let config = "fake_config"; - - let cert_store = - VerifierStore::try_new(CertificateFetcher::PemFile(test_signer.cert_pem_path())) - .unwrap(); - - cert_store - .verify_signature( - &SigningAlgorithm::ED25519, - test_signer.key_id(), - config.as_bytes(), - test_signer.encoded_signature(config).as_bytes(), - ) - .unwrap(); - } - #[test] - fn test_certificate_signature_content_missmatch() { - install_rustls_default_crypto_provider(); - let test_signer = TestCertificateSigner::new(); - - let cert_store = - VerifierStore::try_new(CertificateFetcher::PemFile(test_signer.cert_pem_path())) - .unwrap(); - - let err = cert_store - .verify_signature( - &SigningAlgorithm::ED25519, - test_signer.key_id(), - b"some config", - test_signer - .encoded_signature("some other config") - .as_bytes(), - ) - .unwrap_err(); - - assert_matches!(err, VerifierStoreError::VerifySignature(_)); - } - - #[test] - fn test_certificate_signature_algorithm_missmatch() { - install_rustls_default_crypto_provider(); - let test_signer = TestCertificateSigner::new(); - let config = "fake_config"; - - let cert_store = - VerifierStore::try_new(CertificateFetcher::PemFile(test_signer.cert_pem_path())) - .unwrap(); - - let err = cert_store - .verify_signature( - &SigningAlgorithm::RSA_PKCS1_2048_8192_SHA512, - test_signer.key_id(), - config.as_bytes(), - test_signer.encoded_signature(config).as_bytes(), - ) - .unwrap_err(); - - assert_matches!(err, VerifierStoreError::VerifySignature(_)); - } #[test] - fn test_default_signature_validator_config() { - let config = SignatureValidatorConfig::default(); - assert_eq!( - config.certificate_server_url.to_string(), - DEFAULT_CERTIFICATE_SERVER_URL - ); - assert_eq!(config.enabled, DEFAULT_SIGNATURE_VALIDATOR_ENABLED) - } + pub fn test_valid_signature() { + let pub_key_server = FakePubKeyServer::new(); - #[test] - fn test_signature_validator_config() { - struct TestCase { - name: &'static str, - cfg: &'static str, - expected: SignatureValidatorConfig, - } + let signature_validator = SignatureValidator::new( + SignatureValidatorConfig { + public_key_server_url: Some(pub_key_server.url.clone()), + ..Default::default() + }, + ProxyConfig::default(), + ) + .unwrap(); - impl TestCase { - fn run(self) { - let config: SignatureValidatorConfig = serde_yaml::from_str(self.cfg) - .unwrap_or_else(|err| { - panic!("{} - Invalid config '{}': {}", self.name, self.cfg, err) - }); - assert_eq!(config, self.expected, "{} failed", self.name); - } - } + let config = "value"; - let test_cases = [ - TestCase { - name: "Setup enabled only (false)", - cfg: r#" - enabled: false - "#, - expected: SignatureValidatorConfig { - enabled: false, - certificate_server_url: Url::parse(DEFAULT_CERTIFICATE_SERVER_URL).unwrap(), - public_key_server_url: None, - certificate_pem_file_path: PathBuf::new(), - }, - }, - TestCase { - name: "Setup enabled only (true)", - cfg: r#" - enabled: true - "#, - expected: SignatureValidatorConfig { - enabled: true, - certificate_server_url: Url::parse(DEFAULT_CERTIFICATE_SERVER_URL).unwrap(), - public_key_server_url: None, - certificate_pem_file_path: PathBuf::new(), - }, - }, - TestCase { - name: "Setup url only", - cfg: r#" - certificate_server_url: https://example.com - "#, - expected: SignatureValidatorConfig { - enabled: DEFAULT_SIGNATURE_VALIDATOR_ENABLED, - certificate_server_url: Url::parse("https://example.com").unwrap(), - public_key_server_url: None, - certificate_pem_file_path: PathBuf::new(), - }, - }, - TestCase { - name: "Setup url and enabled", - cfg: r#" - enabled: true - certificate_server_url: https://example.com - "#, - expected: SignatureValidatorConfig { - enabled: true, - certificate_server_url: Url::parse("https://example.com").unwrap(), - public_key_server_url: None, - certificate_pem_file_path: PathBuf::new(), - }, - }, - TestCase { - name: "Setup file and enabled", - cfg: r#" - enabled: true - certificate_pem_file_path: /path/to/file - "#, - expected: SignatureValidatorConfig { - enabled: true, - certificate_server_url: Url::parse(DEFAULT_CERTIFICATE_SERVER_URL).unwrap(), - public_key_server_url: None, - certificate_pem_file_path: PathBuf::from_str("/path/to/file").unwrap(), - }, - }, - TestCase { - name: "Setup file and url and enabled", - cfg: r#" - enabled: true - certificate_server_url: https://example.com - public_key_server_url: https://test.io - certificate_pem_file_path: /path/to/file - "#, - expected: SignatureValidatorConfig { - enabled: true, - certificate_server_url: Url::parse("https://example.com").unwrap(), - public_key_server_url: Some(Url::parse("https://test.io").unwrap()), - certificate_pem_file_path: PathBuf::from_str("/path/to/file").unwrap(), - }, - }, - ]; + let encoded_signature = pub_key_server.sign(config.as_bytes()); + let remote_config = OpampRemoteConfig::new( + AgentID::AgentControl, + Hash::from("test"), + ConfigState::Applying, + ConfigurationMap::new(HashMap::from([( + DEFAULT_AGENT_CONFIG_IDENTIFIER.to_string(), + config.to_string(), + )])), + ) + .with_signature(Signatures::new_default( + encoded_signature.as_str(), + ED25519, + pub_key_server.key_id.as_str(), + )); - test_cases.into_iter().for_each(|tc| tc.run()); + signature_validator + .validate(&AgentIdentity::default(), &remote_config) + .unwrap() } #[test] @@ -518,7 +201,7 @@ pub mod tests { ConfigurationMap::default(), ); - let noop_validator = SignatureValidator::Noop; + let noop_validator = SignatureValidator::new_noop(); assert!( noop_validator @@ -529,7 +212,7 @@ pub mod tests { } #[test] - pub fn test_certificate_signature_validator_err() { + pub fn test_signature_validator_errors() { struct TestCase { name: &'static str, remote_config: OpampRemoteConfig, @@ -537,22 +220,16 @@ pub mod tests { impl TestCase { fn run(self) { - let test_signer = TestCertificateSigner::new(); let pub_key_server = FakePubKeyServer::new(); - let signature_validator = CompositeSignatureValidator::new( - VerifierStore::try_new(CertificateFetcher::PemFile( - test_signer.cert_pem_path(), - )) - .unwrap(), - Some( - VerifierStore::try_new(PublicKeyFetcher::new( - HttpClient::new(HttpConfig::default()).unwrap(), - pub_key_server.url, - )) - .unwrap(), - ), - ); + let signature_validator = SignatureValidator::new( + SignatureValidatorConfig { + public_key_server_url: Some(pub_key_server.url.clone()), + ..Default::default() + }, + ProxyConfig::default(), + ) + .unwrap(); let result = signature_validator.validate(&AgentIdentity::default(), &self.remote_config); @@ -615,7 +292,7 @@ pub mod tests { ) .with_signature(Signatures::new_default( "invalid signature", - ECDSA_P256_SHA256, + ED25519, "fake_key_id", )), }, @@ -625,21 +302,18 @@ pub mod tests { } #[test] - pub fn test_certificate_signature_validator_signature_is_missing_for_agent_control_agent() { - let test_signer = TestCertificateSigner::new(); + pub fn test_signature_is_missing_for_agent_control_agent() { let pub_key_server = FakePubKeyServer::new(); - let signature_validator = CompositeSignatureValidator::new( - VerifierStore::try_new(CertificateFetcher::PemFile(test_signer.cert_pem_path())) - .unwrap(), - Some( - VerifierStore::try_new(PublicKeyFetcher::new( - HttpClient::new(HttpConfig::default()).unwrap(), - pub_key_server.url, - )) - .unwrap(), - ), - ); + let signature_validator = SignatureValidator::new( + SignatureValidatorConfig { + public_key_server_url: Some(pub_key_server.url.clone()), + ..Default::default() + }, + ProxyConfig::default(), + ) + .unwrap(); + let rc = OpampRemoteConfig::new( AgentID::AgentControl, Hash::from("test"), @@ -653,83 +327,4 @@ pub mod tests { .is_ok() ); } - - #[test] - pub fn test_certificate_signature_validator_fallback() { - let test_signer = TestCertificateSigner::new(); - let pub_key_server = FakePubKeyServer::new(); - - let signature_validator = CompositeSignatureValidator::new( - VerifierStore::try_new(CertificateFetcher::PemFile(test_signer.cert_pem_path())) - .unwrap(), - Some( - VerifierStore::try_new(PublicKeyFetcher::new( - HttpClient::new(HttpConfig::default()).unwrap(), - pub_key_server.url, - )) - .unwrap(), - ), - ); - - let config = "value"; - - let encoded_signature = test_signer.encoded_signature(config); - let remote_config = OpampRemoteConfig::new( - AgentID::AgentControl, - Hash::from("test"), - ConfigState::Applying, - ConfigurationMap::new(HashMap::from([( - DEFAULT_AGENT_CONFIG_IDENTIFIER.to_string(), - config.to_string(), - )])), - ) - .with_signature(Signatures::new_default( - encoded_signature.as_str(), - ED25519, // Test signer uses this algorithm - test_signer.key_id(), - )); - - signature_validator - .validate(&AgentIdentity::default(), &remote_config) - .unwrap() - } - #[test] - pub fn test_publickey_signature_validator_signature_is_valid() { - let test_signer = TestCertificateSigner::new(); - let pub_key_server = FakePubKeyServer::new(); - - let signature_validator = CompositeSignatureValidator::new( - VerifierStore::try_new(CertificateFetcher::PemFile(test_signer.cert_pem_path())) - .unwrap(), - Some( - VerifierStore::try_new(PublicKeyFetcher::new( - HttpClient::new(HttpConfig::default()).unwrap(), - pub_key_server.url.clone(), - )) - .unwrap(), - ), - ); - - let config = "value"; - - let encoded_signature = pub_key_server.sign(config.as_bytes()); - let remote_config = OpampRemoteConfig::new( - AgentID::AgentControl, - Hash::from("test"), - ConfigState::Applying, - ConfigurationMap::new(HashMap::from([( - DEFAULT_AGENT_CONFIG_IDENTIFIER.to_string(), - config.to_string(), - )])), - ) - .with_signature(Signatures::new_default( - encoded_signature.as_str(), - ED25519, - pub_key_server.key_id.as_str(), - )); - - signature_validator - .validate(&AgentIdentity::default(), &remote_config) - .unwrap() - } } diff --git a/agent-control/src/opamp/remote_config/validators/signature/verifier.rs b/agent-control/src/opamp/remote_config/validators/signature/verifier.rs index ce1add4bbd..d5baf4ff82 100644 --- a/agent-control/src/opamp/remote_config/validators/signature/verifier.rs +++ b/agent-control/src/opamp/remote_config/validators/signature/verifier.rs @@ -31,11 +31,11 @@ pub enum VerifierStoreError { #[error("fetching verifying key: {0}")] Fetch(String), #[error( - "signature key ID ({signature_key_id}) does not match the latest available key ID ({certificate_key_id})" + "signature key ID ({signature_key_id}) does not match the latest available key ID ({stored_key_id})" )] KeyMismatch { signature_key_id: String, - certificate_key_id: String, + stored_key_id: String, }, #[error("validating signature: {0}")] VerifySignature(String), @@ -97,7 +97,7 @@ where if !verifier.key_id().eq_ignore_ascii_case(key_id) { return Err(VerifierStoreError::KeyMismatch { signature_key_id: key_id.to_string(), - certificate_key_id: verifier.key_id().to_string(), + stored_key_id: verifier.key_id().to_string(), }); } } diff --git a/agent-control/tests/common/opamp.rs b/agent-control/tests/common/opamp.rs index 3a25c8e2d2..1cf1bfc55f 100644 --- a/agent-control/tests/common/opamp.rs +++ b/agent-control/tests/common/opamp.rs @@ -7,7 +7,6 @@ use newrelic_agent_control::opamp::remote_config::DEFAULT_AGENT_CONFIG_IDENTIFIE use newrelic_agent_control::opamp::remote_config::signature::{ ED25519, SIGNATURE_CUSTOM_CAPABILITY, SIGNATURE_CUSTOM_MESSAGE_TYPE, SignatureFields, }; -use newrelic_agent_control::opamp::remote_config::validators::signature::public_key_fingerprint; use opamp_client::opamp::proto::{ AgentConfigFile, AgentConfigMap, AgentDescription, AgentRemoteConfig, AgentToServer, ComponentHealth, CustomMessage, EffectiveConfig, RemoteConfigStatus, ServerToAgent, @@ -15,30 +14,24 @@ 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; use std::hash::{DefaultHasher, Hash, Hasher}; -use std::path::PathBuf; use std::sync::Mutex; use std::{collections::HashMap, net, sync::Arc}; -use tempfile::TempDir; use tokio::task::JoinHandle; const FAKE_SERVER_PATH: &str = "/opamp-fake-server"; const JWKS_SERVER_PATH: &str = "/jwks"; -const CERT_FILE: &str = "server.crt"; +const JWKS_PUBLIC_KEY_ID: &str = "fakeKeyName/0"; /// Represents the state of the FakeServer. struct ServerState { agent_state: HashMap, // Key pair to sign remote configuration key_pair: Ed25519KeyPair, - // Use the legacy system (instead of key_pair) to sign remote configuration - use_legacy_signatures: bool, - legacy_key_pair: KeyPair, // TODO: cleanup when no longer used } #[derive(Default)] @@ -52,12 +45,10 @@ struct AgentState { } impl ServerState { - fn new(cert_key_pair: KeyPair, use_legacy_signatures: bool) -> Self { + fn generate() -> Self { Self { agent_state: HashMap::new(), key_pair: generate_key_pair(), - use_legacy_signatures, - legacy_key_pair: cert_key_pair, } } } @@ -87,7 +78,6 @@ pub struct FakeServer { state: Arc>, port: u16, path: String, - cert_tmp_dir: TempDir, } impl FakeServer { @@ -102,30 +92,11 @@ impl FakeServer { /// Starts and returns new FakeServer in a random port. pub fn start_new() -> Self { - Self::start_new_with_legacy_signatures(false) - } - - /// If `use_legacy_signatures` is set to true, the jwks endpoint is still available but - /// configs are signed using the legacy system. - pub fn start_new_with_legacy_signatures(use_legacy_signatures: bool) -> Self { // While binding to port 0, the kernel gives you a free ephemeral port. let listener = net::TcpListener::bind("0.0.0.0:0").unwrap(); let port = listener.local_addr().unwrap().port(); - // Legacy certificate-based key pair - let legacy_key_pair = KeyPair::generate_for(&PKCS_ED25519).unwrap(); - let cert = CertificateParams::new(vec!["localhost".to_string()]) - .unwrap() - .self_signed(&legacy_key_pair) - .unwrap(); - - let tmp_dir = tempfile::tempdir().unwrap(); - std::fs::write(tmp_dir.path().join(CERT_FILE), cert.pem()).unwrap(); - - let state = Arc::new(Mutex::new(ServerState::new( - legacy_key_pair, - use_legacy_signatures, - ))); + let state = Arc::new(Mutex::new(ServerState::generate())); let handle = tokio_runtime().spawn(Self::run_http_server(listener, state.clone())); @@ -134,7 +105,6 @@ impl FakeServer { state, port, path: FAKE_SERVER_PATH.to_string(), - cert_tmp_dir: tmp_dir, } } @@ -165,10 +135,6 @@ impl FakeServer { .remote_config = Some(response.as_ref().into()); } - pub fn cert_file_path(&self) -> PathBuf { - self.cert_tmp_dir.path().join(CERT_FILE) - } - pub fn get_health_status(&self, identifier: &InstanceID) -> Option { let state = self.state.lock().unwrap(); state @@ -269,26 +235,7 @@ async fn opamp_handler(state: web::Data>>, req: web::Byte let _ = agent_state; // We need to get rid of the mutable reference before leveraging another immutable. - let (key_pair, key_id) = if server_state.use_legacy_signatures { - ( - &Ed25519KeyPair::from_pkcs8(&server_state.legacy_key_pair.serialize_der()).unwrap(), - public_key_fingerprint(&server_state.legacy_key_pair.subject_public_key_info()), - ) - } else { - let public_key = server_state.key_pair.public_key().as_ref().to_vec(); - (&server_state.key_pair, public_key_fingerprint(&public_key)) - }; - - 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, - ); + let server_to_agent = build_response(identifier, remote_config, &server_state.key_pair, flags); HttpResponse::Ok().body(server_to_agent) } @@ -302,7 +249,7 @@ async fn jwks_handler(state: web::Data>>, _req: web::Byte "kty": "OKP", "alg": null, "use": "sig", - "kid": public_key_fingerprint(&public_key), + "kid": JWKS_PUBLIC_KEY_ID, "n": null, "x": enc_public_key, "y": null, @@ -317,9 +264,7 @@ fn build_response( instance_id: InstanceID, agent_remote_config: Option, key_pair: &Ed25519KeyPair, - key_id: String, flags: u64, - use_legacy_signature: bool, ) -> Vec { let mut remote_config = None; let mut custom_message = None; @@ -338,17 +283,12 @@ fn build_response( }), }); - 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) - }; - + // 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()); + let msg = BASE64_STANDARD.encode(digest); let signature = key_pair.sign(msg.as_bytes()); let custom_message_data = HashMap::from([( @@ -356,7 +296,7 @@ fn build_response( vec![SignatureFields { signature: BASE64_STANDARD.encode(signature), signing_algorithm: ED25519, - key_id, + key_id: JWKS_PUBLIC_KEY_ID.to_string(), }], )]); diff --git a/agent-control/tests/k8s/data/k8s_fail_remote_config_missing_required_values/local-data-agent-control.template b/agent-control/tests/k8s/data/k8s_fail_remote_config_missing_required_values/local-data-agent-control.template index db78acd8de..9de5d3927c 100644 --- a/agent-control/tests/k8s/data/k8s_fail_remote_config_missing_required_values/local-data-agent-control.template +++ b/agent-control/tests/k8s/data/k8s_fail_remote_config_missing_required_values/local-data-agent-control.template @@ -2,7 +2,6 @@ fleet_control: endpoint: poll_interval: 5s signature_validation: - certificate_pem_file_path: public_key_server_url: k8s: namespace: diff --git a/agent-control/tests/k8s/data/k8s_garbage_collector_triggers_on_ac_startup/local-data-agent-control.template b/agent-control/tests/k8s/data/k8s_garbage_collector_triggers_on_ac_startup/local-data-agent-control.template index e2687042e1..37c86a60a9 100644 --- a/agent-control/tests/k8s/data/k8s_garbage_collector_triggers_on_ac_startup/local-data-agent-control.template +++ b/agent-control/tests/k8s/data/k8s_garbage_collector_triggers_on_ac_startup/local-data-agent-control.template @@ -2,7 +2,6 @@ fleet_control: endpoint: poll_interval: 5s signature_validation: - certificate_pem_file_path: public_key_server_url: k8s: namespace: diff --git a/agent-control/tests/k8s/data/k8s_opamp_add_sub_agent/local-data-agent-control.template b/agent-control/tests/k8s/data/k8s_opamp_add_sub_agent/local-data-agent-control.template index 911d31d083..75f46d6862 100644 --- a/agent-control/tests/k8s/data/k8s_opamp_add_sub_agent/local-data-agent-control.template +++ b/agent-control/tests/k8s/data/k8s_opamp_add_sub_agent/local-data-agent-control.template @@ -2,7 +2,6 @@ fleet_control: endpoint: poll_interval: 5s signature_validation: - certificate_pem_file_path: public_key_server_url: k8s: namespace: diff --git a/agent-control/tests/k8s/data/k8s_opamp_attributes_existing_agent_type/local-data-agent-control.template b/agent-control/tests/k8s/data/k8s_opamp_attributes_existing_agent_type/local-data-agent-control.template index 278101b7c1..022a057087 100644 --- a/agent-control/tests/k8s/data/k8s_opamp_attributes_existing_agent_type/local-data-agent-control.template +++ b/agent-control/tests/k8s/data/k8s_opamp_attributes_existing_agent_type/local-data-agent-control.template @@ -2,7 +2,6 @@ fleet_control: endpoint: poll_interval: 5s signature_validation: - certificate_pem_file_path: public_key_server_url: k8s: namespace: diff --git a/agent-control/tests/k8s/data/k8s_opamp_cr_subagent_installed_before_crd/local-data-agent-control.template b/agent-control/tests/k8s/data/k8s_opamp_cr_subagent_installed_before_crd/local-data-agent-control.template index 6ae87b3d8e..d066fc5544 100644 --- a/agent-control/tests/k8s/data/k8s_opamp_cr_subagent_installed_before_crd/local-data-agent-control.template +++ b/agent-control/tests/k8s/data/k8s_opamp_cr_subagent_installed_before_crd/local-data-agent-control.template @@ -2,7 +2,6 @@ fleet_control: endpoint: poll_interval: 5s signature_validation: - certificate_pem_file_path: public_key_server_url: k8s: namespace: diff --git a/agent-control/tests/k8s/data/k8s_opamp_foo_cr_subagent/local-data-agent-control.template b/agent-control/tests/k8s/data/k8s_opamp_foo_cr_subagent/local-data-agent-control.template index e2687042e1..37c86a60a9 100644 --- a/agent-control/tests/k8s/data/k8s_opamp_foo_cr_subagent/local-data-agent-control.template +++ b/agent-control/tests/k8s/data/k8s_opamp_foo_cr_subagent/local-data-agent-control.template @@ -2,7 +2,6 @@ fleet_control: endpoint: poll_interval: 5s signature_validation: - certificate_pem_file_path: public_key_server_url: k8s: namespace: diff --git a/agent-control/tests/k8s/data/k8s_opamp_modify_subagent_config/local-data-agent-control.template b/agent-control/tests/k8s/data/k8s_opamp_modify_subagent_config/local-data-agent-control.template index 5fbe0ca0bc..b8c421a2fb 100644 --- a/agent-control/tests/k8s/data/k8s_opamp_modify_subagent_config/local-data-agent-control.template +++ b/agent-control/tests/k8s/data/k8s_opamp_modify_subagent_config/local-data-agent-control.template @@ -2,7 +2,6 @@ fleet_control: endpoint: poll_interval: 5s signature_validation: - certificate_pem_file_path: public_key_server_url: k8s: namespace: diff --git a/agent-control/tests/k8s/data/k8s_opamp_remove_subagent/local-data-agent-control.template b/agent-control/tests/k8s/data/k8s_opamp_remove_subagent/local-data-agent-control.template index 5fbe0ca0bc..b8c421a2fb 100644 --- a/agent-control/tests/k8s/data/k8s_opamp_remove_subagent/local-data-agent-control.template +++ b/agent-control/tests/k8s/data/k8s_opamp_remove_subagent/local-data-agent-control.template @@ -2,7 +2,6 @@ fleet_control: endpoint: poll_interval: 5s signature_validation: - certificate_pem_file_path: public_key_server_url: k8s: namespace: diff --git a/agent-control/tests/k8s/data/k8s_opamp_subagent_configuration_change_after_ac_restarts/local-data-agent-control.template b/agent-control/tests/k8s/data/k8s_opamp_subagent_configuration_change_after_ac_restarts/local-data-agent-control.template index 5fbe0ca0bc..b8c421a2fb 100644 --- a/agent-control/tests/k8s/data/k8s_opamp_subagent_configuration_change_after_ac_restarts/local-data-agent-control.template +++ b/agent-control/tests/k8s/data/k8s_opamp_subagent_configuration_change_after_ac_restarts/local-data-agent-control.template @@ -2,7 +2,6 @@ fleet_control: endpoint: poll_interval: 5s signature_validation: - certificate_pem_file_path: public_key_server_url: k8s: namespace: diff --git a/agent-control/tests/k8s/data/k8s_remote_flux_update/local-data-agent-control.template b/agent-control/tests/k8s/data/k8s_remote_flux_update/local-data-agent-control.template index 38114f95f1..f8df50859d 100644 --- a/agent-control/tests/k8s/data/k8s_remote_flux_update/local-data-agent-control.template +++ b/agent-control/tests/k8s/data/k8s_remote_flux_update/local-data-agent-control.template @@ -10,6 +10,5 @@ fleet_control: endpoint: poll_interval: 5s signature_validation: - certificate_pem_file_path: public_key_server_url: agents: {} diff --git a/agent-control/tests/k8s/flux_self_update.rs b/agent-control/tests/k8s/flux_self_update.rs index 95e7165786..b2694acd23 100644 --- a/agent-control/tests/k8s/flux_self_update.rs +++ b/agent-control/tests/k8s/flux_self_update.rs @@ -91,7 +91,6 @@ fn k8s_remote_flux_update() { k8s.client.clone(), &namespace, &namespace, - Some(opamp_server.cert_file_path()), Some(&opamp_server.endpoint()), Some(&opamp_server.jwks_endpoint()), vec![], @@ -176,7 +175,6 @@ fn k8s_remote_flux_update_with_wrong_version_causes_unhealthy() { k8s.client.clone(), &namespace, &namespace, - Some(opamp_server.cert_file_path()), Some(&opamp_server.endpoint()), Some(&opamp_server.jwks_endpoint()), vec![], diff --git a/agent-control/tests/k8s/scenarios/ac_restarts.rs b/agent-control/tests/k8s/scenarios/ac_restarts.rs index 538f318f18..063a69dfa7 100644 --- a/agent-control/tests/k8s/scenarios/ac_restarts.rs +++ b/agent-control/tests/k8s/scenarios/ac_restarts.rs @@ -40,7 +40,6 @@ fn k8s_opamp_subagent_configuration_change_after_ac_restarts() { k8s.client.clone(), &namespace, &namespace, - Some(server.cert_file_path()), Some(&server.endpoint()), Some(&server.jwks_endpoint()), // This config is intended to be empty @@ -98,7 +97,6 @@ valid: true k8s.client.clone(), &namespace, &namespace, - Some(server.cert_file_path()), Some(&server.endpoint()), Some(&server.jwks_endpoint()), // This config is intended to be empty diff --git a/agent-control/tests/k8s/scenarios/attributes.rs b/agent-control/tests/k8s/scenarios/attributes.rs index 00a9dddad4..f1bc69288b 100644 --- a/agent-control/tests/k8s/scenarios/attributes.rs +++ b/agent-control/tests/k8s/scenarios/attributes.rs @@ -42,7 +42,6 @@ fn k8s_test_attributes_from_existing_agent_type() { k8s.client.clone(), &namespace, &namespace, - Some(server.cert_file_path()), Some(&server.endpoint()), Some(&server.jwks_endpoint()), vec!["local-data-hello-world"], diff --git a/agent-control/tests/k8s/scenarios/config_signature.rs b/agent-control/tests/k8s/scenarios/config_signature.rs index be73704411..b876cf81e2 100644 --- a/agent-control/tests/k8s/scenarios/config_signature.rs +++ b/agent-control/tests/k8s/scenarios/config_signature.rs @@ -32,7 +32,6 @@ fn k8s_signature_disabled() { k8s.client.clone(), &namespace, &namespace, - None, Some(&server.endpoint()), Some(&server.jwks_endpoint()), // This config is intended to be empty diff --git a/agent-control/tests/k8s/scenarios/cr_based_agents.rs b/agent-control/tests/k8s/scenarios/cr_based_agents.rs index c43f82330c..c4fa17ef11 100644 --- a/agent-control/tests/k8s/scenarios/cr_based_agents.rs +++ b/agent-control/tests/k8s/scenarios/cr_based_agents.rs @@ -40,7 +40,6 @@ fn k8s_opamp_foo_cr_subagent() { k8s.client.clone(), &namespace, &namespace, - Some(server.cert_file_path()), Some(&server.endpoint()), Some(&server.jwks_endpoint()), Vec::new(), @@ -122,7 +121,6 @@ fn k8s_opamp_cr_subagent_installed_before_crd() { k8s.client.clone(), &namespace, &namespace, - Some(server.cert_file_path()), Some(&server.endpoint()), Some(&server.jwks_endpoint()), Vec::new(), diff --git a/agent-control/tests/k8s/scenarios/fail_remote_config.rs b/agent-control/tests/k8s/scenarios/fail_remote_config.rs index 2a07efd957..8a7fe6dd54 100644 --- a/agent-control/tests/k8s/scenarios/fail_remote_config.rs +++ b/agent-control/tests/k8s/scenarios/fail_remote_config.rs @@ -30,7 +30,6 @@ fn k8s_fail_remote_config_missing_required_values() { k8s.client.clone(), &namespace, &namespace, - Some(server.cert_file_path()), Some(&server.endpoint()), Some(&server.jwks_endpoint()), vec!["local-data-fake-agent"], diff --git a/agent-control/tests/k8s/scenarios/garbage_collector.rs b/agent-control/tests/k8s/scenarios/garbage_collector.rs index 9be2fdd365..a16ebfa8fc 100644 --- a/agent-control/tests/k8s/scenarios/garbage_collector.rs +++ b/agent-control/tests/k8s/scenarios/garbage_collector.rs @@ -43,7 +43,6 @@ fn k8s_garbage_collector_triggers_on_ac_startup() { k8s.client.clone(), &test_ns, &test_ns, - Some(server.cert_file_path()), Some(&server.endpoint()), Some(&server.jwks_endpoint()), // This config is intended to be empty diff --git a/agent-control/tests/k8s/scenarios/no_opamp.rs b/agent-control/tests/k8s/scenarios/no_opamp.rs index 606f7d81f0..d904017cb5 100644 --- a/agent-control/tests/k8s/scenarios/no_opamp.rs +++ b/agent-control/tests/k8s/scenarios/no_opamp.rs @@ -26,7 +26,6 @@ fn k8s_sub_agent_started_with_no_opamp() { &namespace, None, None, - None, vec!["local-data-hello-world"], tmp_dir.path(), ); diff --git a/agent-control/tests/k8s/scenarios/opamp.rs b/agent-control/tests/k8s/scenarios/opamp.rs index cb41e58377..5534098613 100644 --- a/agent-control/tests/k8s/scenarios/opamp.rs +++ b/agent-control/tests/k8s/scenarios/opamp.rs @@ -42,7 +42,6 @@ fn k8s_opamp_remove_subagent() { k8s.client.clone(), &ac_ns, &agents_ns, - Some(server.cert_file_path()), Some(&server.endpoint()), Some(&server.jwks_endpoint()), vec!["local-data-hello-world"], @@ -148,7 +147,6 @@ fn k8s_opamp_add_subagent() { k8s.client.clone(), &ac_ns, &agents_ns, - Some(server.cert_file_path()), Some(&server.endpoint()), Some(&server.jwks_endpoint()), vec!["local-data-hello-world"], @@ -210,7 +208,6 @@ fn k8s_opamp_modify_subagent_config() { k8s.client.clone(), &namespace, &namespace, - Some(server.cert_file_path()), Some(&server.endpoint()), Some(&server.jwks_endpoint()), vec!["local-data-hello-world"], diff --git a/agent-control/tests/k8s/scenarios/secrets_providers.rs b/agent-control/tests/k8s/scenarios/secrets_providers.rs index c19dd86e78..4bf052f585 100644 --- a/agent-control/tests/k8s/scenarios/secrets_providers.rs +++ b/agent-control/tests/k8s/scenarios/secrets_providers.rs @@ -31,7 +31,6 @@ fn k8s_template_secrets() { &namespace, None, None, - None, // This config is intended to be empty vec!["local-data-hello-world"], tmp_dir.path(), diff --git a/agent-control/tests/k8s/tools/agent_control.rs b/agent-control/tests/k8s/tools/agent_control.rs index 1058aafb06..904176eb59 100644 --- a/agent-control/tests/k8s/tools/agent_control.rs +++ b/agent-control/tests/k8s/tools/agent_control.rs @@ -16,10 +16,10 @@ use newrelic_agent_control::{ agent_control::run::BasePaths, k8s::store::{CM_NAME_LOCAL_DATA_PREFIX, K8sStore, STORE_KEY_LOCAL_DATA_CONFIG}, }; +use std::collections::BTreeMap; use std::io::Read; use std::path::Path; use std::time::Duration; -use std::{collections::BTreeMap, path::PathBuf}; use std::{fs::File, io::Write}; pub const TEST_CLUSTER_NAME: &str = "minikube"; @@ -40,7 +40,6 @@ pub fn start_agent_control_with_testdata_config( client: Client, ac_ns: &str, agents_ns: &str, - remote_config_sign_cert: Option, opamp_endpoint: Option<&str>, jwks_endpoint: Option<&str>, subagent_file_names: Vec<&str>, @@ -61,7 +60,6 @@ pub fn start_agent_control_with_testdata_config( agents_ns, opamp_endpoint, jwks_endpoint, - remote_config_sign_cert, folder_name, local_dir, ); @@ -138,7 +136,6 @@ pub fn create_local_agent_control_config( agents_ns: &str, opamp_endpoint: Option<&str>, jwk_endpoint: Option<&str>, - remote_config_sign_cert: Option, folder_name: &str, tmp_dir: &Path, ) { @@ -161,9 +158,6 @@ pub fn create_local_agent_control_config( if let Some(endpoint) = jwk_endpoint { content = content.replace("", endpoint); } - if let Some(cert_path) = remote_config_sign_cert { - content = content.replace("", cert_path.to_str().unwrap()); - } block_on(create_config_map( client, ac_ns, diff --git a/agent-control/tests/on_host/opamp_auth.rs b/agent-control/tests/on_host/opamp_auth.rs index 3b21cc4977..1a57a9ba05 100644 --- a/agent-control/tests/on_host/opamp_auth.rs +++ b/agent-control/tests/on_host/opamp_auth.rs @@ -47,6 +47,8 @@ fleet_control: client_id: "fake" provider: "local" private_key_path: "{}" + signature_validation: + enabled: false log: level: debug agents: {{}} @@ -105,6 +107,8 @@ fleet_control: endpoint: "{}" headers: api-key: "fakeKey" + signature_validation: + enabled: false log: level: debug agents: {{}} @@ -160,6 +164,8 @@ fleet_control: client_id: "fake" provider: "local" private_key_path: "{}" + signature_validation: + enabled: false log: level: debug agents: {{}} diff --git a/agent-control/tests/on_host/proxy.rs b/agent-control/tests/on_host/proxy.rs index 8d54d92a61..fc407b18c6 100644 --- a/agent-control/tests/on_host/proxy.rs +++ b/agent-control/tests/on_host/proxy.rs @@ -59,7 +59,6 @@ fn proxy_onhost_opamp_agent_control_local_effective_config() { Some(format!( "{{\"url\": \"{proxy_url}\", \"ca_bundle_dir\": \"{proxy_ca_dir}\"}}" )), - opamp_server.cert_file_path(), ); let base_paths = BasePaths { diff --git a/agent-control/tests/on_host/scenarios/attributes.rs b/agent-control/tests/on_host/scenarios/attributes.rs index d218ae0b2f..e9975951b8 100644 --- a/agent-control/tests/on_host/scenarios/attributes.rs +++ b/agent-control/tests/on_host/scenarios/attributes.rs @@ -49,7 +49,6 @@ fn test_attributes_from_non_existing_agent_type() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); let base_paths = BasePaths { @@ -135,7 +134,6 @@ agents: opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); // And the custom-agent has empty config values diff --git a/agent-control/tests/on_host/scenarios/empty_config.rs b/agent-control/tests/on_host/scenarios/empty_config.rs index 1c522e3e53..4ae17bca69 100644 --- a/agent-control/tests/on_host/scenarios/empty_config.rs +++ b/agent-control/tests/on_host/scenarios/empty_config.rs @@ -43,7 +43,6 @@ fn onhost_opamp_sub_agent_set_empty_config_defaults_to_local() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); // And the custom-agent has local config values @@ -122,7 +121,6 @@ fn onhost_opamp_sub_agent_with_no_local_config() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); // There is no local configuration for the sub-agent diff --git a/agent-control/tests/on_host/scenarios/filesystem_ops.rs b/agent-control/tests/on_host/scenarios/filesystem_ops.rs index 52ecda59a3..6d23f7da2d 100644 --- a/agent-control/tests/on_host/scenarios/filesystem_ops.rs +++ b/agent-control/tests/on_host/scenarios/filesystem_ops.rs @@ -60,7 +60,6 @@ deployment: opamp_server.jwks_endpoint(), agents.to_string(), local_dir.to_path_buf(), - opamp_server.cert_file_path(), ); create_sub_agent_values( agent_id.to_string(), @@ -179,7 +178,6 @@ deployment: "# ), local_dir.to_path_buf(), - opamp_server.cert_file_path(), ); // Values. Contains 3 variables: a YAML, a string, and a map[string]yaml (to create files in a directory) create_sub_agent_values( diff --git a/agent-control/tests/on_host/scenarios/health_check.rs b/agent-control/tests/on_host/scenarios/health_check.rs index a187f6fb02..af8eb52d90 100644 --- a/agent-control/tests/on_host/scenarios/health_check.rs +++ b/agent-control/tests/on_host/scenarios/health_check.rs @@ -53,7 +53,6 @@ file: opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); let base_paths = BasePaths { @@ -160,7 +159,6 @@ http: opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); create_sub_agent_values(sub_agent_id.to_string(), "".into(), local_dir.path().into()); diff --git a/agent-control/tests/on_host/scenarios/invalid_remote_config.rs b/agent-control/tests/on_host/scenarios/invalid_remote_config.rs index 20b1cdd4ea..94681cb12a 100644 --- a/agent-control/tests/on_host/scenarios/invalid_remote_config.rs +++ b/agent-control/tests/on_host/scenarios/invalid_remote_config.rs @@ -43,7 +43,6 @@ fn onhost_opamp_sub_agent_invalid_remote_config() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); // And the custom-agent has local config values @@ -133,7 +132,6 @@ fn test_invalid_config_executable_less_supervisor() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); let base_paths = BasePaths { @@ -211,7 +209,6 @@ fn onhost_opamp_sub_agent_invalid_remote_config_rollback_previous_remote() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); // And the custom-agent has local config values diff --git a/agent-control/tests/on_host/scenarios/multiple_executables.rs b/agent-control/tests/on_host/scenarios/multiple_executables.rs index 54735ead54..20056b64f1 100644 --- a/agent-control/tests/on_host/scenarios/multiple_executables.rs +++ b/agent-control/tests/on_host/scenarios/multiple_executables.rs @@ -47,7 +47,6 @@ agents: opamp_server.jwks_endpoint(), agents, local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); let base_paths = BasePaths { @@ -106,7 +105,6 @@ agents: opamp_server.jwks_endpoint(), agents, local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); let base_paths = BasePaths { diff --git a/agent-control/tests/on_host/scenarios/opamp.rs b/agent-control/tests/on_host/scenarios/opamp.rs index 53e5ee3699..a1d93b34e9 100644 --- a/agent-control/tests/on_host/scenarios/opamp.rs +++ b/agent-control/tests/on_host/scenarios/opamp.rs @@ -39,7 +39,6 @@ fn onhost_opamp_agent_control_local_effective_config() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); let base_paths = BasePaths { @@ -87,7 +86,6 @@ fn onhost_opamp_agent_control_remote_effective_config() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); let base_paths = BasePaths { @@ -175,7 +173,6 @@ fn onhost_opamp_agent_control_remote_config_with_unknown_field() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); let base_paths = BasePaths { @@ -259,7 +256,6 @@ fn onhost_opamp_sub_agent_local_effective_config_with_env_var() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); // And the custom-agent has local config values @@ -332,7 +328,6 @@ fn onhost_opamp_sub_agent_remote_effective_config() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); // And the custom-agent has local config values @@ -402,7 +397,6 @@ fn onhost_opamp_sub_agent_empty_local_effective_config() { opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); // And the custom-agent has empty config values @@ -488,7 +482,6 @@ agents: opamp_server.jwks_endpoint(), agents.to_string(), local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), ); let sub_agent_id = AgentID::try_from("no-executables").unwrap(); @@ -554,89 +547,3 @@ status_time_unix_nano: 1725444001 check_latest_health_status_was_healthy(&opamp_server, &sub_agent_instance_id) }); } - -/// Test that if AC signature validation fails using the public key obtained from the JWKS endpoint, it falls back -/// to the previous certificate validation (useful while transitioning). -#[test] -fn test_opamp_with_legacy_signatures() { - let mut opamp_server = FakeServer::start_new_with_legacy_signatures(true); - - let local_dir = tempdir().expect("failed to create local temp dir"); - let remote_dir = tempdir().expect("failed to create remote temp dir"); - - let agents = "{}"; - create_agent_control_config( - opamp_server.endpoint(), - opamp_server.jwks_endpoint(), - agents.to_string(), - local_dir.path().to_path_buf(), - opamp_server.cert_file_path(), - ); - - let base_paths = BasePaths { - local_dir: local_dir.path().to_path_buf(), - remote_dir: remote_dir.path().to_path_buf(), - log_dir: local_dir.path().to_path_buf(), - }; - - // Add custom agent_type to registry - let sleep_agent_type = CustomAgentType::default().build(local_dir.path().to_path_buf()); - - let _agent_control = - start_agent_control_with_custom_config(base_paths.clone(), Environment::OnHost); - - let agent_control_instance_id = get_instance_id(&AgentID::AgentControl, base_paths.clone()); - - let agents = format!( - r#" -agents: - nr-sleep-agent: - agent_type: "{sleep_agent_type}" -"# - ); - - // When a new config with an agent is received from OpAMP - opamp_server.set_config_response(agent_control_instance_id.clone(), agents.as_str()); - - // Then the config should be updated in the remote filesystem. - let expected_config = format!( - r#"agents: - nr-sleep-agent: - agent_type: "{sleep_agent_type}" -"# - ); - let expected_config_parsed = - serde_yaml::from_str::(expected_config.as_str()).unwrap(); - - retry(60, Duration::from_secs(1), || { - let remote_file = remote_dir.path().join(AGENT_CONTROL_CONFIG_FILENAME); - let remote_config = std::fs::read_to_string(remote_file.as_path()) - .unwrap_or("config: \nhash: a-hash\nstate: applying\n".to_string()); - let content_parsed = serde_yaml::from_str::(remote_config.as_str()).unwrap(); - if content_parsed.config != expected_config_parsed { - return Err(format!( - "Agent Control config not as expected, Expected: {expected_config:?}, Found: {remote_config:?}", - ) - .into()); - } - - check_latest_effective_config_is_expected( - &opamp_server, - &agent_control_instance_id, - serde_yaml::to_string(&content_parsed.config).unwrap(), - )?; - check_latest_health_status_was_healthy(&opamp_server, &agent_control_instance_id) - }); - - let subagent_instance_id = get_instance_id( - &AgentID::try_from("nr-sleep-agent").unwrap(), - base_paths.clone(), - ); - - // The sub-agent waits for the remote config to be set, it cannot be empty since it would default to local - // which does not exist. - opamp_server.set_config_response(subagent_instance_id.clone(), "fake_variable: value"); - retry(60, Duration::from_secs(1), || { - check_latest_health_status_was_healthy(&opamp_server, &subagent_instance_id) - }); -} diff --git a/agent-control/tests/on_host/tools/config.rs b/agent-control/tests/on_host/tools/config.rs index 095af9c1d1..25aa12b4a7 100644 --- a/agent-control/tests/on_host/tools/config.rs +++ b/agent-control/tests/on_host/tools/config.rs @@ -17,7 +17,6 @@ pub fn create_agent_control_config( jwks_endpoint: String, agents: String, local_dir: PathBuf, - remote_config_sign_cert: PathBuf, ) { create_agent_control_config_with_proxy( opamp_server_endpoint, @@ -25,7 +24,6 @@ pub fn create_agent_control_config( agents, local_dir, None, - remote_config_sign_cert, ); } @@ -36,7 +34,6 @@ pub fn create_agent_control_config_with_proxy( agents: String, local_dir: PathBuf, proxy: Option, - remote_config_sign_cert: PathBuf, ) { let proxy_config = proxy .map(|config| format!("proxy: {config}")) @@ -50,15 +47,10 @@ fleet_control: poll_interval: 5s signature_validation: public_key_server_url: {} - certificate_pem_file_path: {} agents: {} {} "#, - opamp_server_endpoint, - jwks_endpoint, - remote_config_sign_cert.to_str().unwrap(), - agents, - proxy_config, + opamp_server_endpoint, jwks_endpoint, agents, proxy_config, ); create_file( agent_control_config, diff --git a/docs/CONFIG.md b/docs/CONFIG.md index 17d2c01de9..c42a702fc8 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -58,10 +58,8 @@ fleet_control: retries: 3 # Number of retries if the authentication fails. fleet_id: "some-id" # Fleet identifier. signature_validation: - certificate_server_url: "https://newrelic.com/" # Server to obtain the certificate for signature validation. - public_key_server_url: "https://publickeys.newrelic.com/signing/blob-management/global/agentconfiguration" # Server to obtain the public key for signature validation. - certificate_pem_file_path: "/some/certificate/" # Optional, if set it uses a local certificated instead of fetching it from 'certicate_server_url'. enabled: true # Defaults to true, allows disabling the signature validation. + public_key_server_url: "https://publickeys.newrelic.com/signing/blob-management/global/agentconfiguration" # Server to obtain the public key for signature validation. ``` ### proxy @@ -73,8 +71,6 @@ proxy options can also be configured using the proxy configuration field. If bot 2. `HTTP_PROXY` environment variable 3. `HTTPS_PROXY` environment variable -⚠️ Proxy configuration is currently not compatible with fetching the certificate for signature validation. If you need to setup a proxy you will need to either, add a firewall exception to https://newrelic.com so requests to that endpoints can skip the proxy (recommended), use a local certificate through `fleet_control.signature_validation.certificate_pem_file_path` (certificate rotation should need to be manually handled) or disable signature validation (highly discouraged). - ```yaml proxy: url: https://proxy.url:8080 # Proxy url in format '://:@:' diff --git a/test/k8s-e2e/proxy/ac-values-proxy.yml b/test/k8s-e2e/proxy/ac-values-proxy.yml index 9677d5a3bb..1dc145f45f 100644 --- a/test/k8s-e2e/proxy/ac-values-proxy.yml +++ b/test/k8s-e2e/proxy/ac-values-proxy.yml @@ -71,13 +71,6 @@ agentControlDeployment: # Fleet with name 'ac-e2e-empty-deployment'. The sub-agent reports data to the 'Agent Control Canaries' account. # This fleet doesn't have any deployment configured. fleet_id: NjQyNTg2NXxOR0VQfEZMRUVUfDAxOTgwOTZiLTk2NzctNzllNi05OWJmLTQ4YWRlOTRiYmZjMw - override: - fleet_control: - signature_validation: - # Disable this as is not supported by the proxy. - # On a prod environment is recommended to add an exception for the firewall to newrelic.com or - # mount the downloaded Certificate. - enabled: false agents: prometheus: agent_type: newrelic/com.newrelic.prometheus:0.1.0