Remove trusted proxies around auto.ipxe: #363
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move from using IP auth via the source IP address to using the MAC address in the URL in order to determine the Hardware to lookup when serving the auto.ipxe. This opens up the auto.ipxe script to be able to be served across network address translation (NAT) layers and simplifies the deployment. Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
Improved usage and description. Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
It wasn't working as the env vars weren't correct. Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
Added the new CLI help usage and updated the section on running Smee manually. Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
Unexport functions that don't need to be public. Removed an unused interface and the GetByIP function. Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
README.md
Outdated
| -http-addr [http] local IP:Port to listen on for iPXE HTTP script requests (default "172.17.0.2:80") | ||
| -http-ipxe-binary-enabled [http] enable iPXE HTTP binary server (default "true") | ||
| -http-ipxe-script-enabled [http] enable iPXE HTTP script server (default "true") | ||
| -osie-url [http] URL where OSIE(Hook) images are located |
There was a problem hiding this comment.
Suggested change
| -osie-url [http] URL where OSIE(Hook) images are located | |
| -osie-url [http] URL where OSIE (Hook) images are located |
Is it Hook or HookOS?
cmd/smee/flag.go
Outdated
| @@ -114,7 +113,8 @@ func dhcpFlags(c *config, fs *flag.FlagSet) { | |||
| fs.StringVar(&c.dhcp.syslogIP, "dhcp-syslog-ip", detectPublicIPv4(""), "[dhcp] syslog server IP address to use in DHCP packets (opt 7)") | |||
| fs.StringVar(&c.dhcp.tftpIP, "dhcp-tftp-ip", detectPublicIPv4(":69"), "[dhcp] tftp server IP address to use in DHCP packets (opt 66, etc)") | |||
| fs.StringVar(&c.dhcp.httpIpxeBinaryURL, "dhcp-http-ipxe-binary-url", "http://"+detectPublicIPv4(":8080/ipxe/"), "[dhcp] HTTP ipxe binaries URL to use in DHCP packets") | |||
| fs.StringVar(&c.dhcp.httpIpxeScriptURL, "dhcp-http-ipxe-script-url", "http://"+detectPublicIPv4("/auto.ipxe"), "[dhcp] HTTP ipxe script URL to use in DHCP packets") | |||
| fs.StringVar(&c.dhcp.httpIpxeScript.url, "dhcp-http-ipxe-script-url", "http://"+detectPublicIPv4("/auto.ipxe"), "[dhcp] HTTP ipxe script URL to use in DHCP packets") | |||
| fs.BoolVar(&c.dhcp.httpIpxeScript.injectMacAddress, "dhcp-http-ipxe-script-prepend-mac", true, "[dhcp] prepend the hardware mac address to ipxe script URL base, http://1.2.3.4/auto.ipxe -> http://1.2.3.4/40:15:ff:89:cc:0e/auto.ipxe") | |||
There was a problem hiding this comment.
It looks like we're removing the IP mechanism so what happens when this is set to false?
There was a problem hiding this comment.
The mac addreess will not be included in the http ipxe script url. Setting this to false is useful when you are not using the auto.ipxe script in Smee. I have a comment over the struct field in main for c.dhcp.httpIpxeScript.injectMacAddress
There was a problem hiding this comment.
Thanks for the clarification.
cmd/smee/flag.go
Outdated
| @@ -114,7 +113,8 @@ func dhcpFlags(c *config, fs *flag.FlagSet) { | |||
| fs.StringVar(&c.dhcp.syslogIP, "dhcp-syslog-ip", detectPublicIPv4(""), "[dhcp] syslog server IP address to use in DHCP packets (opt 7)") | |||
| fs.StringVar(&c.dhcp.tftpIP, "dhcp-tftp-ip", detectPublicIPv4(":69"), "[dhcp] tftp server IP address to use in DHCP packets (opt 66, etc)") | |||
| fs.StringVar(&c.dhcp.httpIpxeBinaryURL, "dhcp-http-ipxe-binary-url", "http://"+detectPublicIPv4(":8080/ipxe/"), "[dhcp] HTTP ipxe binaries URL to use in DHCP packets") | |||
| fs.StringVar(&c.dhcp.httpIpxeScriptURL, "dhcp-http-ipxe-script-url", "http://"+detectPublicIPv4("/auto.ipxe"), "[dhcp] HTTP ipxe script URL to use in DHCP packets") | |||
| fs.StringVar(&c.dhcp.httpIpxeScript.url, "dhcp-http-ipxe-script-url", "http://"+detectPublicIPv4("/auto.ipxe"), "[dhcp] HTTP ipxe script URL to use in DHCP packets") | |||
| fs.BoolVar(&c.dhcp.httpIpxeScript.injectMacAddress, "dhcp-http-ipxe-script-prepend-mac", true, "[dhcp] prepend the hardware mac address to ipxe script URL base, http://1.2.3.4/auto.ipxe -> http://1.2.3.4/40:15:ff:89:cc:0e/auto.ipxe") | |||
There was a problem hiding this comment.
Suggested change
| fs.BoolVar(&c.dhcp.httpIpxeScript.injectMacAddress, "dhcp-http-ipxe-script-prepend-mac", true, "[dhcp] prepend the hardware mac address to ipxe script URL base, http://1.2.3.4/auto.ipxe -> http://1.2.3.4/40:15:ff:89:cc:0e/auto.ipxe") | |
| fs.BoolVar(&c.dhcp.httpIpxeScript.injectMacAddress, "dhcp-http-ipxe-script-prepend-mac", true, "[dhcp] prepend the hardware MAC address to ipxe script URL base, http://1.2.3.4/auto.ipxe -> http://1.2.3.4/40:15:ff:89:cc:0e/auto.ipxe") |
? I also noticed ipxe and tftp (probably more) are lower cased. That feels inconsistent against the various HTTPs, DHCPs etc.
There was a problem hiding this comment.
thanks, I've updated all these.
Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
chrisdoherty4
approved these changes
Nov 3, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This moves from using IP auth via the source IP address and the trusted proxies cli flag/env var to using the MAC address in the URL in order to determine the Hardware to lookup when serving the auto.ipxe. For example:
http://192.168.2.3/40:15:ff:89:cc:0e/auto.ipxeThis opens up the
auto.ipxescript to be able to be served across network address translation (NAT) layers and simplifies the deployment by not having to deal with the X-Forwarded-For (XFF) header.Why is this needed
Fixes: #
How Has This Been Tested?
How are existing users impacted? What migration steps/scripts do we need?
Checklist:
I have: