Re: [Sidrops] WGLC = draft-ietf-sidrops-aspa-verification - ENDS 03/22/2023 (Mar 22 2023)

[Amir Herzberg <[e-mail]>]

I've reviewed this draft and recommend advancing it, I think its adoption
is relatively-easy and can have significant benefits - Kudos!

I have few comments, nothing critical though.

1. section 2, change the language and say that an ASe _MAY_ export any
announcement, including from non-customers, to sibling AS. The current
language may be interpreted as if siblings necessarily export everything.

2. section 4:  clarify that ASes A, B which are siblings MUST either both
register the other (in SPAS), or neither register the other in SPAS. I'm
concerned that if you don't clarify this condition, then we may have
deployments where A registers B in its SPAS but B doesn't register A in its
SPAS. Let C be  a provider of  A, and consider an announcement which A
receives from C and exports to B, which exports it to a customer D (who
adopts ASPA). Then D may (incorrectly)  filter this announcement (since it
appears to have been sent `up' from A to B).

3. 2. section 4: you refer to  an ASPA object `showing only AS 0 as a
provider AS' as an AS0 ASPA. It leaves the possibility of an ASPA object
showing as provider AS both AS 0 and another AS. I think you better forbid
this explicitly, i.e., AS 0 should only appear as the only provider AS (if
it appears at all).

4.  section 4: you say that all ASes MUST have an ASPA. I think this should
be only SHOULD as we can't really require/force ASes to adopt a standard.
Or just say that this requirement MUST be met by ASes compliant with the
current specifications.

5. section 5: in the hop-check function, the value `Provider' should better
be renamed to allow for the case the AS(j) is a RS or sibling of AS(i).
Probably best to change the terms in ASPA_Profile too. I realize this
change is a bit problematic since what we mean is really something like
`effective/extended provider', and people are already used to it too, but
maybe something like eProvider or extProvider will work? or UPok? In any
case, using exactly the same term for two different concepts - even with
explanation - is bound to cause confusion and may result in implementation
errors.

and here are some even more minor comments (nitpicks):

- in section 2, clarify that the relationships, except siblings, are
defined in RFC9234
- in section 4 add reference to definition of non-transparent RS AS (or
define)
- In 6.1, s/ AS(i)in/ AS(i) in/
- also  in 6.1: s/ For 2 <= i <= N, if there is an i/If there is an i s.t.
2 <= i <= N and/
- in 6.2, step 4, s/find the lowest value of v/find the lowest value of u/
--
-- 
Amir Herzberg

Comcast professor of Security Innovations, Computer Science and
Engineering, University of Connecticut
Homepage: https://sites.google.com/site/amirherzberg/home
`Applied Introduction to Cryptography' textbook and lectures:
https://sites.google.com/site/amirherzberg/cybersecurity