Skip to content

Commit 175be23

Browse files
feat: add signature validation from local cert (NR-348962) (#997)
* feat: add signature validation from local cert * clean cert file on drop
1 parent ee019eb commit 175be23

File tree

25 files changed

+294
-18
lines changed

25 files changed

+294
-18
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ fn parse_rsa_algorithm(algo: &str) -> Option<SigningAlgorithm> {
186186
#[derive(Debug, Serialize, PartialEq, Clone)]
187187
pub struct Signatures {
188188
#[serde(flatten)]
189-
signatures: HashMap<ConfigID, SignatureData>,
189+
pub signatures: HashMap<ConfigID, SignatureData>,
190190
}
191191

192192
impl<'de> Deserialize<'de> for Signatures {

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

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
1+
use std::path::PathBuf;
12
use std::time::Duration;
23

34
use crate::agent_control::config::AgentTypeFQN;
45
use crate::opamp::remote_config::signature::SIGNATURE_CUSTOM_CAPABILITY;
56
use crate::opamp::remote_config::validators::RemoteConfigValidator;
67
use crate::opamp::remote_config::RemoteConfig;
8+
use nix::NixPath;
79
use serde::{Deserialize, Serialize};
810
use thiserror::Error;
911
use tracing::log::error;
12+
use tracing::{info, warn};
1013
use url::Url;
1114

1215
use super::certificate_fetcher::CertificateFetcher;
@@ -33,11 +36,25 @@ pub fn build_signature_validator(
3336
config: SignatureValidatorConfig,
3437
) -> Result<SignatureValidator, SignatureValidatorError> {
3538
if !config.enabled {
39+
warn!("Remote config signature validation is disabled");
3640
return Ok(SignatureValidator::Noop);
3741
}
3842

39-
let certificate_fetcher =
40-
CertificateFetcher::Https(config.certificate_server_url, DEFAULT_HTTPS_CLIENT_TIMEOUT);
43+
// Certificate from file takes precedence over fetching from the server when it is set
44+
let certificate_fetcher = if !config.certificate_pem_file_path.is_empty() {
45+
warn!(
46+
"Remote config signature validation is enabled, using certificate from file: {}. Certificate rotation is not supported",
47+
config.certificate_pem_file_path.display()
48+
);
49+
CertificateFetcher::PemFile(config.certificate_pem_file_path)
50+
} else {
51+
info!(
52+
"Remote config signature validation is enabled, fetching certificate from: {}",
53+
config.certificate_server_url
54+
);
55+
CertificateFetcher::Https(config.certificate_server_url, DEFAULT_HTTPS_CLIENT_TIMEOUT)
56+
};
57+
4158
let certificate_store = CertificateStore::try_new(certificate_fetcher)
4259
.map_err(|e| SignatureValidatorError::InitializeCertificateStore(e.to_string()))?;
4360

@@ -50,6 +67,10 @@ pub fn build_signature_validator(
5067
pub struct SignatureValidatorConfig {
5168
#[serde(default = "default_signature_validator_url")]
5269
pub certificate_server_url: Url,
70+
/// Path to the PEM file containing the certificate to validate the signature.
71+
/// Takes precedence over fetching from the server when it is set
72+
#[serde(default)]
73+
pub certificate_pem_file_path: PathBuf,
5374
#[serde(default = "default_signature_validator_config_enabled")]
5475
pub enabled: bool,
5576
}
@@ -59,6 +80,7 @@ impl Default for SignatureValidatorConfig {
5980
Self {
6081
enabled: default_signature_validator_config_enabled(),
6182
certificate_server_url: default_signature_validator_url(),
83+
certificate_pem_file_path: PathBuf::new(),
6284
}
6385
}
6486
}
@@ -164,6 +186,7 @@ impl RemoteConfigValidator for CertificateSignatureValidator {
164186
#[cfg(test)]
165187
mod tests {
166188
use std::collections::HashMap;
189+
use std::str::FromStr;
167190

168191
use super::*;
169192
use crate::agent_control::config::AgentID;
@@ -218,6 +241,7 @@ enabled: false
218241
expected: SignatureValidatorConfig {
219242
enabled: false,
220243
certificate_server_url: Url::parse(DEFAULT_CERTIFICATE_SERVER_URL).unwrap(),
244+
certificate_pem_file_path: PathBuf::new(),
221245
},
222246
},
223247
TestCase {
@@ -228,6 +252,7 @@ enabled: true
228252
expected: SignatureValidatorConfig {
229253
enabled: true,
230254
certificate_server_url: Url::parse(DEFAULT_CERTIFICATE_SERVER_URL).unwrap(),
255+
certificate_pem_file_path: PathBuf::new(),
231256
},
232257
},
233258
TestCase {
@@ -238,6 +263,7 @@ certificate_server_url: https://example.com
238263
expected: SignatureValidatorConfig {
239264
enabled: DEFAULT_SIGNATURE_VALIDATOR_ENABLED,
240265
certificate_server_url: Url::parse("https://example.com").unwrap(),
266+
certificate_pem_file_path: PathBuf::new(),
241267
},
242268
},
243269
TestCase {
@@ -249,6 +275,32 @@ certificate_server_url: https://example.com
249275
expected: SignatureValidatorConfig {
250276
enabled: true,
251277
certificate_server_url: Url::parse("https://example.com").unwrap(),
278+
certificate_pem_file_path: PathBuf::new(),
279+
},
280+
},
281+
TestCase {
282+
name: "Setup file and enabled",
283+
cfg: r#"
284+
enabled: true
285+
certificate_pem_file_path: /path/to/file
286+
"#,
287+
expected: SignatureValidatorConfig {
288+
enabled: true,
289+
certificate_server_url: Url::parse(DEFAULT_CERTIFICATE_SERVER_URL).unwrap(),
290+
certificate_pem_file_path: PathBuf::from_str("/path/to/file").unwrap(),
291+
},
292+
},
293+
TestCase {
294+
name: "Setup file and url and enabled",
295+
cfg: r#"
296+
enabled: true
297+
certificate_server_url: https://example.com
298+
certificate_pem_file_path: /path/to/file
299+
"#,
300+
expected: SignatureValidatorConfig {
301+
enabled: true,
302+
certificate_server_url: Url::parse("https://example.com").unwrap(),
303+
certificate_pem_file_path: PathBuf::from_str("/path/to/file").unwrap(),
252304
},
253305
},
254306
];

agent-control/tests/common/opamp.rs

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,23 @@
11
use super::runtime::tokio_runtime;
22
use actix_web::{web, App, HttpResponse, HttpServer};
3+
use base64::prelude::BASE64_STANDARD;
4+
use base64::Engine;
35
use newrelic_agent_control::opamp::instance_id::InstanceID;
6+
use newrelic_agent_control::opamp::remote_config::signature::{
7+
SignatureData, SigningAlgorithm, SIGNATURE_CUSTOM_CAPABILITY, SIGNATURE_CUSTOM_MESSAGE_TYPE,
8+
};
49
use opamp_client::opamp;
510
use prost::Message;
11+
use rcgen::{CertificateParams, KeyPair, PKCS_ED25519};
12+
use std::path::PathBuf;
613
use std::sync::Mutex;
714
use std::time::Duration;
815
use std::{collections::HashMap, net, sync::Arc};
16+
use tempfile::TempDir;
917
use tokio::task::JoinHandle;
1018

1119
const FAKE_SERVER_PATH: &str = "/opamp-fake-server";
20+
const CERT_FILE: &str = "server.crt";
1221

1322
pub type ConfigResponses = HashMap<InstanceID, ConfigResponse>;
1423

@@ -28,13 +37,26 @@ pub type EffectiveConfigs = HashMap<InstanceID, opamp::proto::EffectiveConfig>;
2837
pub type RemoteConfigStatus = HashMap<InstanceID, opamp::proto::RemoteConfigStatus>;
2938

3039
/// Represents the state of the FakeServer.
31-
#[derive(Default)]
3240
struct State {
3341
health_statuses: HealthStatuses,
3442
attributes: Attributes,
3543
config_responses: ConfigResponses,
3644
effective_configs: EffectiveConfigs,
3745
config_status: RemoteConfigStatus,
46+
// Server private key to sign the remote config
47+
key_pair: KeyPair,
48+
}
49+
impl State {
50+
fn new(key_pair: KeyPair) -> Self {
51+
Self {
52+
health_statuses: Default::default(),
53+
attributes: Default::default(),
54+
config_responses: Default::default(),
55+
effective_configs: Default::default(),
56+
config_status: Default::default(),
57+
key_pair,
58+
}
59+
}
3860
}
3961

4062
#[derive(Clone, Debug, Default)]
@@ -52,26 +74,47 @@ impl From<&str> for ConfigResponse {
5274
}
5375

5476
impl ConfigResponse {
55-
fn encode(&self) -> Vec<u8> {
56-
// remote config is only set if there is any content
57-
let remote_config = self
58-
.clone()
59-
.raw_body
60-
.map(|raw_body| opamp::proto::AgentRemoteConfig {
77+
fn encode(&self, key_pair: &KeyPair) -> Vec<u8> {
78+
let mut remote_config = None;
79+
let mut custom_message = None;
80+
81+
if let Some(config) = self.raw_body.clone() {
82+
remote_config = Some(opamp::proto::AgentRemoteConfig {
6183
config_hash: "hash".into(), // fake has for the shake of simplicity
6284
config: Some(opamp::proto::AgentConfigMap {
6385
config_map: HashMap::from([(
6486
"".to_string(),
6587
opamp::proto::AgentConfigFile {
66-
body: raw_body.clone().into_bytes(),
88+
body: config.clone().into_bytes(),
6789
content_type: " text/yaml".to_string(),
6890
},
6991
)]),
7092
}),
7193
});
94+
95+
let key_pair_ring =
96+
ring::signature::Ed25519KeyPair::from_pkcs8(&key_pair.serialize_der()).unwrap();
97+
let signature = key_pair_ring.sign(config.as_bytes());
98+
99+
let custom_message_data: HashMap<String, Vec<SignatureData>> = HashMap::from([(
100+
"fakeCRC".to_string(),
101+
vec![SignatureData {
102+
signature: BASE64_STANDARD.encode(signature.as_ref()),
103+
signing_algorithm: SigningAlgorithm::ED25519,
104+
key_id: "fake".to_string(),
105+
}],
106+
)]);
107+
108+
custom_message = Some(opamp::proto::CustomMessage {
109+
capability: SIGNATURE_CUSTOM_CAPABILITY.to_string(),
110+
r#type: SIGNATURE_CUSTOM_MESSAGE_TYPE.to_string(),
111+
data: serde_json::to_vec(&custom_message_data).unwrap(),
112+
});
113+
}
72114
opamp::proto::ServerToAgent {
73115
instance_uid: "test".into(), // fake uid for the sake of simplicity
74116
remote_config,
117+
custom_message,
75118
..Default::default()
76119
}
77120
.encode_to_vec()
@@ -85,6 +128,7 @@ pub struct FakeServer {
85128
state: Arc<Mutex<State>>,
86129
port: u16,
87130
path: String,
131+
cert_tmp_dir: TempDir,
88132
}
89133

90134
impl FakeServer {
@@ -95,18 +139,29 @@ impl FakeServer {
95139

96140
/// Starts and returns new FakeServer in a random port.
97141
pub fn start_new() -> Self {
98-
let state = Arc::new(Mutex::new(State::default()));
99142
// While binding to port 0, the kernel gives you a free ephemeral port.
100143
let listener = net::TcpListener::bind("0.0.0.0:0").unwrap();
101144
let port = listener.local_addr().unwrap().port();
102145

146+
let key_pair = KeyPair::generate_for(&PKCS_ED25519).unwrap();
147+
let cert = CertificateParams::new(vec!["localhost".to_string()])
148+
.unwrap()
149+
.self_signed(&key_pair)
150+
.unwrap();
151+
152+
let tmp_dir = tempfile::tempdir().unwrap();
153+
std::fs::write(tmp_dir.path().join(CERT_FILE), cert.pem()).unwrap();
154+
155+
let state = Arc::new(Mutex::new(State::new(key_pair)));
156+
103157
let handle = tokio_runtime().spawn(Self::run_http_server(listener, state.clone()));
104158

105159
Self {
106160
handle,
107161
state,
108162
port,
109163
path: FAKE_SERVER_PATH.to_string(),
164+
cert_tmp_dir: tmp_dir,
110165
}
111166
}
112167

@@ -132,6 +187,10 @@ impl FakeServer {
132187
state.config_responses.insert(identifier, response);
133188
}
134189

190+
pub fn cert_file_path(&self) -> PathBuf {
191+
self.cert_tmp_dir.path().join(CERT_FILE)
192+
}
193+
135194
pub fn get_health_status(
136195
&self,
137196
identifier: &InstanceID,
@@ -220,5 +279,5 @@ async fn opamp_handler(state: web::Data<Arc<Mutex<State>>>, req: web::Bytes) ->
220279
// `message` content before removing the config response from the state.
221280
state.config_responses.remove(&instance_id);
222281

223-
HttpResponse::Ok().body(config_response.encode())
282+
HttpResponse::Ok().body(config_response.encode(&state.key_pair))
224283
}

agent-control/tests/k8s/data/k8s_opamp_add_sub_agent/local-data-agent-control.template

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ fleet_control:
22
endpoint: <opamp-endpoint>
33
headers:
44
api-key: test-api-key
5+
signature_validation:
6+
certificate_pem_file_path: <cert-path>
7+
enabled: true
58
k8s:
69
namespace: <ns>
710
cluster_name: <cluster-name>

agent-control/tests/k8s/data/k8s_opamp_attributes_existing_agent_type/local-data-agent-control.template

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ opamp:
22
endpoint: <opamp-endpoint>
33
headers:
44
api-key: test-api-key
5+
signature_validation:
6+
certificate_pem_file_path: <cert-path>
7+
enabled: true
58
k8s:
69
namespace: <ns>
710
cluster_name: <cluster-name>

agent-control/tests/k8s/data/k8s_opamp_cr_subagent_installed_before_crd/local-data-agent-control.template

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ fleet_control:
22
endpoint: <opamp-endpoint>
33
headers:
44
api-key: test-api-key
5+
signature_validation:
6+
certificate_pem_file_path: <cert-path>
7+
enabled: true
58
k8s:
69
namespace: <ns>
710
cluster_name: <cluster-name>

agent-control/tests/k8s/data/k8s_opamp_enabled_with_no_remote_configuration/local-data-agent-control.template

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ fleet_control:
22
endpoint: <opamp-endpoint>
33
headers:
44
api-key: test-api-key
5+
signature_validation:
6+
certificate_pem_file_path: <cert-path>
7+
enabled: true
58
k8s:
69
namespace: <ns>
710
cluster_name: <cluster-name>

agent-control/tests/k8s/data/k8s_opamp_foo_cr_subagent/local-data-agent-control.template

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ fleet_control:
22
endpoint: <opamp-endpoint>
33
headers:
44
api-key: test-api-key
5+
signature_validation:
6+
certificate_pem_file_path: <cert-path>
7+
enabled: true
58
k8s:
69
namespace: <ns>
710
cluster_name: <cluster-name>

agent-control/tests/k8s/data/k8s_opamp_subagent_configuration_change/local-data-agent-control.template

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ fleet_control:
22
endpoint: <opamp-endpoint>
33
headers:
44
api-key: test-api-key
5+
signature_validation:
6+
certificate_pem_file_path: <cert-path>
7+
enabled: true
58
k8s:
69
namespace: <ns>
710
cluster_name: <cluster-name>

agent-control/tests/k8s/data/k8s_opamp_subagent_modify_secret/local-data-agent-control.template

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ fleet_control:
22
endpoint: <opamp-endpoint>
33
headers:
44
api-key: test-api-key
5+
signature_validation:
6+
certificate_pem_file_path: <cert-path>
7+
enabled: true
58
k8s:
69
namespace: <ns>
710
cluster_name: <cluster-name>

0 commit comments

Comments
 (0)