ipoe: implement some rfc3046 specifications #106
Conversation
This file contains hidden or 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
RFC3046 says: > The Relay Agent Information option [82] echoed by a server MUST be removed > by either the relay agent or the trusted downstream network element > which added it when forwarding a server-to-client response back to > the client. 985cb9f ("ipoe: dhcpv4: echo back opt82 if sent by client/unknown relay per rfc3046") was supposed to implement this requirement. However, it does not work because dhcpv4_relay_send() modifies the original incoming request before relaying it. Fix the requirement conformity by working on a copy of the original request in dhcpv4_relay_send(). Tested with the dhtest tool with and without an option 82: > ./dhtest -i ens3 > ./dhtest -c 82,hex,010876786c312e31313102104147454e542d52454d4f54452d49443105040a0f4002 -i ens3 Fixes: 985cb9f ("ipoe: dhcpv4: echo back opt82 if sent by client/unknown relay per rfc3046") Link: https://www.rfc-editor.org/rfc/rfc3046.html#section-2.1 Link: https://github.com/saravana815/dhtest Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
RFC3046 distinguishes "trusted" and "untrusted" circuits from which a DHCP request can come: > The mechanisms for distinguishing between "trusted" and "untrusted" > circuits are specific to the type of circuit termination equipment, > and may involve local administration. For example, a Cable Modem > Termination System may consider upstream packets from most cable > modems as "untrusted", but an ATM switch terminating VCs switched > through a DSLAM may consider such VCs as "trusted" and accept a relay > agent option added by the DSLAM. Add a global and per interface trusted-circuit configuration option to define whether the agent information, such as the giaddr and the option 82, in incoming DHCP requests should be trusted. Security will be enforced on next commits on untrusted interfaces. Default the value to 1 (trust) to keep the current behavior. Link: https://www.rfc-editor.org/rfc/rfc3046.html#section-2.1 Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
DHCP clients must send DHCP requests with a giaddr set to 0. giaddr must be only set by a relay agent. Discard client to server DHCP packets with a non zero giaddr received on an interface on which the agent information value are untrusted. Tested with: > ./dhtest --giaddr 10.66.252.247 -i ens3 Got: > Aug 31 17:36:12 DHCP-RELAY accel-pppd[345]: recv discarded - giaddr recvd on untrusted circuit [DHCPv4 Discover xid=40690d12 giaddr=10.66.252.247 chaddr=0c:be:37:a4:00:00 <Message-Type Discover> <Request-List Subnet,Broadcast,Router,Domain-Name,DNS>] Link: https://www.rfc-editor.org/rfc/rfc2131#page-37 Link: https://github.com/saravana815/dhtest Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
RFC3046 about the DHCP option 82 says:
> Relay agents receiving a DHCP packet from an untrusted circuit with
> giaddr set to zero (indicating that they are the first-hop router)
> but with a Relay Agent Information option already present in the
> packet SHALL discard the packet and increment an error count.
Discard client to server DHCP packets containing the option 82 received
on an interface on which the agent information value are untrusted.
Tested with:
> ./dhtest -c 82,hex,010876786c312e31313102104147454e542d52454d4f54452d49443105040a0f4002 -i ens3
Got:
> Aug 31 17:40:17 DHCP-RELAY accel-ppp[345]: recv discarded - option 82 recvd on untrusted circuit [DHCPv4 Discover xid=59e1ef0e chaddr=0c:be:37:a4:00:00 <Message-Type Discover> <Request-List Subnet,Broadcast,Router,Domain-Name,DNS> <Relay-Agent {Agent-Circuit-ID vxl1.111} {Agent-Remote-ID AGENT-REMOTE-ID1} {Link-Selection 10.15.64.2}>]
Link: https://www.rfc-editor.org/rfc/rfc3046.html#section-2.1
Link: https://github.com/saravana815/dhtest
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
RFC3046 says: > Relay agents configured to add a Relay Agent option which receive a > client DHCP packet with a nonzero giaddr SHALL discard the packet if > the giaddr spoofs a giaddr address implemented by the local agent > itself. Discard DHCP incoming DHCP requests that spoofs giaddr if the Relay Agent option 82 is configured. Tested with a spoofed giaddr and trusted-circuit=1 in config: > ./dhtest --giaddr 10.66.252.247 -i ens3 Link: https://www.rfc-editor.org/rfc/rfc3046.html#section-2.1.1 Link: https://github.com/saravana815/dhtest Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
RFC3046 says: > [...] the relay agent SHALL forward any received DHCP packet > with a valid non-zero giaddr WITHOUT adding any relay agent options. Do not add relay agent options 82 if giaddr is set. Tested with a spoofed giaddr, link-selection=1.1.1.1 and trusted-circuit=1 in config: > ./dhtest --giaddr 10.66.252.247 -i ens3 Link: https://www.rfc-editor.org/rfc/rfc3046.html#section-2.1.1 Link: https://github.com/saravana815/dhtest Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
RFC3046 says: > [...] the relay agent SHALL forward any received DHCP packet > with a valid non-zero giaddr WITHOUT adding any relay agent options. > Per RFC 2131, it shall also NOT modify the giaddr value. Do not replace a valid giaddr. Tested with a spoofed giaddr and trusted-circuit=1 in config: > ./dhtest --giaddr 10.66.252.247 -i ens3 Link: https://www.rfc-editor.org/rfc/rfc3046.html#section-2.1.1 Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
54d229a to
54341ec
Compare
Add trusted-circuit documentation Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
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 b
30AA
ecause 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.
Add rfc3046 optional specifications about relay agent. In bold, RFC mandatory
Also contain the #105 fix