Skip to content

Commit dfbcddb

Browse files
robmryrobmry
committed
Fix incorrect validation of port mapping
Regression introduced in 01eecb6. A port mapping from a specific IPv6 host address can be used by a container on an IPv4-only network, docker-proxy makes the connection. Signed-off-by: Rob Murray <[email protected]>
1 parent ff1e2c0 commit dfbcddb

3 files changed

Lines changed: 76 additions & 15 deletions

File tree

‎integration/networking/bridge_test.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"net"
7+
"net/http"
78
"os/exec"
89
"regexp"
910
"runtime"
@@ -1055,3 +1056,38 @@ func TestPortMappedHairpin(t *testing.T) {
10551056
defer c.ContainerRemove(ctx, res.ContainerID, containertypes.RemoveOptions{Force: true})
10561057
assert.Check(t, is.Contains(res.Stderr.String(), "404 Not Found"))
10571058
}
1059+
1060+
// Check that a container on an IPv4-only network can have a port mapping
1061+
// from a specific IPv6 host address (using docker-proxy).
1062+
// Regression test for https://github.com/moby/moby/issues/48067 (which
1063+
// is about incorrectly reporting this as invalid config).
1064+
func TestProxy4To6(t *testing.T) {
1065+
skip.If(t, testEnv.DaemonInfo.OSType == "windows", "uses bridge network and docker-proxy")
1066+
skip.If(t, testEnv.IsRootless)
1067+
1068+
ctx := setupTest(t)
1069+
d := daemon.New(t)
1070+
d.StartWithBusybox(ctx, t)
1071+
defer d.Stop(t)
1072+
1073+
c := d.NewClientT(t)
1074+
defer c.Close()
1075+
1076+
const netName = "ipv4net"
1077+
network.CreateNoError(ctx, t, c, netName)
1078+
1079+
serverId := container.Run(ctx, t, c,
1080+
container.WithNetworkMode(netName),
1081+
container.WithExposedPorts("80"),
1082+
container.WithPortMap(nat.PortMap{"80": {{HostIP: "::1"}}}),
1083+
container.WithCmd("httpd", "-f"),
1084+
)
1085+
defer c.ContainerRemove(ctx, serverId, containertypes.RemoveOptions{Force: true})
1086+
1087+
inspect := container.Inspect(ctx, t, c, serverId)
1088+
hostPort := inspect.NetworkSettings.Ports["80/tcp"][0].HostPort
1089+
1090+
resp, err := http.Get("http://[::1]:" + hostPort)
1091+
assert.NilError(t, err)
1092+
assert.Check(t, is.Equal(resp.StatusCode, 404))
1093+
}

‎libnetwork/drivers/bridge/port_mapping_linux.go‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ func (n *bridgeNetwork) addPortMappings(
6060
}
6161

6262
disableNAT4, disableNAT6 := n.getNATDisabled()
63-
if err := validatePortBindings(cfg,
64-
!disableNAT4 && len(containerIPv4) > 0,
65-
!disableNAT6 && len(containerIPv6) > 0); err != nil {
63+
if err := validatePortBindings(cfg, !disableNAT4, !disableNAT6, containerIPv6); err != nil {
6664
return nil, err
6765
}
6866

@@ -157,27 +155,32 @@ const validationErrLimit = 6
157155
// docker-proxy is used to forward between the default IPv6 address and the
158156
// container's IPv4. So, simply disallowing a non-zero IPv6 default when NAT6
159157
// is disabled for the network would be incorrect.)
160-
func validatePortBindings(pbs []types.PortBinding, nat4, nat6 bool) error {
158+
func validatePortBindings(pbs []types.PortBinding, nat4, nat6 bool, cIPv6 net.IP) error {
161159
var errs []error
162160
for i := range pbs {
163161
pb := &pbs[i]
164162
disallowHostPort := false
165163
if !nat4 && len(pb.HostIP) > 0 && pb.HostIP.To4() != nil && !pb.HostIP.Equal(net.IPv4zero) {
166164
// There's no NAT4, so don't allow a nonzero IPv4 host address in the mapping. The port will
167-
// accessible via any host interface.
165+
// be accessible via any host interface.
168166
errs = append(errs,
169167
fmt.Errorf("NAT is disabled, omit host address in port mapping %s, or use 0.0.0.0::%d to open port %d for IPv4-only",
170168
pb, pb.Port, pb.Port))
171169
// The mapping is IPv4-specific but there's no NAT4, so a host port would make no sense.
172170
disallowHostPort = true
173171
} else if !nat6 && len(pb.HostIP) > 0 && pb.HostIP.To4() == nil && !pb.HostIP.Equal(net.IPv6zero) {
174-
// There's no NAT6, so don't allow an IPv6 host address in the mapping. The port will
175-
// accessible via any host interface.
176-
errs = append(errs,
177-
fmt.Errorf("NAT is disabled, omit host address in port mapping %s, or use [::]::%d to open port %d for IPv6-only",
178-
pb, pb.Port, pb.Port))
179-
// The mapping is IPv6-specific but there's no NAT6, so a host port would make no sense.
180-
disallowHostPort = true
172+
// If the container has no IPv6 address, the userland proxy will proxy between the
173+
// host's IPv6 address and the container's IPv4. So, even with no NAT6, it's ok for
174+
// an IPv6 port mapping to include a specific host address or port.
175+
if len(cIPv6) > 0 {
176+
// There's no NAT6, so don't allow an IPv6 host address in the mapping. The port will
177+
// accessible via any host interface.
178+
errs = append(errs,
179+
fmt.Errorf("NAT is disabled, omit host address in port mapping %s, or use [::]::%d to open port %d for IPv6-only",
180+
pb, pb.Port, pb.Port))
181+
// The mapping is IPv6-specific but there's no NAT6, so a host port would make no sense.
182+
disallowHostPort = true
183+
}
181184
} else if !nat4 && !nat6 {
182185
// There's no NAT, so it would make no sense to specify a host port.
183186
disallowHostPort = true

‎libnetwork/drivers/bridge/port_mapping_linux_test.go‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ func TestValidatePortBindings(t *testing.T) {
186186
name string
187187
nat4 bool
188188
nat6 bool
189+
ctrIPv6 net.IP
189190
pbs []types.PortBinding
190191
expErrs []string
191192
}{
@@ -196,7 +197,8 @@ func TestValidatePortBindings(t *testing.T) {
196197
},
197198
},
198199
{
199-
name: "no nat with addrs",
200+
name: "no nat with addrs",
201+
ctrIPv6: newIPNet(t, "fd2c:b48c:69fb::2/128").IP,
200202
pbs: []types.PortBinding{
201203
{Proto: types.TCP, HostIP: newIPNet(t, "233.252.0.2/24").IP, Port: 80},
202204
{Proto: types.TCP, HostIP: newIPNet(t, "2001:db8::2/64").IP, Port: 80},
@@ -246,7 +248,16 @@ func TestValidatePortBindings(t *testing.T) {
246248
},
247249
},
248250
{
249-
name: "no nat and addrs and ports",
251+
name: "nat4 and addrs and ports",
252+
nat4: true,
253+
pbs: []types.PortBinding{
254+
{Proto: types.TCP, HostIP: newIPNet(t, "233.252.0.2/24").IP, HostPort: 8080, Port: 80},
255+
{Proto: types.TCP, HostIP: newIPNet(t, "2001:db8::2/64").IP, HostPort: 8080, Port: 80},
256+
},
257+
},
258+
{
259+
name: "no nat and addrs and ports",
260+
ctrIPv6: newIPNet(t, "fd2c:b48c:69fb::2/128").IP,
250261
pbs: []types.PortBinding{
251262
{Proto: types.TCP, HostIP: newIPNet(t, "233.252.0.2/24").IP, HostPort: 8080, Port: 80},
252263
{Proto: types.TCP, HostIP: newIPNet(t, "2001:db8::2/64").IP, HostPort: 8080, Port: 80},
@@ -258,6 +269,17 @@ func TestValidatePortBindings(t *testing.T) {
258269
"host port must not be specified in mapping [2001:db8::2]:8080:80/tcp because NAT is disabled",
259270
},
260271
},
272+
{
273+
name: "no nat no ctrIPv6 and addrs and ports",
274+
pbs: []types.PortBinding{
275+
{Proto: types.TCP, HostIP: newIPNet(t, "233.252.0.2/24").IP, HostPort: 8080, Port: 80},
276+
{Proto: types.TCP, HostIP: newIPNet(t, "2001:db8::2/64").IP, HostPort: 8080, Port: 80},
277+
},
278+
expErrs: []string{
279+
"NAT is disabled, omit host address in port mapping 233.252.0.2:8080:80/tcp, or use 0.0.0.0::80 to open port 80 for IPv4-only",
280+
"host port must not be specified in mapping 233.252.0.2:8080:80/tcp because NAT is disabled",
281+
},
282+
},
261283
{
262284
name: "max errs reached",
263285
pbs: []types.PortBinding{
@@ -283,7 +305,7 @@ func TestValidatePortBindings(t *testing.T) {
283305
for _, tc := range testcases {
284306
tc := tc
285307
t.Run(tc.name, func(t *testing.T) {
286-
err := validatePortBindings(tc.pbs, tc.nat4, tc.nat6)
308+
err := validatePortBindings(tc.pbs, tc.nat4, tc.nat6, tc.ctrIPv6)
287309
if tc.expErrs == nil {
288310
assert.Check(t, err)
289311
} else {

0 commit comments

Comments
 (0)