libnet: Don't forward to upstream resolvers on internal nw #47538

robmry · 2024-03-08T16:20:46Z

- What I did

CVE-2024-29018

Picked up Albin's change from #46609 - and added a regression test.

Commit cbc2a71 makes connect syscall fail fast when a container is only attached to an internal network. Thanks to that, if such a container tries to resolve an "external" domain, the embedded resolver returns an error immediately instead of waiting for a timeout.

This commit makes sure the embedded resolver doesn't even try to forward to upstream servers - this is important when the host's DNS server is running on a localhost address. In this case, the internal resolver's upstream DNS requests are made from the host's network namespace, so they work even when the network is declared as 'internal'. Communication with external DNS servers is unexpected for an internal network.

- How Albin did it

Add a way to enable/disable upstream forwarding to the embedded resolver. When an endpoint joins/leaves a sandbox, this forwarding policy is modified based on whether there's an endpoint providing external connectivity.

- How to verify it

New regression test.

- Description for the changelog

Do not forward requests to external DNS servers for a container that is only connected to an 'internal' network. Previously, requests were forwarded if the host's DNS server was running on a localhost address, like systemd's 127.0.0.53.

- A picture of a cute animal (not mandatory but encouraged)

neersighted

LGTM, with one missing comment.

neersighted · 2024-03-14T14:17:59Z

libnetwork/sandbox_dns_unix.go

-		// an internal network. This way, it's the driver responsibility to make sure `connect` syscall fails fast when
-		// no external connectivity is available (eg. by not setting a default gateway).
-		sb.resolver = NewResolver(resolverIPSandbox, true, sb)
+		sb.resolver = NewResolver(resolverIPSandbox, sb.getGatewayEndpoint() != nil, sb)


Let's add a comment here that explains the proxyDNS/internal relationship.

That's done, thank you.

corhere

LGTM, comments aside

integration/networking/resolvconf_test.go

libnetwork/resolver.go

Commit cbc2a71 makes `connect` syscall fail fast when a container is only attached to an internal network. Thanks to that, if such a container tries to resolve an "external" domain, the embedded resolver returns an error immediately instead of waiting for a timeout. This commit makes sure the embedded resolver doesn't even try to forward to upstream servers. Co-authored-by: Albin Kerouanton <albinker@gmail.com> Signed-off-by: Rob Murray <rob.murray@docker.com>

corhere

LGTM, thanks!

akerouanton

LGTM. Thanks for moving this forward.

DerDakon · 2024-03-26T07:10:51Z

Shouldn't the GHSA-mq39-4gv4-mvpx package/version list also refer to moby/moby? It's currently only talking about docker.

neersighted · 2024-03-26T07:22:29Z

The name of the Go codebase in this repo is github.com/docker/docker. While the upstream repository is moby/moby, we have not yet completed a migration to the new name, which remains the source of truth for the codebase here.

since moby/moby#47538, dns isn't forward automaticlly into internal networks, and we need to create a specfic network that has a gatway, for it to work as we expect and forward DNS requests out Fixes: scylladb#7302 Ref: moby/moby#47538

DerDakon · 2024-03-26T12:22:53Z

The name of the Go codebase in this repo is github.com/docker/docker. While the upstream repository is moby/moby, we have not yet completed a migration to the new name, which remains the source of truth for the codebase here.

Correct. But as I read it both packages are equally affected, right? So both should be listed in the advisory, otherwise moby would not be matched by automated tools.

I.e. I'm confused that e.g. GHSA-rc4r-wh2q-q6c4 uses moby/moby but this one does not.

neersighted · 2024-03-26T12:30:14Z

There is only one package; github.com/docker/docker is a redirect to github.com/moby/moby. As far as Go is concerned, there is only one package, as relative imports do not exist.

That GHSA has an incorrect Go package identified, and should be corrected: github/advisory-database#4144.

since moby/moby#47538, dns isn't forward automaticlly into internal networks, and we need to create a specfic network that has a gatway, for it to work as we expect and forward DNS requests out Fixes: scylladb#7302 Ref: moby/moby#47538

DerDakon · 2024-03-26T13:45:02Z

There is only one package; github.com/docker/docker is a redirect to github.com/moby/moby. As far as Go is concerned, there is only one package, as relative imports do not exist.

That GHSA has an incorrect Go package identified, and should be corrected: github/advisory-database#4144.

Ok, thanks for the clarification. Here are some more:

neersighted · 2024-03-26T13:52:54Z

It sounds like we'll have to reach out to GitHub's security team regarding how they editorialize advisories. Thanks for bringing this to our attention!

since moby/moby#47538, dns isn't forward automaticlly into internal networks, and we need to create a specfic network that has a gatway, for it to work as we expect and forward DNS requests out Fixes: #7302 Ref: moby/moby#47538

since moby/moby#47538, dns isn't forward automaticlly into internal networks, and we need to create a specfic network that has a gatway, for it to work as we expect and forward DNS requests out Fixes: #7302 Ref: moby/moby#47538 (cherry picked from commit b23e17a)

neersighted · 2024-04-06T15:17:52Z

#47662 is open to track this regression; please 👍 there instead of commenting on related issues, or adding a "me too."

jsoriano · 2024-04-10T19:32:19Z

Hey, I see this change was backported to 25.0 and 23.0, is 24.0 not affected by this issue?

akerouanton · 2024-04-11T05:31:30Z

23.0 is the long-term support version that Mirantis bundle in their Container Runtime. IIRC it's also the version currently used by Azure. They're both maintaining it because it's used in their products.
Since 26.0 was a fresh new major version when the security fix was released, we (at Docker) decided to backport it to 25.0 to not force Engine users to migrate to 26.0 right away. At this point and AFAIK we (Docker Inc.) don't plan to release any new 25.0 patch release, and I think neither Mirantis nor Microsoft will make it their LTS version.
24.0 is EOL since no company (ie. neither Docker, Mirantis nor Microsoft) have interest into maintaining it. It's affected but unmaintained and thus won't receive the security patch.

jsoriano · 2024-04-11T06:42:00Z

@akerouanton thanks for the clarification!

since moby/moby#47538, dns isn't forward automaticlly into internal networks, and we need to create a specfic network that has a gatway, for it to work as we expect and forward DNS requests out Fixes: #7302 Ref: moby/moby#47538 (cherry picked from commit b23e17a)

robmry requested review from corhere and neersighted March 8, 2024 16:21

robmry self-assigned this Mar 8, 2024

robmry added kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. area/networking Networking area/networking/dns Networking labels Mar 8, 2024

robmry added this to the 26.0.0 milestone Mar 8, 2024

vvoland modified the milestones: 26.0.0, 27.0.0 Mar 14, 2024

robmry force-pushed the libnet-resolver-nxdomain branch from 3a5a6c3 to 999a014 Compare March 14, 2024 11:57

robmry marked this pull request as ready for review March 14, 2024 11:59

vvoland modified the milestones: 27.0.0, 26.0.0 Mar 14, 2024

neersighted approved these changes Mar 14, 2024

View reviewed changes

robmry force-pushed the libnet-resolver-nxdomain branch from 999a014 to 13d4546 Compare March 14, 2024 14:43

corhere requested changes Mar 14, 2024

View reviewed changes

integration/networking/resolvconf_test.go Show resolved Hide resolved

libnetwork/resolver.go Outdated Show resolved Hide resolved

robmry force-pushed the libnet-resolver-nxdomain branch from 13d4546 to 790c303 Compare March 14, 2024 17:47

corhere approved these changes Mar 14, 2024

View reviewed changes

thaJeztah added process/cherry-pick/23.0 process/cherry-pick/25.0 labels Mar 14, 2024

akerouanton approved these changes Mar 18, 2024

View reviewed changes

neersighted merged commit 641e341 into moby:master Mar 18, 2024

vvoland mentioned this pull request Mar 19, 2024

[25.0 backport] libnet: Don't forward to upstream resolvers on internal nw #47589

Merged

vvoland removed the process/cherry-pick/25.0 label Mar 19, 2024

corhere mentioned this pull request Mar 19, 2024

[23.0 backport] libnet: Don't forward to upstream resolvers on internal nw #47593

Merged

corhere added process/cherry-picked and removed process/cherry-pick/23.0 labels Mar 19, 2024

neersighted mentioned this pull request Mar 20, 2024

libnet: Don't forward to upstream resolvers on internal nw #46609

Closed

corhere mentioned this pull request Mar 21, 2024

Windows DNS resolver forwarding #47584

Merged

fruch mentioned this pull request Mar 26, 2024

[YCSB][alternaor-dns][GCP] Temporary failure in name resolution scylladb/scylla-cluster-tests#7302

Closed

2 tasks

fruch mentioned this pull request Mar 26, 2024

fix(alternator-dns): move ycsb runs into it's own docker network scylladb/scylla-cluster-tests#7304

Merged

3 tasks

This was referenced Mar 26, 2024

[chore] bump docker deps to v25.0.5 carvel-dev/imgpkg#648

Merged

[chore] bump docker/docker, docker/cli to v25.0.5 carvel-dev/vendir#370

Merged

robmry deleted the libnet-resolver-nxdomain branch March 27, 2024 16:32

robmry mentioned this pull request Apr 3, 2024

IPVLAN L3 is not forwarding non Docker DNS requests outside of the container anymore #47662

Closed

This was referenced Apr 11, 2024

chore(deps): bump github.com/docker/docker from 24.0.7+incompatible to 24.0.9+incompatible helm/helm#12899

Merged

Update docker dependencies to 26.1 helm/helm#12939

Closed

kaanyalti mentioned this pull request Apr 15, 2024

upgrade opentelemetry-collector-contrib to v0.98.0 elastic/elastic-agent#4572

Merged

7 tasks

robmry mentioned this pull request May 10, 2024

Forward DNS requests into --internal networks #47821

Merged

vvoland mentioned this pull request May 14, 2024

[26.1 backport] Forward DNS requests into --internal networks #47832

Merged

libnet: Don't forward to upstream resolvers on internal nw #47538

libnet: Don't forward to upstream resolvers on internal nw #47538

Conversation

robmry commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neersighted left a comment

Choose a reason for hiding this comment

Uh oh!

neersighted Mar 14, 2024

Choose a reason for hiding this comment

Uh oh!

robmry Mar 14, 2024

Choose a reason for hiding this comment

Uh oh!

corhere left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

corhere left a comment

Choose a reason for hiding this comment

Uh oh!

akerouanton left a comment

Choose a reason for hiding this comment

Uh oh!

DerDakon commented Mar 26, 2024

Uh oh!

neersighted commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DerDakon commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neersighted commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DerDakon commented Mar 26, 2024

Uh oh!

neersighted commented Mar 26, 2024

Uh oh!

neersighted commented Apr 6, 2024

Uh oh!

jsoriano commented Apr 10, 2024

Uh oh!

akerouanton commented Apr 11, 2024

Uh oh!

jsoriano commented Apr 11, 2024

Uh oh!

Uh oh!

robmry commented Mar 8, 2024 •

edited

Loading

neersighted commented Mar 26, 2024 •

edited

Loading

DerDakon commented Mar 26, 2024 •

edited

Loading

neersighted commented Mar 26, 2024 •

edited

Loading