Skip to content

Commit ff1f8ff

Browse files
johntmyersjohntmyers
authored
fix(network): align destination checks for direct connections (#5004)
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
1 parent 3313851 commit ff1f8ff

4 files changed

Lines changed: 461 additions & 42 deletions

File tree

‎crates/openshell-supervisor-network/src/policy_dns/mod.rs‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,10 @@ fn eligible_endpoints(
373373
continue;
374374
}
375375
let raw_allowed_ips = value_string_array(&endpoint.endpoint, "allowed_ips");
376-
let exact_declared_host = !pattern.contains('*') && pattern == name.as_str();
376+
let exact_declared_host = !pattern.contains('*')
377+
&& pattern == name.as_str()
378+
&& value_field(&endpoint.endpoint, "advisor_proposed")
379+
!= Some(&regorus::Value::Bool(true));
377380
let destination_plan = build_validation_plan(
378381
name.as_str(),
379382
name.as_str(),
@@ -1126,6 +1129,68 @@ process: { run_as_user: sandbox, run_as_group: sandbox }
11261129
assert!(matches!(result, Err(PolicyDnsError::NoValidAddress)));
11271130
}
11281131

1132+
#[tokio::test]
1133+
async fn advisor_endpoints_filter_private_answers_without_losing_public_or_explicit_ips() {
1134+
for protocol in [
1135+
"",
1136+
"tcp",
1137+
"rest, access: read-only",
1138+
"rest, access: read-only, tls: skip",
1139+
] {
1140+
for (family, private, public) in [
1141+
(AddressFamily::Ipv4, "10.2.3.4", "8.8.8.8"),
1142+
(AddressFamily::Ipv6, "fd12::4", "2001:4860:4860::8888"),
1143+
] {
1144+
let private = private.parse::<IpAddr>().unwrap();
1145+
let public = public.parse::<IpAddr>().unwrap();
1146+
let yaml = BASE_POLICY.replace(
1147+
"protocol: tcp",
1148+
&format!("protocol: {protocol}, advisor_proposed: true"),
1149+
);
1150+
let yaml = yaml.replace("protocol: , ", "");
1151+
let private_only = service(&yaml, vec![private]);
1152+
assert!(
1153+
matches!(
1154+
private_only
1155+
.answer_query("db.example", family, Instant::now())
1156+
.await,
1157+
Err(PolicyDnsError::NoValidAddress)
1158+
),
1159+
"{protocol}: {private}"
1160+
);
1161+
for addresses in [vec![public], vec![private, public]] {
1162+
let service = service(&yaml, addresses);
1163+
let now = Instant::now();
1164+
let answer = service
1165+
.answer_query("db.example", family, now)
1166+
.await
1167+
.unwrap();
1168+
let mapping = service
1169+
.store
1170+
.lookup(answer.address, 5432, answer.policy_generation, now)
1171+
.unwrap();
1172+
assert_eq!(mapping.record.contracts[0].pinned_addresses, [public]);
1173+
}
1174+
// Explicit narrow address permission retains its usual precedence.
1175+
let yaml = yaml.replace(
1176+
"advisor_proposed: true",
1177+
&format!("advisor_proposed: true, allowed_ips: ['{private}']"),
1178+
);
1179+
let service = service(&yaml, vec![private, public]);
1180+
let now = Instant::now();
1181+
let answer = service
1182+
.answer_query("db.example", family, now)
1183+
.await
1184+
.unwrap();
1185+
let mapping = service
1186+
.store
1187+
.lookup(answer.address, 5432, answer.policy_generation, now)
1188+
.unwrap();
1189+
assert_eq!(mapping.record.contracts[0].pinned_addresses, [private]);
1190+
}
1191+
}
1192+
}
1193+
11291194
#[tokio::test]
11301195
async fn allowed_ips_filters_each_answer_without_rejecting_usable_addresses() {
11311196
let yaml =

‎crates/openshell-supervisor-network/src/policy_dns/store.rs‎

Lines changed: 68 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,26 @@ pub(crate) struct MappingLookup {
8484
}
8585

8686
impl MappingLookup {
87-
pub(crate) fn pinned_addresses(&self) -> Vec<IpAddr> {
87+
/// Select only contracts authorized for this process and destination port.
88+
pub(crate) fn pinned_addresses_for(
89+
&self,
90+
endpoint_ids: &[PolicyEndpointId],
91+
) -> Result<Vec<IpAddr>, MappingLookupError> {
8892
let mut seen = HashSet::new();
89-
self.record
93+
let addresses = self
94+
.record
9095
.contracts
9196
.iter()
92-
.filter(|contract| contract.port == self.port)
97+
.filter(|contract| {
98+
contract.port == self.port && endpoint_ids.contains(&contract.endpoint_id)
99+
})
93100
.flat_map(|contract| contract.pinned_addresses.iter().copied())
94101
.filter(|address| seen.insert(*address))
95-
.collect()
102+
.collect::<Vec<_>>();
103+
if addresses.is_empty() {
104+
return Err(MappingLookupError::EndpointMismatch);
105+
}
106+
Ok(addresses)
96107
}
97108

98109
pub(crate) fn endpoint_ids(&self) -> impl Iterator<Item = &PolicyEndpointId> {
@@ -112,18 +123,7 @@ impl MappingLookup {
112123
&self,
113124
endpoint_id: &PolicyEndpointId,
114125
) -> Result<UpstreamConnector, MappingLookupError> {
115-
let mut seen = HashSet::new();
116-
let addresses = self
117-
.record
118-
.contracts
119-
.iter()
120-
.filter(|contract| contract.port == self.port && &contract.endpoint_id == endpoint_id)
121-
.flat_map(|contract| contract.pinned_addresses.iter().copied())
122-
.filter(|address| seen.insert(*address))
123-
.collect::<Vec<_>>();
124-
if addresses.is_empty() {
125-
return Err(MappingLookupError::EndpointMismatch);
126-
}
126+
let addresses = self.pinned_addresses_for(std::slice::from_ref(endpoint_id))?;
127127
let plan = build_pinned_validation_plan(addresses)
128128
.map_err(|_| MappingLookupError::InvalidMapping)?;
129129
validate_destination(DestinationRequest {
@@ -735,7 +735,10 @@ mod tests {
735735
.lookup_intent(observed.synthetic_address, 443, 1, now)
736736
.unwrap();
737737
assert_eq!(intent.record.normalized_name.as_str(), "unknown.example");
738-
assert!(intent.pinned_addresses().is_empty());
738+
assert!(matches!(
739+
intent.pinned_addresses_for(&[]),
740+
Err(MappingLookupError::EndpointMismatch)
741+
));
739742
assert!(matches!(
740743
store.lookup_intent(observed.synthetic_address, 443, 2, now),
741744
Err(MappingLookupError::StalePolicy)
@@ -1021,6 +1024,54 @@ mod tests {
10211024
assert_eq!(connector.addrs(), &["203.0.113.8:5432".parse().unwrap()]);
10221025
}
10231026

1027+
#[tokio::test]
1028+
async fn endpoint_selection_unions_authorized_contracts_only_and_binds_connector() {
1029+
let store = store(1);
1030+
let now = Instant::now();
1031+
let mut request = request("must-not-resolve.invalid", 1, Duration::from_secs(5));
1032+
let first = request.contracts[0].clone();
1033+
let mut second = first.clone();
1034+
second.endpoint_id.endpoint_index = 1;
1035+
second.pinned_addresses = vec!["203.0.113.9".parse().unwrap(), first.pinned_addresses[0]];
1036+
let mut unrelated = first.clone();
1037+
unrelated.endpoint_id.policy_name = "other-binary".to_string();
1038+
unrelated.pinned_addresses = vec!["10.2.3.4".parse().unwrap()];
1039+
let mut other_port = second.clone();
1040+
other_port.port = 443;
1041+
other_port.pinned_addresses = vec!["203.0.113.10".parse().unwrap()];
1042+
request
1043+
.contracts
1044+
.extend([second.clone(), unrelated.clone(), other_port]);
1045+
let record = store.publish(request, 1, now).unwrap();
1046+
let lookup = store
1047+
.lookup(record.synthetic_address, 5432, 1, now)
1048+
.unwrap();
1049+
assert_eq!(
1050+
lookup
1051+
.pinned_addresses_for(&[first.endpoint_id.clone(), second.endpoint_id])
1052+
.unwrap(),
1053+
[
1054+
"203.0.113.8".parse::<IpAddr>().unwrap(),
1055+
"203.0.113.9".parse().unwrap()
1056+
]
1057+
);
1058+
let connector = lookup.connector_for(&first.endpoint_id).await.unwrap();
1059+
assert_eq!(connector.addrs(), &["203.0.113.8:5432".parse().unwrap()]);
1060+
assert!(matches!(
1061+
lookup
1062+
.connector_for(&PolicyEndpointId {
1063+
policy_name: "absent".to_string(),
1064+
endpoint_index: 0
1065+
})
1066+
.await,
1067+
Err(MappingLookupError::EndpointMismatch)
1068+
));
1069+
assert!(matches!(
1070+
lookup.pinned_addresses_for(&[]),
1071+
Err(MappingLookupError::EndpointMismatch)
1072+
));
1073+
}
1074+
10241075
#[tokio::test]
10251076
async fn connector_preserves_resolver_address_order_while_deduplicating() {
10261077
let store = store(1);

0 commit comments

Comments
 (0)