Fix connect to host and prevent disconnect from host for host network #17476

coolljt0725 · 2015-10-29T07:26:12Z

There are two commits in this PR.
The first one is to fix connect to host. If a container start with default network(bridge) or none
it can't be connect to host as desired. but if disconnect the container from bridge or none, you can
connect to host, but this will cause some problem.

$ docker run -ti --rm --name container1 busybox
$ docker network disconnect bridge container1
$ docker network connect host container1

we able to connect to host after the container disconnect from bridge
but actually, the container still use its own network namespace, docker inspect still show its own private sandbox the container use and ifconfig in the container still the information of its own private network namespace.
From what I understand the container has private network namespace cannot connect to other network namespace, https://github.com/docker/docker/blob/master/daemon/container_unix.go#L742 is supposed to do thar but may not go there because container.NetworkSettings.Networks could be nil ,correct me if I'm wrong :) @mavenugo

The second commit is to prevent from disconnecting from host for a container start with --net=host. if a container with --net=host disconnect from host, it still in the host network namespace, but able to connect to other network, and cause some problem.


$ docker run -ti --rm --net=host --name container1 busybox
$ docker network disconnect host container1
$ docker network connect bridge container1
Error response from daemon: failed to set gateway while updating gateway: network is unreachable

though connect to bridge failed, but it has create a eth0 network interface on my host

eth0: flags=4163  mtu 1500
        inet 172.17.0.2  netmask 255.255.0.0  broadcast 0.0.0.0
        inet6 fe80::42:acff:fe11:2  prefixlen 64  scopeid 0x20
        ether 02:42:ac:11:00:02  txqueuelen 0  (Ethernet)
        RX packets 8  bytes 648 (648.0 B)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 10  bytes 732 (732.0 B)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0

and docker inspect show the endpoint information

"Networks": {
            "bridge": {
                "EndpointID": "ddc8cd9e738f9255fc3288c82d77adf6d226ed1ec0e1c8000935b0269e39080a",
                "Gateway": "",
                "IPAddress": "172.17.0.2",
                "IPPrefixLen": 16,
                "IPv6Gateway": "",
                "GlobalIPv6Address": "",
                "GlobalIPv6PrefixLen": 0,
                "MacAddress": "02:42:ac:11:00:02"
            }
        }

So I think we should prevent user from disconnect from host
ping @tiborvass

tiborvass · 2015-11-04T19:07:32Z

Ping @mavenugo @mrjana

mrjana · 2015-11-04T19:22:13Z

daemon/container_unix.go

@@ -728,7 +728,9 @@ func (container *Container) updateNetworkSettings(n libnetwork.Network) error {
 	if container.NetworkSettings == nil {
 		container.NetworkSettings = &network.Settings{Networks: make(map[string]*network.EndpointSettings)}
 	}
-
+	if !container.hostConfig.NetworkMode.IsHost() && runconfig.NetworkMode(n.Type()).IsHost() {


We should not be needing this extra check. The check for IsPrivate() should disallow connecting to host. Are you able to connect to host after the container is started on a private network?

@mrjana yes, I able to connect to host if I disconnect the container with private network from bridge

docker run -d --name c1 busybox top docker network disconnect bridge c1 docker network connect host c1 docker inspect c1 .... "Networks": { "host": { "EndpointID": "6b09beacac0470bed008ee62c55f4161701c64d5a7ccaddd8a155eb0c779bf5f", "Gateway": "", "IPAddress": "", "IPPrefixLen": 0, "IPv6Gateway": "", "GlobalIPv6Address": "", "GlobalIPv6PrefixLen": 0, "MacAddress": "" } }

because if we disconnect it from bridge, the "NetworkSettings" is nil and the check below for IsPrivate() has no chance to run

mrjana · 2015-11-04T19:25:00Z

@coolljt0725 I am ok with the design. But had a comment on the code changes. Can you please clarify?

When you try to connect a container to host, which is already connected to a private network, container.NetworkSettings.Networks cannot be nil

mrjana · 2015-11-04T19:25:56Z

@coolljt0725 Also it needs a rebase

coolljt0725 · 2015-11-05T08:16:03Z

rebased

thaJeztah · 2015-11-05T10:48:10Z

@mrjana @tiborvass is this a 1.9.1 fix?

mrjana · 2015-11-05T19:19:58Z

@thaJeztah Yes

mrjana · 2015-11-05T19:21:15Z

LGTM

cpuguy83 · 2015-11-05T20:32:04Z

Moving to code review since it's got @mrjana's blessing.

cpuguy83 · 2015-11-07T20:45:41Z

daemon/container_unix.go

@@ -720,6 +720,10 @@ func (daemon *Daemon) updateNetworkSettings(container *Container, n libnetwork.N
 		container.NetworkSettings = &network.Settings{Networks: make(map[string]*network.EndpointSettings)}
 	}

+	if !container.hostConfig.NetworkMode.IsHost() && runconfig.NetworkMode(n.Type()).IsHost() {
+		return runconfig.ErrConflictSharedNetwork


How come this is ErrConflictSharedNetwork and the other one is ErrConflictHostNetwork when they are both host?

This is to prevent user from connecting to host and the other is prevent user from disconnecting from host

But shouldn't these both be ErrConflictHostNetwork?

Both be ErrConflictHostNetwork is also make sense, just need to modify the description of ErrConflictHostNetwork, I'll update

coolljt0725 · 2015-11-08T07:00:48Z

@cpuguy83 updated

coolljt0725 · 2015-11-09T01:04:30Z

@cpuguy83 updated

cpuguy83 · 2015-11-09T01:06:00Z

LGTM

Container has private network namespace can not to connect to host and container with host network can not be disconnected from host. Signed-off-by: Lei Jitang <leijitang@huawei.com>

calavera · 2015-11-09T19:14:16Z

LGTM.

Unfortunately, this won't merge cleanly in the 1.9 release branch. @tiborvass should we open a new PR against that branch with a similar fix?

calavera · 2015-11-10T17:48:30Z

It looks like this is okay for the release, no merged conflicts.

Merging!

Fix connect to host and prevent disconnect from host for host network

GordonTheTurtle added the status/0-triage label Oct 29, 2015

calavera added status/2-code-review area/networking status/1-design-review and removed status/0-triage status/2-code-review labels Oct 31, 2015

mrjana reviewed Nov 4, 2015
View reviewed changes

coolljt0725 force-pushed the prevent_connect_to_host branch from f28e6d5 to 04baf0d Compare November 5, 2015 07:57

cpuguy83 added status/2-code-review and removed status/1-design-review labels Nov 5, 2015

thaJeztah added this to the 1.9.1 milestone Nov 6, 2015

cpuguy83 reviewed Nov 7, 2015
View reviewed changes

coolljt0725 force-pushed the prevent_connect_to_host branch from 04baf0d to 75e5648 Compare November 8, 2015 06:58

coolljt0725 force-pushed the prevent_connect_to_host branch from 75e5648 to 8140ca3 Compare November 9, 2015 01:04

coolljt0725 force-pushed the prevent_connect_to_host branch from 8140ca3 to e04435e Compare November 9, 2015 01:07

Prevent connecting to host and prevent disconnecting from host

a2d8c93

Container has private network namespace can not to connect to host and container with host network can not be disconnected from host. Signed-off-by: Lei Jitang <leijitang@huawei.com>

coolljt0725 force-pushed the prevent_connect_to_host branch from e04435e to a2d8c93 Compare November 9, 2015 02:21

calavera added a commit that referenced this pull request Nov 10, 2015

Merge pull request #17476 from coolljt0725/prevent_connect_to_host

470fc94

Fix connect to host and prevent disconnect from host for host network

calavera merged commit 470fc94 into moby:master Nov 10, 2015

calavera added status/5-cherry-pick and removed status/2-code-review labels Nov 10, 2015

tiborvass added impact/changelog and removed status/5-cherry-pick labels Nov 12, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix connect to host and prevent disconnect from host for host network #17476

Fix connect to host and prevent disconnect from host for host network #17476

coolljt0725 commented Oct 29, 2015

tiborvass commented Nov 4, 2015

mrjana Nov 4, 2015

coolljt0725 Nov 5, 2015

mrjana commented Nov 4, 2015

mrjana commented Nov 4, 2015

coolljt0725 commented Nov 5, 2015

thaJeztah commented Nov 5, 2015

mrjana commented Nov 5, 2015

mrjana commented Nov 5, 2015

cpuguy83 commented Nov 5, 2015

cpuguy83 Nov 7, 2015

coolljt0725 Nov 8, 2015

cpuguy83 Nov 8, 2015

coolljt0725 Nov 9, 2015

coolljt0725 commented Nov 8, 2015

coolljt0725 commented Nov 9, 2015

cpuguy83 commented Nov 9, 2015

calavera commented Nov 9, 2015

calavera commented Nov 10, 2015

Fix connect to host and prevent disconnect from host for host network #17476

Fix connect to host and prevent disconnect from host for host network #17476

Conversation

coolljt0725 commented Oct 29, 2015

tiborvass commented Nov 4, 2015

mrjana Nov 4, 2015

Choose a reason for hiding this comment

coolljt0725 Nov 5, 2015

Choose a reason for hiding this comment

mrjana commented Nov 4, 2015

mrjana commented Nov 4, 2015

coolljt0725 commented Nov 5, 2015

thaJeztah commented Nov 5, 2015

mrjana commented Nov 5, 2015

mrjana commented Nov 5, 2015

cpuguy83 commented Nov 5, 2015

cpuguy83 Nov 7, 2015

Choose a reason for hiding this comment

coolljt0725 Nov 8, 2015

Choose a reason for hiding this comment

cpuguy83 Nov 8, 2015

Choose a reason for hiding this comment

coolljt0725 Nov 9, 2015

Choose a reason for hiding this comment

coolljt0725 commented Nov 8, 2015

coolljt0725 commented Nov 9, 2015

cpuguy83 commented Nov 9, 2015

calavera commented Nov 9, 2015

calavera commented Nov 10, 2015