Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binding DHCP server to a specific interface instead of 0.0.0.0 #361

Closed
bigzaqui opened this issue Nov 1, 2023 · 2 comments · Fixed by #362
Closed

Binding DHCP server to a specific interface instead of 0.0.0.0 #361

bigzaqui opened this issue Nov 1, 2023 · 2 comments · Fixed by #362
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. size/XS estimate of the amount of work to address the issue

Comments

@bigzaqui
Copy link

bigzaqui commented Nov 1, 2023

Hello folks,

some context: i am running tinkerbell on a server with 2 interfaces, one (eth0, ip 192.168.1.10) is in the private network, and the second one (eth1, ip 1.1.1.1) , is in an internet public facing network. the servers I want to provision are in the internal (eth0) network only.

Expected Behaviour

While testing boots 0.8.0 on the tinkerbell stack (run with docker-compose, similar to https://github.com/tinkerbell/sandbox/blob/main/deploy/stack/compose/docker-compose.yml), i noticed that if I change BOOTP_BIND from its default (: 0.0.0.0:67) to bind to the private interface ip (192.168.1.10:67), the DHCP requests (eg DHCPDISCOVER) broadcast are not replied by boots dhcp server. I'm not sure, but I think its because the IP.dst is not 192.168.1.10, so the kernel does not pass the package to boots:

IP: 0.0.0.0 (AA:AA:AA:AA:AA:AA) > 255.255.255.255 (ff:ff:ff:ff:ff:ff)

reading the code of boots 0.8.0, I noticed that the github.com/packethost/dhcp4-go package used had no way to specify the interface to listen from, only the addr attribute. But I see that now with smee 0.10.1 that dhcp4-go package is no longer used and instead, github.com/insomniacslk/dhcp/dhcpv4/server4 is used (via https://github.com/tinkerbell/dhcp) , and server4 has a way to specifiy the iface name on the first argument of NewIPv4UDPConn func (https://github.com/insomniacslk/dhcp/blob/6a2c8fbdcc1cc23250a45236cd7b968cb43d76e2/dhcpv4/server4/conn_unix.go#L19)

conn, err := server4.NewIPv4UDPConn("", net.UDPAddrFromAddrPort(bindAddr))

but currently, is hardcoded in smee to ""

Possible Solution

add a flag to specify the name of the interface to pass to NewIPv4UDPConn?

Context

I have boots listening on eth1 on a internet facing interface. is not ideal since I like to have all ports facing eth1 closed (except 22 for ssh), and I would like to close it.

Your Environment

Docker-compose running in Bare metal server

@jacobweinstock
Copy link
Member

Hey @bigzaqui , thanks for the request. PR #362 will enabled this via an env var and cli flag. Look right for your request?

@jacobweinstock jacobweinstock self-assigned this Nov 1, 2023
@jacobweinstock jacobweinstock added kind/feature Categorizes issue or PR as related to a new feature. size/XS estimate of the amount of work to address the issue labels Nov 1, 2023
@bigzaqui
Copy link
Author

bigzaqui commented Nov 1, 2023

thanks for checking this so fast @jacobweinstock ! , the PR lgtm except for this comment https://github.com/tinkerbell/smee/pull/362/files#r1378993737

@mergify mergify bot closed this as completed in #362 Nov 2, 2023
mergify bot added a commit that referenced this issue Nov 2, 2023
## Description


Users with multiple interfaces can limit listening to a single interface.

## Why is this needed



Fixes: #361 

## How Has This Been Tested?





## How are existing users impacted? What migration steps/scripts do we need?





## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. size/XS estimate of the amount of work to address the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants