Skip to content

Commit

Permalink
coredns: allow host lookup of names
Browse files Browse the repository at this point in the history 
In my rework it seems I broke the weird way how we tried to resolve
names when we have no source ip match. Let's do this properly and move
the logic all into lookup() which makes it much cleaner and no
duplication needed.

Now there is catch here, we only lookup the network for the current
network listener, so if the user has multiple networks they always must
target the requests against the correct ip depeding on what network the
name is in. But this is the same as it worked before and seems safer
than allowing a unknown ip to access all names I guess.

Fixes containers#508

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
@Luap99
Luap99 committed Sep 20, 2024
1 parent fc08648 commit 5a21a4e
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 41 deletions.
30 changes: 20 additions & 10 deletions src/backend/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,12 @@ impl DNSBackend {

// Handle a single DNS lookup made by a given IP.
// Returns all the ips for the given entry name
pub fn lookup(&self, requester: &IpAddr, entry: &str) -> Option<Vec<IpAddr>> {
pub fn lookup(
&self,
requester: &IpAddr,
network_name: &str,
entry: &str,
) -> Option<Vec<IpAddr>> {
// Normalize lookup entry to lowercase.
let mut name = entry.to_lowercase();

Expand All @@ -70,9 +75,21 @@ impl DNSBackend {
name.truncate(name.len() - self.search_domain.len())
}

// if this is a fully qualified name, remove dots so backend can perform search
if name.ends_with(".") {
name.truncate(name.len() - 1)
}

let owned_netns: Vec<String>;

let nets = match self.ip_mappings.get(requester) {
Some(n) => n,
None => return None,
// no source ip found let's just allow access to the current network where the request was made
// On newer rust versions in CI we can return &vec![network_name.to_string()] directly without the extra assignment to the outer scope
None => {
owned_netns = vec![network_name.to_string()];
&owned_netns
}
};

let mut results: Vec<IpAddr> = Vec::new();
Expand All @@ -85,14 +102,7 @@ impl DNSBackend {
continue;
}
};
// if this is a fully qualified name, remove dots so backend can perform search
if !name.is_empty() {
if let Some(lastchar) = name.chars().last() {
if lastchar == '.' {
name = (name[0..name.len() - 1]).to_string();
}
}
}

if let Some(addrs) = net_names.get(&name) {
results.append(&mut addrs.clone());
}
Expand Down
23 changes: 5 additions & 18 deletions src/dns/coredns.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,26 +456,13 @@ fn reply_ip<'a>(
src_address: SocketAddr,
req: &'a mut Message,
) -> Option<&'a Message> {
let mut resolved_ip_list: Vec<IpAddr> = Vec::new();
// attempt intra network resolution
match backend.lookup(&src_address.ip(), name) {
let resolved_ip_list = match backend.lookup(&src_address.ip(), network_name, name) {
// If we go success from backend lookup
Some(ips) => {
resolved_ip_list = ips;
}
// For everything else assume the src_address was not in ip_mappings
None => {
debug!("No backend lookup found, try resolving in current resolvers entry");
if let Some(container_mappings) = backend.name_mappings.get(network_name) {
if let Some(ips) = container_mappings.get(name) {
resolved_ip_list.clone_from(ips);
}
}
}
}
if resolved_ip_list.is_empty() {
return None;
}
Some(ips) => ips,
None => return None,
};

if record_type == RecordType::A {
for record_addr in resolved_ip_list {
if let IpAddr::V4(ipv4) = record_addr {
Expand Down
26 changes: 13 additions & 13 deletions src/test/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ mod tests {
let res = parse_configs("src/test/config/podman")
.expect("parse config error")
.0
.lookup(&IP_10_88_0_2, "condescendingnash");
.lookup(&IP_10_88_0_2, "", "condescendingnash");
assert_eq!(res, Some(vec![IP_10_88_0_2]));
}
#[test]
Expand All @@ -165,7 +165,7 @@ mod tests {
let res = parse_configs("src/test/config/podman")
.expect("parse config error")
.0
.lookup(&IP_10_88_0_2, "helloworld");
.lookup(&IP_10_88_0_2, "", "helloworld");
assert_eq!(res, Some(vec![IP_10_88_0_5]));
}
#[test]
Expand All @@ -179,7 +179,7 @@ mod tests {
let res = parse_configs("src/test/config/podman")
.expect("parse config error")
.0
.lookup(&IP_10_88_0_2, "HELLOWORLD");
.lookup(&IP_10_88_0_2, "", "HELLOWORLD");
assert_eq!(res, Some(vec![IP_10_88_0_5]));
}
#[test]
Expand All @@ -189,7 +189,7 @@ mod tests {
let res = parse_configs("src/test/config/podman")
.expect("parse config error")
.0
.lookup(&IP_10_88_0_2, "somebadquery");
.lookup(&IP_10_88_0_2, "", "somebadquery");
assert_eq!(res, None);
}
#[test]
Expand All @@ -202,7 +202,7 @@ mod tests {
let res = parse_configs("src/test/config/podman")
.expect("parse config error")
.0
.lookup(&IP_10_88_0_2, "trustingzhukovsky");
.lookup(&IP_10_88_0_2, "", "trustingzhukovsky");
assert_eq!(res, Some(vec![IP_10_88_0_4]));
}
#[test]
Expand All @@ -215,7 +215,7 @@ mod tests {
let res = parse_configs("src/test/config/podman")
.expect("parse config error")
.0
.lookup(&IP_10_88_0_2, "ctr1");
.lookup(&IP_10_88_0_2, "", "ctr1");
assert_eq!(res, Some(vec![IP_10_88_0_4]));
}
#[test]
Expand All @@ -228,9 +228,9 @@ mod tests {
assert!(conf.1.contains_key("podman_v6_entries"));
assert!(!conf.2.contains_key("podman_v6_entries"));

let ips = conf.0.lookup(&IP_10_89_0_2, "test1");
let ips = conf.0.lookup(&IP_10_89_0_2, "", "test1");
assert_eq!(ips, Some(vec![IP_10_89_0_2, IP_FDFD_733B_DC3_220B_2]));
let ips = conf.0.lookup(&IP_FDFD_733B_DC3_220B_2, "test1");
let ips = conf.0.lookup(&IP_FDFD_733B_DC3_220B_2, "", "test1");
assert_eq!(ips, Some(vec![IP_10_89_0_2, IP_FDFD_733B_DC3_220B_2]));
}
#[test]
Expand All @@ -244,9 +244,9 @@ mod tests {
assert!(conf.1.contains_key("podman_v6_entries"));
assert!(!conf.2.contains_key("podman_v6_entries"));

let ips = conf.0.lookup(&IP_10_89_0_2, "test2");
let ips = conf.0.lookup(&IP_10_89_0_2, "", "test2");
assert_eq!(ips, Some(vec![IP_10_89_0_3, IP_FDFD_733B_DC3_220B_3]));
let ips = conf.0.lookup(&IP_FDFD_733B_DC3_220B_2, "test2");
let ips = conf.0.lookup(&IP_FDFD_733B_DC3_220B_2, "", "test2");
assert_eq!(ips, Some(vec![IP_10_89_0_3, IP_FDFD_733B_DC3_220B_3]));
}
#[test]
Expand All @@ -261,7 +261,7 @@ mod tests {
assert!(conf.1.contains_key("podman_v6_entries"));
assert!(!conf.2.contains_key("podman_v6_entries"));

let ips = conf.0.lookup(&IP_10_89_0_2, "88dde8a24897");
let ips = conf.0.lookup(&IP_10_89_0_2, "", "88dde8a24897");
assert_eq!(ips, Some(vec![IP_10_89_0_3, IP_FDFD_733B_DC3_220B_3]));
}
/* -------------------------------------------- */
Expand Down Expand Up @@ -440,7 +440,7 @@ mod tests {
"fddd::1".parse().unwrap()
]
);
match backend.lookup(&"10.0.0.2".parse().unwrap(), "testmulti1") {
match backend.lookup(&"10.0.0.2".parse().unwrap(), "", "testmulti1") {
Some(ip_vec) => {
assert_eq!(
ip_vec,
Expand All @@ -455,7 +455,7 @@ mod tests {
_ => panic!("unexpected dns result"),
}

match backend.lookup(&"10.0.0.2".parse().unwrap(), "testmulti2") {
match backend.lookup(&"10.0.0.2".parse().unwrap(), "", "testmulti2") {
Some(ip_vec) => {
assert_eq!(
ip_vec,
Expand Down
11 changes: 11 additions & 0 deletions test/200-two-networks.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ load helpers
# b1 should be able to resolve itself
dig "$b1_pid" "bone" "$b_gw"
assert $b1_ip

# we should be able to resolve a from the host if we use the a gw as server
run_in_host_netns dig +short "aone" "@$a_gw"
assert $a1_ip
# but NOT when using b as server
expected_rc=1 run_in_host_netns "host" "-t" "ns" "aone" "$b_gw"
assert "$output" =~ "Host aone not found"
assert "$output" =~ "NXDOMAIN"
# but b on network b is allowed again
run_in_host_netns dig +short "bone" "@$b_gw"
assert $b1_ip
}

@test "two subnets with isolated container and one shared" {
Expand Down

0 comments on commit 5a21a4e

Please sign in to comment.