Skip to content

daemon: disallow container port 0#51684

Merged
thaJeztah merged 1 commit into
moby:masterfrom
akerouanton:disallow-ctr-port-0
Dec 11, 2025
Merged

daemon: disallow container port 0#51684
thaJeztah merged 1 commit into
moby:masterfrom
akerouanton:disallow-ctr-port-0

Conversation

@akerouanton

@akerouanton akerouanton commented Dec 11, 2025

Copy link
Copy Markdown
Member

- What I did

Although container port 0 is invalid, it's currently accepted by the Engine. Users could mistakenly declare -p 0:0 and end up with a port mapping that does nothing. In that case, the Engine would allocate an ephemeral host port and create an iptables / nftables rule that DNAT to container port 0. This is obviously wrong.

Chain DOCKER (2 references)
 pkts bytes target     prot opt in     out     source               destination
    0     0 DNAT       6    --  !docker0 *       0.0.0.0/0            0.0.0.0/0            tcp dpt:32768 to:172.18.0.2:0

Instead of failing silently, return an error message to the API client when it calls the ContainerCreate endpoint.

- Human readable description for the release notes

Return an error when a container is created with a port-mapping pointing to container port 0.

@akerouanton
daemon: disallow container port 0
43780fe 
Signed-off-by: Albin Kerouanton <albin.kerouanton@docker.com>
@akerouanton akerouanton added this to the 29.2.0 milestone Dec 11, 2025
@akerouanton akerouanton self-assigned this Dec 11, 2025
@akerouanton akerouanton added area/networking Networking area/daemon Core Engine kind/bugfix PR's that fix bugs area/networking/portmapping Networking labels Dec 11, 2025
@thaJeztah

thaJeztah commented Dec 11, 2025

Copy link
Copy Markdown
Member

Although container port 0 is invalid, it's currently accepted by the Engine

Isn't 0 valid though? And a representation of "ephemeral port"? Or is Go the outlier here? https://go.dev/play/p/qgCeeKRAl88

@thaJeztah

thaJeztah commented Dec 11, 2025

Copy link
Copy Markdown
Member

This probably needs changes in the API change-log, and swagger (although we currently are not documenting the min/max values);

moby/api/swagger.yaml

Lines 4934 to 4951 in c64b781

EndpointPortConfig:
type: "object"
properties:
Name:
type: "string"
Protocol:
type: "string"
enum:
- "tcp"
- "udp"
- "sctp"
TargetPort:
description: "The port inside the container."
type: "integer"
PublishedPort:
description: "The port on the swarm hosts."
type: "integer"
PublishMode:

@akerouanton

akerouanton commented Dec 11, 2025

Copy link
Copy Markdown
Member Author

Isn't 0 valid though? And a representation of "ephemeral port"? Or is Go the outlier here?

It is, for the host port. I'm not changing the semantic of host port 0 here. There's no such thing as an ephemeral container port.

This probably needs changes in the API change-log, and swagger (although we currently are not documenting the min/max values);

The API docs says:

PortMap describes the mapping of container ports to host ports, using the container's port-number and protocol as key in the format /, for example, 80/udp.

If a container's port is mapped for multiple protocols, separate entries are added to the mapping table.

Container port 0 is invalid and doesn't match the current definition (because container port 0 doesn't even exist). That's also why this change isn't done for a specific API version.

@thaJeztah

thaJeztah commented Dec 11, 2025

Copy link
Copy Markdown
Member

I'm not changing the semantic of host port 0 here. T

Oh! You're right. That didn't sink in, and yes, that makes sense.

thaJeztah
thaJeztah approved these changes Dec 11, 2025
View reviewed changes

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah thaJeztah merged commit a11c3b9 into moby:master Dec 11, 2025
303 of 312 checks passed
@akerouanton akerouanton deleted the disallow-ctr-port-0 branch December 11, 2025 13:53
@vvoland vvoland mentioned this pull request Dec 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes
@vvoland vvoland vvoland approved these changes
@robmry robmry robmry approved these changes

Assignees

@akerouanton akerouanton

Labels

area/daemon Core Engine area/networking/portmapping Networking area/networking Networking impact/changelog kind/bugfix PR's that fix bugs process/cherry-picked

Projects

None yet

Milestone

29.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@akerouanton @thaJeztah @vvoland @robmry