Skip to content

Commit

Permalink
Merge branch 'BOUN-933-add-ip-parsing-check-in-invariants' into 'master'
Browse files Browse the repository at this point in the history 
(BOUN-933) Add ipv4 and ipv6 parsing checks in API BN registry invariants

 

See merge request dfinity-lab/public/ic!17180
  • Loading branch information
@nikolay-komarevskiy
nikolay-komarevskiy committed Jan 18, 2024
2 parents 0fa7c02 + a0f01a8 commit c1a1563
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 6 deletions.
7 changes: 6 additions & 1 deletion rs/registry/canister/src/common/test_helpers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::mutations::node_management::do_add_node::connection_endpoint_from_str
use crate::registry::Registry;
use ic_base_types::{NodeId, PrincipalId, SubnetId};
use ic_nns_test_utils::registry::{invariant_compliant_mutation, new_node_keys_and_node_id};
use ic_protobuf::registry::node::v1::IPv4InterfaceConfig;
use ic_protobuf::registry::node::v1::NodeRecord;
use ic_protobuf::registry::subnet::v1::SubnetListRecord;
use ic_protobuf::registry::subnet::v1::SubnetRecord;
Expand Down Expand Up @@ -96,8 +97,12 @@ pub fn prepare_registry_with_nodes(
"128.0.{effective_id}.1:1234"
))),
http: Some(connection_endpoint_from_string(&format!(
"128.0.{effective_id}.1:4321"
"[fe80::{effective_id}]:4321"
))),
public_ipv4_config: Some(IPv4InterfaceConfig {
ip_addr: format!("128.0.{effective_id}.1"),
..Default::default()
}),
node_operator_id: PrincipalId::new_user_test_id(999).into_vec(),
// Preset this field to Some(), in order to allow seamless creation of ApiBoundaryNodeRecord if needed.
domain: Some("node-example.com".into()),
Expand Down
112 changes: 107 additions & 5 deletions rs/registry/canister/src/invariants/api_boundary_node.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::collections::HashMap;
use std::{
collections::HashMap,
net::{Ipv4Addr, Ipv6Addr},
};

use ic_base_types::NodeId;

Expand All @@ -11,6 +14,7 @@ use super::common::{
/// * Ensure API Boundary Nodes have unique domain names
/// * Ensure each API Boundary Node record has a corresponding NodeRecord
/// * Ensure that both `domain` and `http` fields of the NodeRecord are not None
/// * Ensure that both `ipv6`(required) and `ipv4`(optional) of the NodeRecord can be parsed
pub(crate) fn check_api_boundary_node_invariants(
snapshot: &RegistrySnapshot,
) -> Result<(), InvariantCheckError> {
Expand Down Expand Up @@ -43,13 +47,29 @@ pub(crate) fn check_api_boundary_node_invariants(
});
};

let Some(_http) = node_record.http else {
let Some(http) = node_record.http else {
return Err(InvariantCheckError {
msg: format!("http field of the NodeRecord with id={api_bn_id} is None"),
source: None,
});
};

let Ok(_ipv6) = http.ip_addr.parse::<Ipv6Addr>() else {
return Err(InvariantCheckError {
msg: "failed to parse ipv6 address of the node".to_string(),
source: None,
});
};

if let Some(ipv4_config) = node_record.public_ipv4_config {
let Ok(_ipv4) = ipv4_config.ip_addr.parse::<Ipv4Addr>() else {
return Err(InvariantCheckError {
msg: "failed to parse ipv4 address of the node".to_string(),
source: None,
});
};
}

if let Some(existing_api_bn) = domain_to_id.get(&domain) {
return Err(InvariantCheckError {
msg: format!("domain {domain} has two nodes associated with it with id={existing_api_bn} and id={api_bn_id}"),
Expand All @@ -67,7 +87,7 @@ mod tests {
use ic_base_types::{NodeId, PrincipalId};
use ic_protobuf::registry::{
api_boundary_node::v1::ApiBoundaryNodeRecord,
node::v1::{ConnectionEndpoint, NodeRecord},
node::v1::{ConnectionEndpoint, IPv4InterfaceConfig, NodeRecord},
};
use ic_registry_keys::{make_api_boundary_node_record_key, make_node_record_key};

Expand All @@ -93,7 +113,14 @@ mod tests {
snapshot.insert(
make_node_record_key(node_id).into_bytes(), // key
encode_or_panic(&NodeRecord {
http: Some(ConnectionEndpoint::default()),
public_ipv4_config: Some(IPv4InterfaceConfig {
ip_addr: "127.0.0.1".to_string(),
..Default::default()
}),
http: Some(ConnectionEndpoint {
ip_addr: "2001:db8:3333:4444:5555:6666:7777:8888".to_string(),
..Default::default()
}),
domain: Some(domain.into()),
..Default::default()
}), // record
Expand Down Expand Up @@ -198,7 +225,14 @@ mod tests {
snapshot.insert(
make_node_record_key(node_id).into_bytes(), // key
encode_or_panic(&NodeRecord {
http: Some(ConnectionEndpoint::default()),
public_ipv4_config: Some(IPv4InterfaceConfig {
ip_addr: "127.0.0.1".to_string(),
..Default::default()
}),
http: Some(ConnectionEndpoint {
ip_addr: "2001:db8:3333:4444:5555:6666:7777:8888".to_string(),
..Default::default()
}),
domain: Some(domain.into()),
..Default::default()
}), // record
Expand All @@ -216,4 +250,72 @@ mod tests {
)
);
}

#[test]
fn test_check_api_boundary_node_ipv6_parsing_error() {
let mut snapshot = RegistrySnapshot::new();
let (node_id, domain) = (0, "example-1.com");
let node_id: NodeId = PrincipalId::new_node_test_id(node_id).into();

snapshot.insert(
make_api_boundary_node_record_key(node_id).into_bytes(), // key
encode_or_panic(&ApiBoundaryNodeRecord::default()), // record
);
snapshot.insert(
make_node_record_key(node_id).into_bytes(), // key
encode_or_panic(&NodeRecord {
public_ipv4_config: Some(IPv4InterfaceConfig {
ip_addr: "127.0.0.1".to_string(),
..Default::default()
}),
http: Some(ConnectionEndpoint {
ip_addr: "not_an_ipv6".to_string(),
..Default::default()
}),
domain: Some(domain.into()),
..Default::default()
}), // record
);

assert_eq!(
check_api_boundary_node_invariants(&snapshot)
.unwrap_err()
.msg,
"failed to parse ipv6 address of the node".to_string(),
);
}

#[test]
fn test_check_api_boundary_node_ipv4_parsing_error() {
let mut snapshot = RegistrySnapshot::new();
let (node_id, domain) = (0, "example-1.com");
let node_id: NodeId = PrincipalId::new_node_test_id(node_id).into();

snapshot.insert(
make_api_boundary_node_record_key(node_id).into_bytes(), // key
encode_or_panic(&ApiBoundaryNodeRecord::default()), // record
);
snapshot.insert(
make_node_record_key(node_id).into_bytes(), // key
encode_or_panic(&NodeRecord {
public_ipv4_config: Some(IPv4InterfaceConfig {
ip_addr: "not_an_ipv4".to_string(),
..Default::default()
}),
http: Some(ConnectionEndpoint {
ip_addr: "2001:db8:3333:4444:5555:6666:7777:8888".to_string(),
..Default::default()
}),
domain: Some(domain.into()),
..Default::default()
}), // record
);

assert_eq!(
check_api_boundary_node_invariants(&snapshot)
.unwrap_err()
.msg,
"failed to parse ipv4 address of the node".to_string(),
);
}
}

0 comments on commit c1a1563

Please sign in to comment.