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

Sriram's edits and comments for verification draft v-20 #27

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ksriram25
Copy link
Contributor

@mitradir @cjeker @job @ksriram25

  1. Typos fixed.
  2. Minor text edits and wording improvements throughout.
  3. Inviting discussion about mutual-transit, Complex, impact on OTC implementation, etc. (see two comments marked with [sriram comment]

@mitradir @cjeker @job @ksriram25  

1. Typos fixed.
2. Minor text edits and wording improvements throughout.
3. Inviting discussion about mutual-transit, Complex, impact on OTC implementation, etc. (see two comments marked with [sriram comment]
@mitradir mitradir self-requested a review October 18, 2024 19:50
Copy link
Collaborator

@mitradir mitradir left a comment

Choose a reason for hiding this comment

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

Sriram, thank you for correcting the typos. There are several semantic mistakes, please correct them (or I can do it for you). My intention is to keep the wording around complex relations as simple as possible in accordance with RFC9234.

@@ -178,6 +178,12 @@
A special case of the Complex relation is mutual-transit when ASes MAY export everything (both customer and non-customer routes) to each other, i.e., customer-to-provider relationship applies in each direction.
All peering relationships are defined locally.
</t>
<t>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We had the case of mutual transit and complex relations in BGP Roles, which was carefully resolved. Why do we need here another and more complicated wording?

Copy link
Contributor Author

@ksriram25 ksriram25 Oct 19, 2024

Choose a reason for hiding this comment

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

@mitradir
By this wording in the draft about mutual-transit -- "i.e., customer-to-provider relationship applies in each direction. All peering relationships are defined locally" -- can you please clarify whether you mean that each AS labels (defines/configures) itself as Customer and considers the remote AS as Provider for the BGP session? After you clarify, I can offer further insights.

BTW, using the word "define" as in "All peering relationships are defined locally" isn't correct English. Instead "set locally" or "configured locally" should be used. "Define" means "describing the meaning" of a term.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The draft says:

For path verification purposes in this document, the peering
relationships an AS can have in relation to a neighbor AS are
Customer, Provider, Peer, Route Server (RS), RS-client, and Complex.
These peering relationships are defined in [RFC9234]. A special case
of the Complex relation is mutual-transit when ASes MAY export
everything (both customer and non-customer routes) to each other,
i.e., customer-to-provider relationship applies in each direction.

Following the link to RFC9234

If the local AS has more than one peering Role with the remote AS,
such a peering relation is called "Complex". An example is when the
peering relationship is Provider-to-Customer for some prefixes while
it is Peer-to-Peer for other prefixes [GAO-REXFORD].

In the case of mutual transit, both sides have peering relations Customer and Provider for all or a subset of prefixes.

This is followed by:

In the Complex relationship case (Section 3), an AS MUST include the
neighbor AS in its SPAS if neighbor plays Provider role for a subset
of received or sent prefixes. Each of the two ASes in a mutual-
transit pair MUST register its ASPA including the other AS in its
SPAS.

Tell me if you see any inconsistency.

@@ -334,27 +340,27 @@ authorized(AS x, AS y) = / Else, "Provider+" if the U-SPAS entry

<t>
Determine the maximum up-ramp length as I, where I is the minimum index for which authorized(A(I), A(I+1)) returns "Not Provider+".
If there is no such I, the maximum up-ramp length is set equal to the AS_PATH length N. This parameter is defined as max_up_ramp.
If there is no such I, the maximum up-ramp length is set equal to the AS_PATH length N. This parameter (maximum up-ramp length) is denoted by max_up_ramp.
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need double clarification in brackets?

Copy link
Contributor Author

@ksriram25 ksriram25 Oct 19, 2024

Choose a reason for hiding this comment

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

@mitradir
N and "I" (upper case I) and "maximum up-ramp length" are all parameters that were used in the preceding sentences. So, in the last sentence, it is not clear which of these is implied by "this parameter". That is why I put in the clarification to help to the reader. I have changed it a bit to the following:
The maximum up-ramp length is abbreviated as max_up_ramp.

@@ -394,7 +400,7 @@ authorized(AS x, AS y) = / Else, "Provider+" if the U-SPAS entry

<section title="Algorithm for Downstream Paths" anchor="Downflow">
<t>
The downstream verification algorithm described here is applied when a route is received from a Provider.
The downstream verification algorithm described here is applied when a route is received from a Provider. Here a transit provider, RS, and mutual-transit neighbor are all effectively instances of "Provider".
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not correct, a route received by the RS client must follow the upflow procedure. Check 7.2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mitradir
I put in RS by mistake. I have fixed this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see the reason to explicitly add mutual-transit in this part of the document either. The Complex case is covered in 8.3. I'm starting to think that removing any special mention of mutual-transit will improve the document readability.

@@ -466,8 +472,8 @@ authorized(AS x, AS y) = / Else, "Provider+" if the U-SPAS entry
If multiple eBGP sessions can segregate the Complex peering relationship into eBGP sessions with normal peering relationships the receiving/verifying AS SHOULD select the algorithm (per <xref target="Upflow"/> or <xref target="Downflow"/>) for each of the normal sessions based on its peering relation type.
</t>
<t>
If Complex peering relation can't be segregated (i.e., when a Complex BGP relationship occurs within one single BGP session), an operator may want to achieve an equivalent outcome by configuring policies on a per-prefix basis corresponding to the peering relation for the prefix.
If this option is not feasible, the operator MAY apply <xref target="Downflow"/>.
If a Complex peering relation cannot be segregated (i.e., when a Complex BGP relationship occurs within one single BGP session), an operator may want to achieve an equivalent outcome by applying an appropriate algorithm (<xref target="Upflow"/> or <xref target="Downflow"/>) on a per-prefix basis corresponding to the peering relation for the prefix.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is also a mistake. An operator may apply upflow procedure to a prefix if he knows that it is playing 'provider' and 'peer' role.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mitradir
Where is the mistake? My wording does not restrict it to one procedure. It says that "... applying an appropriate algorithm (Section 7.2 or Section 7.3)...". Aren't these two sections the upflow and downflow procedures, respectively?

While such attacks may happen, it does not seem to be a realistic scenario.
Normally a customer and their transit provider would have a signed agreement, and a policy violation (of the above kind) should have legal consequences or the customer can just drop the relationship with such a provider and remove the corresponding ASPA record.
An upstream provider might be able to propagate some instances of hijacked prefixes with forged-origin or forged-path-segment to a customer. While such attacks may happen, it does not seem to be a realistic scenario. Normally a customer and their transit provider would have a signed agreement, and a policy violation (of the above-mentioned kind) should have legal consequences or the customer can just drop the relationship with such a provider and remove the corresponding ASPA record.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you want to change this wording?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mitradir
v-19 text says "...such attacks might go undetected by its customers or upper providers." But the truth is that "hijacked prefixes with forged-origin or forged-path-segment" are detected by upper providers. Only customers cannot detect that. I feel that we do not need to mention other path manipulations here without saying what they are. Anyway, in the next section (9.3), we do talk about other path manipulations in the form of altering AS prepends.

I changed the section title (9.2) to "Attack by a Provider" from "Provider as Trusted Point" because it is not true that the provider is always considered as a trusted point by a customer in the draft procedures. Just that the customer is not able to detect some forms of attacks by the provider while it can detect other forms of attacks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But the truth is that "hijacked prefixes with forged-origin or forged-path-segment" are detected by upper providers.

They are not if a Provider prepends customer AS.

@ksriram25 ksriram25 changed the title Sriram's edits and comments for v-20 Sriram's edits and comments for verification draft v-20 Oct 19, 2024
@mitradir
Copy link
Collaborator

mitradir commented Nov 3, 2024

Sriram,

I incorporated the typos you found and removed mutual transit since it was creating unnecessary ambiguity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants