Re: [sidr] AD Review of draft-ietf-sidr-bgpsec-protocol-18

["Alvaro Retana (aretana)" <[e-mail]>]

Just noticed one more thing:  the reference to I-D.ietf-sidr-rtr-keying is no longer needed.

Thanks!

Alvaro.

On 12/1/16, 9:10 PM, "Alvaro Retana (aretana)" <[e-mail]<mailto:[e-mail]>> wrote:


On 11/27/16, 12:21 PM, "Sriram, Kotikalapudi (Fed)" <[e-mail]> wrote:



Sriram:



Hi!  Thanks for addressing my comments.  I have some remaining comments below.  I flagged a couple of items that I think the Chairs should consult the WG on, but I’ll leave it up to them to decide that – note that I don’t think we need to WGLC the whole document again, just a couple of decisions.



I also added a couple of new comments at the end.



I think we are closer, but there are still a couple of items where we’re not agreeing:  treatment of duplicates and confederations.  We can continue the discussion – I would love to hear other opinions as well (from the WG).



In the meantime, I will start the IETF Last Call and request a RTG-Dir review as well.  Addressing what we seem to agree on (I proposed some changes below for clarity) in the next few days would definitely help the process.



Thanks!



Alvaro.





…

> [Sriram] Following the clarifications and additional guidance you provided in Seoul (when

> you, Chris, and I met), I have added a new Section 7 (Operations and Management

> Considerations). The topics you mention above are all covered in the new Section 7.



Thanks for adding this Section.  I think the essence of what we talked about is there, but the text needs some editing.  I’m putting some suggestions below – keep in mind that this document is a specification, so you can be prescriptive with what you want to happen.



OLD>

Detailed recommendations concerning operations and management

   issues with BGPsec are provided in [I-D.ietf-sidr-bgpsec-ops].



NEW>

The Best Current Practice concerning operational deployment of BGPSec is provided in

[I-D.ietf-sidr-bgpsec-ops].





OLD>

   This document specifies BGPsec Version 0 only.  What should the

   action be if the Version is not 0 in the BGPsec capability

   advertisement from a peer?  If the intersection of BGPsec capability

   advertisements from both sides does not include Version 0, then

   BGPsec Version 0 has not been successfully negotiated.  However, a

   BGP session is still negotiated and hence the ability to exchange

   routes is still there.  BGP (unsigned) messages are exchanged.  So

   the chain of ASNs is not broken, i.e. they may not be contiguous

   BGPsec peers but are still contiguous BGP peers.



   Let us say that BGPsec capability was negotiated successfully between

   two peers, and subsequently it was reset.  In the meantime, assume

   that the 4-byte ASN capability or the multi-protocol capability was

   lost between the two peers.  So now the BGPsec session reset results

   in failure of BGPsec capability negotiation and only a BGP session is

   established.  Would the network operator be notified that this

   downgrade from BGPsec to BGP has happened?  What if the operator

   always wants a secure session only?  Can they require that no BGP

   session be established if BGPsec capability negotiation fails?

   Operators are advised to speak with their vendors to set up knobs and

   alerts to have such operations and management features in their

   BGPsec capable routers.



NEW>

   Section 2.2 describes the negotiation required to establish a BGPsec-capable

   peering session.  Not only must the BGPsec capability be exchanged (and agreed

   on), but the BGP multiprotocol extension [RFC4760] for the same AFI and the

   four-byte AS capability [RFC6793] must also be exchanged.  Failure to properly

   negotiate a BGPsec session, due to a missing capability, for example, may still

   result in the exchange of BGP (unsigned) updates.  While the BGP chain of ASNs

   is not broken, the security can be reduced and a contiguous set of BGPsec peers

   may not exist anymore.  It is RECOMMENDED than an implementation log the

   failure to properly negotiate a BGPsec session if the local BGP speaker is configured

   for it.  Also, an implementation MUST have ability to prevent a BGP session from

   being established if configured for BGPsec use.





…

> M1.1. Section 2.1. (The BGPsec Capability) includes a Version field and some Reserved bits,

> but there are no IANA registries defined for how to manage these spaces.  Please define

> the registries and the corresponding registration procedures.

>

> [Sriram] Done. Please see the updated IANA Considerations section.



s/Reserved/Unassigned



Given that there are only 3 unassigned bits and that changing versions should be a big deal, I think that “IETF Review” is not a strong enough registration procedure.  Suggestion: use Standards Action instead.



[*] Chairs: we don’t need to WGLC the whole document, but it would be a good idea to run the registration procedures by the WG once we settle on them – this could even be done in parallel with the IETF Last Call.





> M1.2. Section 3.1. (Secure_Path) defines the Flags field and assigns the first bit (BTW, is

> that the MSB or the LSB, please clarify), but doesn't set up the registry or registration

> procedures.

>

> [Sriram] It is the MSB (left most bit) and that is now clarified in Section 3.1. Set up of the

> registry for the Flags field is now included in the updated IANA Considerations section.



Same comment as above about “IETF Review” vs “Standards Action”.



…

> M2.2. The definition of the BGPsec_Path Attribute (and its details) don't have clear error

> handling procedures defined (RFC7606).  Section 4.3. (Processing Instructions for

> Confederation Members) does say this: "(As discussed in Section 5.2, any error in the

> BGPsec_Path attribute MUST be handled using the "treat-as-withdraw", approach as

> defined in RFC 7606 [RFC7606].)" (including the parentheses).  However, 5,2 only says:

> "BGPsec speakers MUST handle these errors using the "treat-as-withdraw" approach as

> defined in RFC 7606 [RFC7606]."   Note: "these errors" is not the same as "any

> error".  Please discuss error handling in a more prominent place. (Hint: it may fit in a fault

> management discussion in the Operations and Management Section.)

>

> [Sriram]  s /As discussed in Section 5.2, any error in the BGPsec_Path attribute MUST be

> handled using the "treat-as-withdraw", approach as defined in RFC 7606 [RFC7606]./ As

> discussed in Section 5.2, any syntactical or protocol violation errors in the BGPsec_Path

> attribute MUST be handled using the "treat-as-withdraw" approach as defined in RFC 7606

> [RFC7606]./

>

> [Sriram]  Section 4.3 has been updated accordingly.





I didn’t explain myself correctly.  Section 4.3. (Processing Instructions for Confederation Members) covers only the processing for confederations, so putting text in there about any errors is good, but doesn’t cover the general case (for processing any update).



OTOH, Section 5.2. (Validation Algorithm) doesn’t actually talk about “any error” (as referenced now from 4.3) – it only talks about “these errors”, which correspond to the list of checks in the section.  IOW, there is no global specification that any error must result in treat-as-withdraw.



Solution:  take the text out of 4.3 and update the text in 5.2; suggestion:



OLD>

   If any of these checks fail, it is an error in the BGPsec_Path

   attribute.  Any of these errors in the BGPsec_Path attribute are

   handled as per RFC 7606 [RFC7606].  BGPsec speakers MUST handle these

   errors using the "treat-as-withdraw" approach as defined in RFC 7606

   [RFC7606].



NEW>

   If any of these checks fail, it is an error in the BGPsec_Path

   attribute.  BGPsec speakers MUST handle any syntactical or protocol errors

   in the BGPsec_Path attribute using the "treat-as-withdraw" approach as defined

   in RFC 7606 [RFC7606].







…

> M7.1. “With regards to the processing of duplicate update messages, if the first update

> message is valid, then an implementation SHOULD NOT run the validation procedure on the

> second, duplicate update message (even if the bits of the signature field are

> different).”  Even though a discussion about non-deterministic signature algorithms

> precedes this text, the validation is still not run.  How can the validity of the Path be

> guaranteed in this case?  Should this be the action for all algorithms or only ones known to

> be non-deterministic?

>

> [Sriram] Definition of duplicate update for BGPsec: A BGPsec update is a duplicate if it is

> identical to another update in the Adj-RIB-in, including SKIs and Algorithm ID, but not

> including the signatures. If the first update message (having the *same SKIs* as the

> duplicate) is *Valid*, then the duplicate’s validity state need not be computed.  If validity of

> the duplicate were computed and found 'Valid', then it gives the router no new

> information. Alternatively, if it were found 'Not Valid', then it only implies that some bit

> errors occurred in the signatures. Therefore, the BGPsec speaker should keep the 'Valid'

> original update and ignore the duplicate. However, if the original update were 'Not Valid',

> then performing validation of the duplicate is relevant and SHOULD be done.

>

> [Sriram] Added a paragraph in the new Section 7 (Operations and Management

> Considerations) to include the above observations.



I saw the text, and have 3 questions/comments:



1. “…it only implies that some bit errors occurred in the signatures.”   What does that mean?  Are you implying that the sender got the signature wrong?  Or maybe that there was some corruption in-transit?  Both cases are *bad*, but the case where the sender could get the signature wrong is specially so because if it wasn’t a duplicate update, then it would result “Not Valid” and there seems to be no way to determine if in fact the validity failed or if it is the case of some bit errors. ☹



2. “Therefore, the BGPsec speaker should keep the 'Valid' original update and ignore the duplicate.” No!  This is not how BGP works…see below.



3. “…if the original update were 'Not Valid', then performing validation of the duplicate is relevant and SHOULD be done.”  Why is this not a “MUST”?  IOW, if the original update was “Not Valid” when would it be ok to not perform validation on a new update?





> [Sriram] Note that the above applies to both non-deterministic and deterministic signature

> algorithms.

>

> M7.2. rfc4271 (in Section 9. (UPDATE Message Handling) talks about the implicit withdraw

> of a route “if the NLRI of the new route is identical to the one the router currently has

> stored…”.  The same NLRI case seems to be a particular condition of the “duplicate

> update” described here.  It might be a good idea to mention that a “duplicate update”

> results in the implicit withdraw of the original update.  What happens if a third duplicate

> route is received (the first one was valid, the second one was not validated), should it be

> validated?

>

> [Sriram] In RFC 4271, when there is a duplicate update, the NRLI and path and all other

> attributes are identical. There is no implicit withdraw. The router keeps what it already

> has. In the case of BGPsec, a router keeps the original if it was valid, and ignores the

> duplicate. It checks the validity of the duplicate only if the original were invalid.



In plain BGP, if a duplicate update (as defined above where everything is identical) is received, then the original one is replaced by the new one – in this specific case (where everything is identical), it may look like the router keeps what it already has…but the general rule is to replace the old update with a new one for the same prefix.



What you are proposing above (“In the case of BGPsec, a router keeps the original if it was valid, and ignores the duplicate.”) is a significant change with respect to BGP that is not explicitly called out.  I haven’t seen this behavior mentioned anywhere in the draft – it has only come up now.



IMHO, not only should the implicit withdraw behavior not change, but by ignoring the second update you’re ignoring a potentially important change in the topology/structure of the network.



…

> M10.  In Section 7.2. (On the Removal of BGPsec Signatures): “…the protocol specifies that

> a BGPsec speaker choosing to propagate the route advertisement in such an update

> message SHOULD add its signature to each of the Signature_Blocks.”   I believe the

> reference is Section 4.2 (please add it).  However, that Section doesn’t use normative

> language (“For each Signature_Block corresponding to an algorithm suite that the BGPsec

> speaker does support, the BGPsec speaker adds a new Signature Segment to the

> Signature_Block.”)   In any case, the “SHOULD” in 7.2 is out of place because it is

> referencing a fact (pointing at the other section) and not being used normatively – if you

> want the “SHOULD” to be normative, then it should be back in 4.2.

>

> [Sriram] Yes, agree. Fixed the wording per your suggestion in both places (Sections 4.2 and

> 7.2). Added reference to Section 4.2 in Section 7.2.



Now 4.2 reads: “For each Signature_Block corresponding to an algorithm suite that the BGPsec speaker does support, the BGPsec speaker SHOULD add a new Signature Segment to the Signature_Block”   Should that “SHOULD” be a “MUST”?





…

> M12. The mandatory addition of Secure_Path and Signature segments in a Confederation

> results in the inconsistent authorization chain mentioned in Section 4.3. (Processing

> Instructions for Confederation Members): “For a signature produced by a peer BGPsec

> speaker outside of a confederation, the Target AS will always be the AS Confederation

> Identifier (the public AS number of the confederation) as opposed to the Member-AS

> Number.”  It seems to me that this discontinuity in the ASN list breaks the “cryptographic

> assurance that…Every AS on the path of ASes listed in the update message has explicitly

> authorized the advertisement of the route to the subsequent AS in the path” because there

> is no way to verify that the Member-AS in the first Secure_Path segment is in fact the one

> peering with the external neighbor.  I realize that the risk may be minimized by an

> “internal trust” factor, but I would like to see a discussion about this issue in the Security

> Considerations.

>

> [Sriram] See the new second paragraph in Section 7.4. The security consideration that you

> mention here about with the way confederations are handled is described and discussed

> there.



Personally, I would have preferred if you actually solved the problem.



One solution is to borrow from draft-ietf-sidr-as-migration and forward sign from the Confederation AS (the public number) to the Member-AS with pCount=0.  Note that this operation would take place inside the border Confederation router, so there are no issues with pCount=0 and the full path continuity is preserved.



[*] Chairs: I think that this part (whether it is solved or not) should also be bounced by the WG.





…

> m8. Section 5.1. (Overview of BGPsec Validation): “It is expected that BGP peers will

> generally prefer routes received via 'Valid' BGPsec update messages over both routes

> received via 'Not Valid' BGPsec update messages and routes received via update messages

> that do not contain the BGPsec_Path attribute…(See [I-D.ietf-sidr-bgpsec-ops]…”  I read

> this piece of text as saying that routes in Not Valid updates are expected to be used (even

> though they are not valid).  Besides the fact that I-D.ietf-sidr-bgpsec-ops does recommend

> using Not Valid announcements (in Section 7), are there other reasons for this document to

> expect their use?

>

> [Sriram] If only a ‘Not Valid’ update is available for a prefix, then the update is used since

> otherwise the prefix would be unreachable. Both BGPsec and RPKI/origin validation are

> expected to depref invalid updates rather than ignore them. This is driven by operator

> policy and can vary. Hence, we are deliberately not using any normative language here.



Honestly, using “Not Valid” updates makes no sense to me.  We’re building all this machinery, changing BGP, etc.. just to end up using updates that are not valid anyway.  Why are we doing all this work then?!?  ☹



I understand that, specially at the beginning, people may want to use “Not Valid” updates…but that is an operational consideration (as you said above).  Please take the text out from this document that recommends using “Not Valid”…and let I-D.ietf-sidr-bgpsec-ops deal with that recommendation.



…



New comment:



N1. Section 2.2. (Negotiating BGPsec Support) says that because “BGPsec update messages can be quite large, therefore any BGPsec speaker…SHOULD also announce support for the capability to receive BGP extended messages [I-D.ietf-idr-bgp-extended-messages].”   What does this mean with respect to the session establishment requirements?  This capability is not mandatory (SHOULD is used and not MUST), but as BGPsec is deployed more and more, the system may not work anymore unless it is exchanged.   Why wasn’t “MUST” used?  I’m guessing that part of the reason may be availability of the feature…  I think that the potential of having a properly negotiated BGPsec session still not work (because the messages are too big) should at least be mentioned in the new Operations and Management Considerations Section.



N2. The references to RFC5226 and RFC6487 should be Normative.