hostIP: "127.0.0.1" shouldn't be hardcoded (to support exposing <nerdctl|docker|podman> run -p 0.0.0.0:80:80)

[AkihiroSuda]

Description

Currently <nerdctl|docker|podman> run -p 0.0.0.0:80:80 does not listen the port on host 0.0.0.0, because hostIP: "127.0.0.1" is hardcoded:

lima/pkg/limayaml/default.yaml

Lines 218 to 222 in 71861bc

	# # Lima internally appends this fallback rule at the end:
	# - guestIP: "127.0.0.1"
	# guestPortRange: [1, 65535]
	# hostIP: "127.0.0.1"
	# hostPortRange: [1, 65535]

We should probably use 0.0.0.0 when the guest uses 0.0.0.0.

The guest agent is already aware of "0.0.0.0" IP.

lima/pkg/guestagent/api/api.go

Lines 13 to 28 in 71861bc

	type IPPort struct {
	IP net.IP `json:"ip"`
	Port int `json:"port"`
	}
	func (x *IPPort) String() string {
	return net.JoinHostPort(x.IP.String(), strconv.Itoa(x.Port))
	}
	type Info struct {
	// LocalPorts contain 127.0.0.1 and 0.0.0.0.
	// LocalPorts do NOT contain addresses such as 127.0.0.53 and 192.168.5.15.
	//
	// In future, LocalPorts will contain IPv6 addresses (::1 and ::) as well.
	LocalPorts []IPPort `json:"localPorts"`
	}

Background:

• 3-part port binding for docker run or docker-compose is not respected, so services can be exposed on network abiosoft/colima#186 (comment)

cc @abiosoft

[AkihiroSuda]

This is a breaking change, and probably needs bumping up the major version.

[jandubois]

I don't understand why we need to change it; this is just being secure by default, which seems like a reasonable position to me.

You can always change it in your lima.yaml file yourself:

portForwards:
- guestPortRange: [1, 65535] 
  hostIP: "0.0.0.0" 

This is what Rancher Desktop (and I think colima) do because Docker Desktop does the same thing. This is already possible without changing the defaults.

It would be useful for a rule to match only when the guest binds to 0.0.0.0. We can't do that via guestIP because 0.0.0.0 already means any interface, but we could just add another property that would signal that, e.g.

portForwards:
- guestBindAll: true
  guestPortRange: [1, 65535] 
  hostIP: "0.0.0.0"

Please suggest a better name than guestBindAll. It would require an exact match, and is only valid when the guestIP is 0.0.0.0 (the default, so doesn't need to be specified).

[abiosoft]

Please suggest a better name than guestBindAll. It would require an exact match, and is only valid when the guestIP is 0.0.0.0 (the default, so doesn't need to be specified).

The name is not as bad as you make it sound :D.

It would be useful for a rule to match only when the guest binds to 0.0.0.0. We can't do that via guestIP because 0.0.0.0 already means any interface, but we could just add another property that would signal that, e.g.

This can work. To achieve forwarding to both 127.0.0.1 and 0.0.0.0 on the host, all that is needed to be done is an explicit config for both.

portForwards:
- guestPortRange: [1, 65535]
  hostIP: "127.0.0.1"
- guestPortRange: [1, 65535]
  guestBindAll: true 
  hostIP: "0.0.0.0"

This is a welcoming suggestion as it would no longer be a breaking change if guestBindAll defaults to false.

[jandubois]

when the guestIP is 0.0.0.0 (the default, so doesn't need to be specified).

That is wrong, the default is 127.0.0.1. But if the bind specifies 0.0.0.0, then it matches the rule because that also binds to 127.0.0.1.

So the rules need to be in the opposite order: we need to start with a rule that matches 0.0.0.0, and only 0.0.0.0, followed by a rule matching 0.0.0.0 as a wildcard for any interface (or leave it at 127.0.0.1, so other interfaces are not being forwarded):

portForwards:
- guestPortRange: [1, 65535]
  guestBindAll: true
  guestIP: "0.0.0.0"
  hostIP: "0.0.0.0"
- guestPortRange: [1, 65535]
  guestIP: "0.0.0.0"
  hostIP: "127.0.0.1"

I still don't like the name (it is not self-explanatory to me), but I struggle to find a good one. Maybe even guestIPExactMatch is better, I don't know. Maybe tomorrow I have better ideas. 😄

Maybe guestIPZero (and guestIP must not be specified in that case)?

I'm going to try to implement this, but I won't be able to finish it tonight.

[jandubois]

I'm going to try to implement this, but I won't be able to finish it tonight.

This was easier than expected (but then I haven't tested it yet): #660.

Still needs docs and tests, but if somebody has time to try it and provide feedback, that would be great. I'll finish it tomorrow!

[abiosoft]

I'm going to try to implement this, but I won't be able to finish it tonight.

This was easier than expected (but then I haven't tested it yet): #660.

Still needs docs and tests, but if somebody has time to try it and provide feedback, that would be great. I'll finish it tomorrow!

Tested and works as expected 🎉

Config

portForwards:
  - guestIPZero: true
    guestIP: 0.0.0.0
    guestPortRange: [1, 65536]
    hostIP: 0.0.0.0
    hostPortRange: [1, 65536]
  - guestIP: 127.0.0.1
    guestPortRange: [1, 65536]
    hostIP: 127.0.0.1
    hostPortRange: [1, 65536]

Execution

# 127.0.0.1
$ docker run --rm -d -p 127.0.0.1:8080:80 caddy
$ lsof -nP -iTCP:"8080" | grep LISTEN
ssh     50881 abiola   21u  IPv4 0xcfe12ce33b303c13      0t0  TCP 127.0.0.1:8080 (LISTEN)

# 0.0.0.0
$ docker run --rm -d -p 8080:80 caddy
$ lsof -nP -iTCP:"8080" | grep LISTEN
ssh     50881 abiola   20u  IPv4 0xcfe12ce33b569c13      0t0  TCP *:8080 (LISTEN)

[LionsAd]

Thanks - this looks great 👍

One consideration:

Maybe instead of a new property „*“ could be used in the same way as the lsof report shows to distinguish between 0.0.0.0 and 0.0.0.0 with guest ip zero set to true?

Besides that guestIPZero is easy enough to set.

[jandubois]

Maybe instead of a new property „*“ could be used in the same way as the lsof report shows to distinguish between 0.0.0.0 and 0.0.0.0 with guest ip zero set to true?

If we were starting from scratch, then guestIP: * would be great to specify the "any interface" option. Unfortunately I did not consider that we might need a literal match against 0.0.0.0 when doing the initial implementation, so that pattern has now the wildcard behaviour attached to it.

To maintain backwards compatibility, we cannot change this. So we don't need an alternative syntax to specify wildcarding, but rather the opposite. Adding a restricting rule qualifier is still the best I can think of.

I think I will go with guestIPMustBeZero instead of just guestIPZero and add additional documentation. Hopefully this will work for @AkihiroSuda; let's postpone further bikeshedding on this until the PR is no longer in draft status.

[AkihiroSuda]

guestIPMustBeZero

SGTM

[jandubois]

@AkihiroSuda When testing with nerdctl, it always seems to bind to 127.0.0.1. Even if I explicitly tell it to bind to 0.0.0.0:

nerdctl run -p 0.0.0.0:8080:80 --name nginx -d nginx

docker binds to 0.0.0.0 by default, but can be told to bind to something else. So you can choose to keep a port internal. With nerdctl you have to decide by the time the lima instance starts which ports you want to forward externally, and which ones you don't, which is less flexible.

Is this a bug in nerdctl or by design? At least it is an incompatibility to docker.

[AkihiroSuda]

nerdctl

Works for me, both rootless and rootful

$ nerdctl run -p 0.0.0.0:8080:80 --name nginx -d nginx
$ curl 192.168.5.15:8080
...
<title>Welcome to nginx!</title>
...

[abiosoft]

AkihiroSuda When testing with nerdctl, it always seems to bind to 127.0.0.1. Even if I explicitly tell it to bind to 0.0.0.0:

nerdctl run -p 0.0.0.0:8080:80 --name nginx -d nginx

I notice same behaviour and it seems to be specific to nerdctl on Alpine. When I tried with Ubuntu, I got the desired behaviour.

[jandubois]

@abiosoft I have a fix in #667; just working on a test, and making sure the test fails without the fix.

So the fix is currently reverted to make the test fail, but you can see the required code change at 105aee6

[abiosoft]

@abiosoft I have a fix in #667; just working on a test, and making sure the test fails without the fix.

So the fix is currently reverted to make the test fail, but you can see the required code change at 105aee6

I have tested it and can verify that the commit fixed it for nerdctl on Alpine.