Re: [Sidrops] Feedback for draft-ietf-sidrops-aspa-verification-11

["Sriram, Kotikalapudi (Fed)" <[e-mail]>]

Hi Claudio,

Thanks for your thorough review and very helpful comments. We (authors) appreciate it. Glad to hear that you found [sriram1] helpful for understanding the algorithms described in the draft.   

Your suggestions for changes are absolutely on the mark. Those are the kind of improvements that I had on mind for the next version. The draft had legacy text that needed some rewriting. Obviously, it still needs some more work for improving the readability.  

Since you found [sriram1] helpful, I can possibly use some of that notation and style of algorithm description in the draft as well. I will work on generating a version-12 in the next few weeks. Your comments are clear, and I plan to take each of them into consideration while updating.

No comments inline below. Will share inline comments after the draft is updated.   

Sriram 

---------------------------------------------------
From: Claudio Jeker <[e-mail]> Wed, 02 November 2022 17:23 UTC

Hi Sidrops and draft authors,

Here some feedback to draft-ietf-sidrops-aspa-verification-11 now
that I started to implement this for OpenBGPD.

Section 1 & 2
There is a big bit missing in the introduction which gives an
overview what ASPA does. This section would also explain some of
the technical bits and jargon used in this draft.
Especially explain how downstream/upstream checks work. I found the
presentation in [sriram1] important to understand the concept of
ASPA verification. Without that presentation I think I would not
correctly understand many things in this draft.

Section 4

   A relying party (RP) must have access to a local cache of the
   complete set of cryptographically valid ASPAs when performing the
   customer-provider verification procedure.

and


   The following algorithm describes the customer-provider verification
   procedure for a selected AFI:

   1.  Retrieve all cryptographically valid ASPAs with the selected AFI
       that have a customer value of AS1.  The union of SPAS from these
       ASPAs forms the set of authorized providers.


What has an RP to do with the code running on a BGP router.  Also
the Customer-Provider Verification Procedure is the core lookup
function. There should be no step 1. instead there should be a
concept similar to VRPs. IMO the above texts should be in section
3. Maybe mention that either static config or RTR can be used to
distribute verified ASPA sets and that all sets are merged together.

Also I think using the terms "Unknown", "Valid", "Invalid" is
inappropriate when it comes to customer-provider lookups. Mainly
because "Invalid" is misleading. "Valid" should be "Provider" and
"Invalid" should be "Not-Provider". I also accept other terms but
mixing Valid/Invalid here with the use in Section 5 (and the final
outcome of the ASPA lookup is confusing).  The C-P lookup does not
define if a path is valid or not. It just tells if there is a C-P
relation or not (or it is unknown).


Section 5

Is the enforce neighbor-as a proper MUST or is this just la la la
nice to have? From my understanding it is an important bit of not
allowing empty or wrong paths in.
In OpenBGPD enforce neighbor-as is on by default but it can be
turned off.  So should this be enforced for customer, provider, rs
and peer sessions?  Also on rs-client session enforce-as becomes
more complex (see below).

Section 5.1

Please just take all of this behind the barn and start over.
The text is a mess, it is not how the code should be implemented.
It is confusing and needs a lot of mental gymnastics to follow the
text. This part is crucial to properly implement ASPA.

Using the same I everywhere is confusing (also I is a very bad letter
in text) and then this monstrosity
   "AS(1),AS(2),..., AS(I-1), AS(I), AS(I+1),..., AS(N)"
does not help either. It has no real value (especially the repetition).
Also I really dislike that it is the wrong way around. The path is
actually AS(N) ...  AS(1) since AS(1) is the origin AS. More about
this further below.

The calculation of the reverse indexes is only required for provider
sessions (local role customer). Also it would be nice to not depend
on reversing the AS path but instead explain that one can start from
AS(N) and backtrack selecting the maximum instead of minimum and
then use N - 'found index' as the result.

Also note that Section 5.1.1 adds injury to insult by just confusing
even more about what the heck is going on.
The text should just mention that for Non-Transparent RS sessions the
RS ASN should be skipped in the ASPATH and only the reminder checked
as defined in Section 5.1.
Also detecting this case will be fun: will it be enough to depend on
'role rs-client' and 'enforce neighbor-as yes'? An alternative is to
have 'role rs-client non-transparent' as config.

I have doubts that this non-transparent IXP check works in real life.
While it does work for the immediate rs-client it requires that the
non-transparent IXP is listed as provider in all its rs-clients
ASPA objects. I guess this should be listed in Section 5.4. At least
currently there is nothing like that mentioned in the draft.

Section 5.3

Second paragraph (the left and right are confusing especially since
it is the reverse of what you normally get (ASPA reversed the ASPATH
and so everything is the wrong way). The left-most 'AS(1)' is the
origin AS (which is normally the right-most AS) and right-most
'AS(N)' is the neighbor-as (which is normally the left-most AS).

   If there is an upstream ramp but no downstream ramp or vice versa,
   then clearly the UPDATE is valid (i.e., not a route leak).  However,
   if both ramps exist, then the UPDATE is Valid if and only if either
   one or zero AS hops exist between the apexes of the two ramps, i.e.,
   there is no AS between the apexes (see [sriram1] for formal proof).
   If there are one or more ASes between the apexes of the upstream and
   downstream ramps, then the UPDATE is a route leak (Invalid) or the
   presence of a leak cannot be known using available ASPAs (Unknown)
   [sriram1].

I find 'only if either one or zero AS hops exist ... i.e., there
is no AS between the apexes' confusing. If zero AS hops exist
then the upstream and downstream ramp touch (both end on the same
node). Now they can also overlap but the text totally ignores this
here. If there is one AS hop then the two apexes are neighbors
(AS(up apex + 1) = AS(down apex). Without context from looking at
sriram1 most of this is too hard to understand.

Section 5.4

This section is in the wrong spot. It feels more like something
that belongs under 7.


In general the text in Section 5 is too complex and I fear there
will be off by one errors. Without the presentation from [sriram1]
I would have a hard time to implement this with confidence of doing
it right. There is too much mental gymnastic required to visualize
the text correctly to implement this. If one step is missed then
the outcome could be wrong.

Regards
-- 
:wq Claudio