Skip to content

Add support for unicast Reverse Path Forwarding (URPF) check. #1307

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add support for unicast Reverse Path Forwarding (URPF) check. #1307

wants to merge 7 commits into from

Conversation

rszarecki
Copy link
Contributor

Change Scope

  • This change introduces support for uRPF - the security techniqyes protecting against source addrress spoofing - described in:
    • rfc2827.
    • rfc3704.
    • rfc5635.
    • rfc8704
      The uRPF Protocol-independent, network-instance functionality with per ingress-interface configuration. This change adds following tree:
module: openconfig-interfaces
  +--rw interfaces
     +--rw interface* [name]
        +--rw subinterfaces
           +--rw subinterface* [index]
              +--rw oc-ip:ipv4
               | +--rw oc-ip:urpf
               |    +--rw oc-ip:config
               |    |  +--rw oc-ip:enabled?               boolean
               |    |  +--rw oc-ip:mode?                  oc-ni-types:urpf-mode
               |    |  +--rw oc-ip:allow-default-route?   boolean
               |    |  +--rw oc-ip:allow-drop-next-hop?   boolean
               |    |  +--rw oc-ip:allow-feasible-path?   boolean
               |    +--rw oc-ip:state
               |       +--rw oc-ip:enabled?               boolean
               |       +--rw oc-ip:mode?                  oc-ni-types:urpf-mode
               |       +--rw oc-ip:allow-default-route?   boolean
               |       +--rw oc-ip:allow-drop-next-hop?   boolean
               |       +--rw oc-ip:allow-feasible-path?   boolean
               |       +--rw oc-ip:counters
               |          +--rw oc-ip:urpf-drop-pkts?    oc-yang:counter64
               |          +--rw oc-ip:urpf-drop-bytes?   oc-yang:counter64
              +--rw oc-ip:ipv6
                 +--rw oc-ip:urpf
                    +--rw oc-ip:config
                    |  +--rw oc-ip:enabled?               boolean
                    |  +--rw oc-ip:mode?                  oc-ni-types:urpf-mode
                    |  +--rw oc-ip:allow-default-route?   boolean
                    |  +--rw oc-ip:allow-drop-next-hop?   boolean
                    |  +--rw oc-ip:allow-feasible-path?   boolean
                    +--rw oc-ip:state
                       +--rw oc-ip:enabled?               boolean
                       +--rw oc-ip:mode?                  oc-ni-types:urpf-mode
                       +--rw oc-ip:allow-default-route?   boolean
                       +--rw oc-ip:allow-drop-next-hop?   boolean
                       +--rw oc-ip:allow-feasible-path?   boolean
                       +--rw oc-ip:counters
                          +--rw oc-ip:urpf-drop-pkts?    oc-yang:counter64
                          +--rw oc-ip:urpf-drop-bytes?   oc-yang:counter64
  • Change is backward compatible

Platform Implementations

@rszarecki rszarecki requested a review from a team as a code owner May 13, 2025 02:41
@dplore
Copy link
Member

dplore commented May 13, 2025

/gcbrun

@dplore dplore moved this to Ready to discuss in OC Operator Review May 13, 2025
@OpenConfigBot
Copy link

OpenConfigBot commented May 13, 2025 •
edited
Loading

No major YANG version changes in commit ad19daa

@dplore
Copy link
Member

dplore commented May 13, 2025

/gcbrun

dplore
dplore reviewed May 13, 2025
View reviewed changes
release/models/interfaces/openconfig-if-ip.yang Outdated
}
leaf mode {
type urpf-mode;
default STRICT;
Copy link
Member

Choose a reason for hiding this comment

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

Here and throughout, we have an OC bias for new models against specifying defaults, unless this is defined by RFC. I recommend we remove the default and require the client to specify the node they want.

@dplore dplore moved this from Ready to discuss to Waiting for author in OC Operator Review May 13, 2025
@dplore
Copy link
Member

dplore commented May 13, 2025

/gcbrun

@dplore
Copy link
Member

dplore commented May 13, 2025

/gcbrun

dplore
dplore reviewed May 13, 2025
View reviewed changes
release/models/interfaces/openconfig-if-ip.yang
}
leaf allow-feasible-path {
type boolean;
default false;
Copy link
Member

Choose a reason for hiding this comment

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

In OC operators discussion it was observed this option is rarely supported so Rafal thought it reasonable to have a default.

However, IMO, it is less confusing and more consistent if we don't assign a default here and stick to a policy of only defining defaults when the feature specification requires a default.

release/models/interfaces/openconfig-if-ip.yang
@@ -1224,6 +1246,79 @@ revision "2023-06-30" {
}
}

grouping urpf-config {
description
"Grouping configuration coniguration of URPF";
Copy link
Member

Choose a reason for hiding this comment

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

nit: I recommend moving the reference statement from the counters up to the grouping here.

release/models/interfaces/openconfig-if-ip.yang
}
leaf mode {
type urpf-mode;
description
Copy link
Member

Choose a reason for hiding this comment

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

  when "../enabled = 'true'" {

release/models/interfaces/openconfig-if-ip.yang
Comment on lines +1262 to +1267
"The URPF in STRICT mode requires packet source address to LPM some route
in FIB, AND next-hop of this route must be interface packet was
received on.
The URPF in LOOSE mode requires packet source address to LPM some route
in FIB.
This leaf is irrelevalnt and should be ignored if URPF is not enabled.";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"The URPF in STRICT mode requires packet source address to LPM some route
in FIB, AND next-hop of this route must be interface packet was
received on.
The URPF in LOOSE mode requires packet source address to LPM some route
in FIB.
This leaf is irrelevalnt and should be ignored if URPF is not enabled.";
"The URPF in STRICT mode requires the ingress packet source address
to have a longest prefix match (LPM) for a route in the forwarding table
with a next-hop of the interface the packet was received on.
The URPF in LOOSE mode requires the ingress packet source address to
LPM a route in the forwarding table, but may have any next-hop.
This leaf is only valid if URPF is enabled.";

release/models/interfaces/openconfig-if-ip.yang
Comment on lines +1272 to +1274
"If set to false, and packets source address LPM the default route -
0.0.0.0/0 or ::/0 - URPF fails and packet is discarded, as if it would not
LPM any route.";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"If set to false, and packets source address LPM the default route -
0.0.0.0/0 or ::/0 - URPF fails and packet is discarded, as if it would not
LPM any route.";
"If set to false, and the packet's source address LPMs to the
default route (0.0.0.0/0 or ::/0) then the URPF check fails and the
packet is discarded.";

release/models/interfaces/openconfig-if-ip.yang
Comment on lines +1279 to +1281
"If set to false, and packets source address LPM the route with DROP
next-hop URPF fails and packet is discarded, as if it would not
LPM any route.";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"If set to false, and packets source address LPM the route with DROP
next-hop URPF fails and packet is discarded, as if it would not
LPM any route.";
"If set to false, and the packet's source address LPMs to a route
with DROP as the next-hop, then the URPF check fails and the packet
is discarded.";

release/models/interfaces/openconfig-if-ip.yang
Comment on lines +1287 to +1290
"The routing system may select subset of equally-good path as result of
tie-break (e.g. BGP RID) or ECMP width limits. If set to true, packet
are accepted if source address LPM to any equally-good path, even if it
is not selected for forwading.";
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand this description completely.

If the source address LPM matches an entry in a routing protocol RIB (BGP local RIB or ISIS RIB), but not in the forwarding table, it should pass the URPF check and forwarded? Is that that intention?

@rszarecki
Copy link
Contributor Author

rszarecki commented May 13, 2025 via email

dplore
dplore approved these changes May 14, 2025
View reviewed changes
release/models/interfaces/openconfig-if-ip.yang
Comment on lines +1287 to +1290
"The routing system may select subset of equally-good path as result of
tie-break (e.g. BGP RID) or ECMP width limits. If set to true, packet
are accepted if source address LPM to any equally-good path, even if it
is not selected for forwading.";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"The routing system may select subset of equally-good path as result of
tie-break (e.g. BGP RID) or ECMP width limits. If set to true, packet
are accepted if source address LPM to any equally-good path, even if it
is not selected for forwading.";
"The routing system may select subset of all learned paths. For example,
BGP without multi-path enabled will install only one of many possible ECMP
paths into the FIB or ECMP multipath limits may select only a subset of all
available paths. If set to true, the URPF check passes if the source address
LPMs to any path in the RIB, even if it is not selected for forwarding in the
FIB.";

Thanks for the more detailed explanation. I suggest adding this wording to
make the intent even more clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dplore dplore dplore approved these changes

Assignees
No one assigned
Labels
non-breaking
Projects
OC Operator Review
Status: Waiting for author
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rszarecki @dplore @OpenConfigBot