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

"Sriram, Kotikalapudi (Fed)" <kotikalapudi.sriram@nist.gov> Sun, 27 November 2016 17:21 UTC

Return-Path: <kotikalapudi.sriram@nist.gov>
X-Original-To: sidr@ietfa.amsl.com
Delivered-To: sidr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id EFFAF1294F9; Sun, 27 Nov 2016 09:21:36 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.901
X-Spam-Level:
X-Spam-Status: No, score=-1.901 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=nistgov.onmicrosoft.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id H6k91xeYUTmD; Sun, 27 Nov 2016 09:21:33 -0800 (PST)
Received: from gcc01-CY1-obe.outbound.protection.outlook.com (mail-cy1gcc01on0106.outbound.protection.outlook.com [23.103.200.106]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id CB5F0129474; Sun, 27 Nov 2016 09:21:32 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nistgov.onmicrosoft.com; s=selector1-nist-gov; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=JY+EkKGo+cLtze+Pgm3UnGdNL5GHFAK5Hhso3V57oEU=; b=ZNquYg+dGczglOIdmyFkz3E7sBDJ+lsQXOxRtiDDGxfW5BKNL/f6lxW7qbDI5os6oESF8FrK7jwIY1qlve94sfmXMSs/oFbwnd/iREg3VDfWn7u7CujLlAZVOUM/ooBezxGed8d33J+NQJkvJtpux1svRt12/Y8kHfqYm/Fam58=
Received: from DM2PR09MB0446.namprd09.prod.outlook.com (10.161.252.145) by DM2PR09MB0448.namprd09.prod.outlook.com (10.161.252.147) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.747.13; Sun, 27 Nov 2016 17:21:30 +0000
Received: from DM2PR09MB0446.namprd09.prod.outlook.com ([10.161.252.145]) by DM2PR09MB0446.namprd09.prod.outlook.com ([10.161.252.145]) with mapi id 15.01.0747.015; Sun, 27 Nov 2016 17:21:30 +0000
From: "Sriram, Kotikalapudi (Fed)" <kotikalapudi.sriram@nist.gov>
To: "Alvaro Retana (aretana)" <aretana@cisco.com>, "draft-ietf-sidr-bgpsec-protocol@ietf.org" <draft-ietf-sidr-bgpsec-protocol@ietf.org>
Thread-Topic: AD Review of draft-ietf-sidr-bgpsec-protocol-18
Thread-Index: AQHSNR6CtOW/YAot7EOz/vffo+bn+6DtE+nUgAAdepM=
Date: Sun, 27 Nov 2016 17:21:29 +0000
Message-ID: <DM2PR09MB04460C753A2AA4249C03DD82848B0@DM2PR09MB0446.namprd09.prod.outlook.com>
References: <D401650E.13F93A%aretana@cisco.com>, <DM2PR09MB0446AD92D6F8149E41E0DE1E848B0@DM2PR09MB0446.namprd09.prod.outlook.com>
In-Reply-To: <DM2PR09MB0446AD92D6F8149E41E0DE1E848B0@DM2PR09MB0446.namprd09.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: spf=none (sender IP is ) smtp.mailfrom=kotikalapudi.sriram@nist.gov;
x-originating-ip: [129.6.218.53]
x-ms-office365-filtering-correlation-id: 81c3487d-69af-46b9-458a-08d416e9d3ad
x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:DM2PR09MB0448;
x-microsoft-exchange-diagnostics: 1; DM2PR09MB0448; 7:PylJWYvxC3dbyHi/ieo8cLHI31i2XA9M3TCAGGGm4q3JUvcPPWX3GchXkZtXjbK19jhHE5Lnzla+GpRKhXXi8OBQJ2EMwwZwvAVvfK2KR0VuSBv5JExBkHH0xZmqbpqSQaIz22KqoHrMc8N0Gn8jpAzhBSFgyqGpps89p+l/vLTLnnrYGGOAOF/vM3HByqWQl/gQUHkwotOJtMov7TdWoOqzF9k++zsxyzpJAeHsgt2XD/pHn7R3wIioldAkaiMkr662Pa9RVHkx1WjtfFKcbKecQD09IWlP9F4wscnWUOQVFJ/4QEiCVkshBTms34FBurLkcIriabwnfFscNnaucBST81qq5zVlI3dQtKOqVqdgCc/tj7V8C6jVZLKDVYytHL4vT2PHE2tgFFQrQ6sHCZYV0vCB2hTLYWT+TSZE93SjtM4IoZwVFNfvKTWFFj0LAPJ0Z//uTUNO3/LbvrDBew==
x-microsoft-antispam-prvs: <DM2PR09MB0448498260D914E85A2CB99C848B0@DM2PR09MB0448.namprd09.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:(158342451672863)(192374486261705);
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6060326)(6040361)(6045199)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026)(6041248)(6061324)(20161123560025)(20161123555025)(20161123564025)(20161123562025)(6072148); SRVR:DM2PR09MB0448; BCL:0; PCL:0; RULEID:; SRVR:DM2PR09MB0448;
x-forefront-prvs: 0139052FDB
x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(6009001)(7916002)(51444003)(66654002)(189002)(45984002)(199003)(55674003)(74316002)(7696004)(8676002)(77096006)(230783001)(551984002)(81156014)(6606003)(106356001)(122556002)(99286002)(105586002)(7736002)(86362001)(2950100002)(106116001)(97736004)(101416001)(5660300001)(3660700001)(345774005)(7846002)(50986999)(54356999)(76176999)(66066001)(3280700002)(5001770100001)(81166006)(229853002)(39410400001)(2906002)(39450400002)(19627405001)(33656002)(4326007)(39380400001)(39400400001)(6116002)(102836003)(92566002)(189998001)(76576001)(68736007)(2900100001)(9686002)(39060400001)(38730400001)(6506003)(5890100001)(8936002)(2501003)(3846002); DIR:OUT; SFP:1102; SCL:1; SRVR:DM2PR09MB0448; H:DM2PR09MB0446.namprd09.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en;
received-spf: None (protection.outlook.com: nist.gov does not designate permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: multipart/alternative; boundary="_000_DM2PR09MB04460C753A2AA4249C03DD82848B0DM2PR09MB0446namp_"
MIME-Version: 1.0
X-OriginatorOrg: nist.gov
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Nov 2016 17:21:29.8840 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 2ab5d82f-d8fa-4797-a93e-054655c61dec
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR09MB0448
Archived-At: <https://mailarchive.ietf.org/arch/msg/sidr/VcjM0D9j0EtJ3Rx-BlLPNgmr0bs>
Cc: "sidr-chairs@ietf.org" <sidr-chairs@ietf.org>, Matthias Waehlisch <m.waehlisch@fu-berlin.de>, "sidr@ietf.org" <sidr@ietf.org>
Subject: Re: [sidr] AD Review of draft-ietf-sidr-bgpsec-protocol-18
X-BeenThere: sidr@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: Secure Interdomain Routing <sidr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/sidr>, <mailto:sidr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/sidr/>
List-Post: <mailto:sidr@ietf.org>
List-Help: <mailto:sidr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/sidr>, <mailto:sidr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 27 Nov 2016 17:21:37 -0000

Hi Alvaro,

Thank you very much for this thorough and detailed set of comments.

They greatly help improve the clarity, accuracy, and presentation in the document.

I have worked with each of the comments and incorporated changes accordingly

in the document. Please see version-19 that was just submitted.

Many thanks to Sean Turner for his help with the updated IANA considerations section.

My responses to your individual comments are shown below and are marked with [Sriram].

Let me know if the pdf opens OK for you.

Also, please let me know if I missed anything.

Regards,

Sriram

P.S. I tried to send this message with a pdf attachment earlier to the SIDR list.

But looks like that post was not accepted by the IETF email exploder due to the attachment, may be?

So I have copied and pasted here the text from the pdf.

There may be line-wrap issue -- let us see.

-----------

Dear authors:

Hi!  First of all, thank you for taking on the duties of editing this document.


I have several comments (see below).  For the most part, I think they should be easy to solve as many are related to clarifications.  Most of the comments I classified as Major are due to the use of Normative language.


The biggest concern I have with this document is the lack of an Operations and Management Considerations Section – please take a look at RFC5706 (Guidelines for Considering Operations and Management of New Protocols and Protocol Extensions).  Some of the information suggested there is present in draft-ietf-sidr-bgpsec-ops, and (ironically) in the “Design and Deployment Considerations” section of draft-ietf-sidr-bgpsec-overview.  However, important items such as migration or management are missing.  I would like to see a well thought out Operations and Management Section in this document before moving it forward.  Note that I’m not suggesting that a YANG model (for example) is required to move forward, but I would like to see considerations about migration, and the impact on network operations, to mention two items, all in one place in the document.  I would like the authors/Chairs/Shepherd/WG to consider even merging in draft-ietf-sidr-bgpsec-ops as the base for this new section (or at least reference it prominently).

[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. Additional responses noted here below under your specific comments.

Thanks!

Alvaro.

Major:

M1. Registry Definitions

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.

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.



M2. Error Handling — Several sections don't have proper error handling procedures specified. (This is a management issue that I think is underspecified.)

[Sriram] Now there is management and operations section (Section 7) added where error handling and other ops/mgmt. issues are discussed.

M2.1. Section 2.2. (Negotiating BGPsec Support) doesn't fully specify the error handling behavior of the Capability, and it fails open.

[Sriram] See my responses below for M2.1.1.

The new management and operations section (Section 7) covers this case.

M2.1.1. What should the action be if the Version is not 0?

[Sriram]: From Section 2.2:

“BGP

   update messages without the BGPsec_Path attribute MAY be sent within

   a session regardless of whether or not the use of BGPsec is

   successfully negotiated.”

[Sriram] Based on the above, there is a possibility that BGPsec is not successfully negotiated but BGP is session is established – I think that is what you are calling fail open. If the intersection of BGPsec capability advertisements from both sides does not include Version 0, then BGPsec Version 0 has not been successfully negotiated. But a BGP session is still negotiated and BGP (unsigned) messages are exchanged. I have now included this wording in the new ops/mgmt. Section 7.

M2.1.2. "…a BGP speaker MUST NOT advertise the capability of BGPsec support for a particular AFI unless it has also advertised the multiprotocol extension capability for the same AFI [RFC4760]."  What should happen if it does advertise an AFI that is not covered by the multiprotocol extension capability?   Or if the multi-protocol capability is not advertised at all?  To clarify: if the multi-protocol capability is not advertised then support for BGPsec can’t be advertised either – does that mean that a BGP speaker configured to use BGPsec must not use it if not negotiated?  I know the answer is “yes”, but I’m trying to get to the point of provisioning and expectations – why configure BGPsec if no one is expected to support it?

[Sriram]: See response to 2.1.1 above. Also, during early phase of deployment, small groups of (consisting of 2 or more) ASes are likely to coordinate and deploy BGPsec over contiguous regions (ASes). These regions will grow over time and conjoin.  Also based on clarifications you provided in Seoul, I have also included error handling considerations for the case when BGPsec resets and some necessary condition (like 4-byte ASN or MP-NLRI capability) got dropped in the meantime.

M2.1.3. Missing the four-byte AS capability results in BGPsec not working ("BGPsec has not been successfully negotiated"), but the ability of exchanging routes is still there, leaving the system in a fail open state and potentially breaking the chain of ASNs.  Personally, it doesn't seem like a good result — please at least include some text about this in the Security Considerations section.

[Sriram] Even though BGPsec is not successfully negotiated, the ability of exchanging routes is still there – BGP messages are exchanged though not BGPsec; it so the chain of ASNs in not really broken (they are not contiguous for BGPsec but still contiguous for BGP). This observation is included in the new Section 7 (ops/mgmt. section).

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.

M2.2.1. There are multiple places in the BGPsec_Path Attribute that could end up in an error, everything from setting bits in the Flags field to wrong Length fields.  Should all errors result in the same behavior?

[Sriram] Any errors that are syntactical errors or detected to be protocol violation errors in the BGPsec_Path attribute should result in the same behavior (see above comment). I think the updated Section 5.2 enumerates them reasonably well now. But bit errors that may result in change in the value of an ASN or a signature would not fall in that category.

M3. Section 4.1. (General Guidance): "When propagating a received route advertisement to an internal peer, the BGPsec speaker typically populates the BGPsec_Path attribute by copying the BGPsec_Path attribute from the received update message.  That is, the BGPsec_Path attribute is copied verbatim…. Note that when a BGPsec speaker chooses to forward a BGPsec update message to an iBGP peer, the BGPsec attribute SHOULD NOT be removed, unless the peer doesn't support BGPsec."  The first part of the guidance says that a new BGPsec_Path attribute is created by copying the received attribute (which is then presumably removed), but the second part says that the received attribute SHOULD NOT be removed.  Please clarify so that there is consistency -- I understand that the result is the same, but the description is not and we should try to avoid confusion.  Parts of Section 4.2. (Constructing the BGPsec_Path Attribute) also talk about actions like "…and there is an existing BGPsec_Path attribute, then the BGPsec speaker prepends its new Secure_Path Segment (places in first position) onto the existing Secure_Path", which hint at propagating a received BGPsec_Path attribute (and not creating a new one).

[Sriram] Yes, agree. We’ll remove redundant and confusing statements and simply state the stuff in the first set of quotes as follows (here we avoid using the word copying which causes some confusion):

[Sriram] When a BGPsec speaker chooses to forward a BGPsec update message to an iBGP peer, the BGPsec_Path attribute SHOULD NOT be removed, unless the peer doesn't support BGPsec. In the case when an iBGP peer doesn't support BGPsec, then the BGPsec update is reconstructed to a BGP update with AS_PATH and then forwarded (see Section 4.4). In particular, when forwarding to a BGPsec capable iBGP peer, the BGPsec_Path attribute SHOULD NOT be removed even in the case where the BGPsec update message has not been successfully validated.

M4. Section 4.2. (Constructing the BGPsec_Path Attribute) says that "The AS number in this Secure_Path segment MUST match the AS number in the AS number resource extension field of the Resource PKI router certificate(s) that will be used to verify the digital signature(s) constructed by this BGPsec speaker [I-D.ietf-sidr-bgpsec-pki-profiles]."  However, there is no extension field or certificate in I-D.ietf-sidr-bgpsec-pki-profiles with that name.  Please be precise with the names.

[Sriram] Modified as follows: The AS number in this Secure_Path segment MUST match the AS number in the Subject field of the Resource PKI router certificate that will be used to verify the digital signature constructed by this BGPsec speaker (see Section 3.1.1.1 in [I-D.ietf-sidr-bgpsec-pki-profiles] and RFC 6487 [RFC6487]).

M5. In 4.2: “BGPsec speakers SHOULD drop incoming update messages with pCount set to zero in cases where the BGPsec speaker does not expect its peer to set pCount to zero.”  This text seems to assume some kind of configuration/provisioning.  Note that Section 5.2. (Validation Algorithm) also has similar text about receiving an UPDATE “from a peer that is not expected to set pCount equal to zero”.

[Sriram] A peer that is an Internet Exchange Point (IXP) (i.e. Route Server) with a transparent AS is expected to set pCount = 0 in its Secure_Path segment while forwarding an update to a peer (see Section 4.2).  Clearly, such an IXP SHOULD configure itself to set its own pCount = 0. As stated in Section 4.2, “BGPsec speakers SHOULD drop incoming update messages with pCount set to zero in cases where the BGPsec speaker does not expect its peer to set pCount to zero.”  This means that a BGPsec speaker SHOULD be configured so that it permits pCount =0 from an IXP peer and never permits pCount = 0 from a peer that is not an IXP.

Added the above paragraph in the new Section 7.

M6. In 4.2: “If the received BGPsec update message contains two Signature_Blocks and the BGPsec speaker supports both of the corresponding algorithm suites, then the new update message generated by the BGPsec speaker SHOULD include both of the Signature_Blocks.”  Why is this “SHOULD” not a “MUST”?   When/why would a speaker remove one or the 2?  If one is removed, should there be a requirement that the one that was used to successfully validate the update be kept?  Note that Section 7.2 later talks about the problems of removing signatures…

[Sriram]  See below.

M6.1. Note that later in this section the text says that “a 'Valid' BGPsec update message may contain a Signature_Block which is not deemed 'Valid' (e.g., contains signatures that BGPsec does not successfully verify).  Nonetheless, such Signature_Blocks MUST NOT be removed.”   Taking this “MUST NOT” along with the “SHOULD” above, the door is open to remove the Signature_Block used to verify the validity and just forward the one not used (which may itself not be valid).

[Sriram] Your observations are right. Therefore, we made the following change (SHOULD --> MUST): If the received BGPsec update message contains two Signature_Blocks and the BGPsec speaker supports both of the corresponding algorithm suites, then the new update message generated by the BGPsec speaker MUST include both of the Signature_Blocks.

M6.2. Section 5.2. (Validation Algorithm) opens this door even more when saying that “If at least one Signature_Block is marked as 'Valid', then the validation algorithm terminates and the BGPsec update message is deemed to be 'Valid'.”  The text here doesn’t require that both Signature_Blocks be verified, but implies that as long as the first one is valid then the second one doesn’t really need to be verified.  Is that the intent?

[Sriram]  Yes. There is no problem with early termination since both algorithms are in use in parallel and either one can be used for verification. In fact, it helps with route processor performance to terminate early on a ‘Valid’ outcome after successfully verifying one of the Signature_Blocks.

[Sriram]  We have added a para in the new Section 7 on performance enhancement during BGPsec update validation.

M7. Section 5. (Processing a Received BGPsec Update) talks about “duplicate update messages” (one where “it differs from the first update message only in the Signature fields (within the BGPsec_Path attribute)”).

[Sriram]  ECDSA P256 algorithm produces a different set of signature bits when it signs the same data again at a later time.

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.

[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.

M8. Section 5.1. (Overview of BGPsec Validation) says that "BGPsec specifies no changes to the BGP decision process."  However, Section 5. (Processing a Received BGPsec Update) also says that "a BGPsec speaker MUST utilize the AS path information in the BGPsec_Path attribute in all cases where it would otherwise use the AS path information in the AS_PATH attribute."  I'm assuming that the Decision Process is included in "all cases".  Even though 5.1 refers to the use the validation state in the decision process, please make sure that it is clear that the decision process is modified by the use of the different attribute.

[Sriram] I agree that making use of validation state in BGPsec implies a change in the decision process.  So we will remove the sentence that says "BGPsec specifies no changes to the BGP decision process."

M8.1. Please specifically include a section (or text somewhere) about how the information in the BGPsec_Path attribute is used in the Decision Process.  For example, how should the "number of AS numbers" in the path be calculated for 9.1.2.2. (Breaking Ties (Phase 2)) in rfc4271?  The text talks about the "effective length" being the sum of the pCount values, but the "effective length" (at least with that name) is not what is used in rfc4172 — please be clear and consistent.

[Sriram] Done the following: s/ effective length of the AS path/ length of the AS path/

[Sriram] Paragraph in Section 3.1 updated as follows:

[Sriram] The pCount field contains the number of repetitions of the associated autonomous system number that the signature covers.  This field enables a BGPsec speaker to mimic the semantics of prepending multiple copies of their AS to the AS_PATH without requiring the speaker to generate multiple signatures. Note that Section 9.1.2.2 ("Breaking Ties") in [RFC4271] mentions "number of AS numbers" in the AS_PATH attribute that is used in the route selection process. This metric (number of AS numbers) is the same as the AS path length obtained in BGPsec by summing the pCount values in the BGPsec_Path attribute.

M8.2. [minor] Section 3. (The BGPsec_Path Attribute) reads: "The information in Secure_Path is used by BGPsec speakers in the same way that information from the AS_PATH is used by non-BGPsec speakers."  This is pretty much the same information that is in Section 5, but with more specificity.  It would help if the more specific case was the one normatively called out.

[Sriram] Made the change -- more specific language (Secure_Path as opposed to BGPsec_Path attribute) is now used in Section 5 also.

M9. Section 5.2. (Validation Algorithm).  RFC4271 also specifies a series of validity checks when an UPDATE is received (Section 6.3) – should that check be run before or after the algorithm specified here?  The algorithm focuses on verifying the validity of the BGPsec_Path attribute (and not the whole UPDATE), so I’m assuming it should be executed instead of checking the AS_PATH.  Please include some text about the interaction/changes.

[Sriram] Section 5.2 has been updated to reflect the above comment.

M9.1.  Section 5.2. (Validation Algorithm): “…then the BGPsec speaker MUST treat the update message in the same manner that the BGPsec speaker would treat an (unsigned) update message that arrived without a BGPsec_Path attribute.”   What exactly does this mean?  If the BGPsec_Path attribute is not received, then the AS_PATH should be – does the text imply that the AS_PATH should be reconstructed?  I guess it should be if the update is to be propagated – but the question is while the update is being processed, which AS path information is used, the one in a reconstructed AS_PATH or the one in the BGPsec_Path while assuming that all the signatures are correct?

[Sriram] Changed the wording. The corrected wording in Section 5.2 is as follows:

[Sriram] Next, the BGPsec speaker examines the Signature_Blocks in the BGPsec_Path attribute.  A Signature_Block corresponding to an algorithm suite that the BGPsec speaker does not support is not considered in validation.  If there is no Signature_Block corresponding to an algorithm suite that the BGPsec speaker supports, then the BGPsec speaker MUST strip the Signature_Block(s), reconstruct the AS_PATH (see Section 4.4), from the Secure_Path, and treat the update as if it was received as an unsigned BGP update.

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.

M11. The Figures (which are not numbered) in 4.2. (Constructing the BGPsec_Path Attribute) and 5.2. (Validation Algorithm) present the sequence of octets to be hashed.  I’m guessing that the order of the Signature and Secure_Path Segments may be important, is it?  The order in the Figures is not clear to me, if the order is important, please be clear about it; if not, please also say so.

[Sriram] Yes, the order of the Signature and Secure_Path Segments is important. I have put in additional wording for clarifying/rationale and emphasized that the order is important – in both sections.

[Sriram] Figures are numbered now in the document.

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.

M13. What about replay attacks?  There is no mention of the risk or potential mitigation anywhere.  Please include in the Security Considerations section.

[Sriram]: Added a new paragraph and reference to ietf-sidr-bgpsec-rollover in Section 8.4.

Minor:

m1. The use of SKI, Subject Key Identifier without a qualifier (field, extension) is confusing at times.   Please expand SKI on first use.  An example: (from 3.2) “The Subject Key Identifier contains the value in the Subject Key Identifier extension of the RPKI…”  The first mention should include “field”, like similar text in 4.2.

[Sriram]: Done.

m2. Section 4. (BGPsec Update Messages) says: "A BGPsec speaker that is not a member of such a confederation MUST set the Flags field of the Secure_Path Segment to zero in all BGPsec update messages it sends."  While only one flag is defined, the correct statement is "…set the Confed_Segment flag…".

[Sriram]: Done.

m3. Section 4.1. (General Guidance): s/"A BGPsec update message MUST advertise a route to only a single NLRI."/"A BGPsec update message MUST advertise a route to only a single prefix."   This section contains other places where the NLRI term is not used correctly.  In short, the NLRI in a BGP Update contains one or more prefixes — so the text should talk about a single prefix, not a single NLRI.

[Sriram]  Replaced “NLRI” with “prefix” in all places in the document where appropriate.

m4. In between the text above, the following is written: "However, in the case that the BGPsec speaker is performing an AS Migration, the BGPsec speaker may add an additional signature on ingress before copying the BGPsec_Path attribute (see [I-D.ietf-sidr-as-migration] for more details)."  Because I-D.ietf-sidr-as-migration is marked as Updating this document, I suggest you remove this text (and the one in 4.2) -- note that the statements made in this document are not normative anyway and I-D.ietf-sidr-as-migration can stand on its own by clearly specifying what is needed for AS Migration.

[Sriram] Yes, good suggestion – removed the text. Done.

m5. In 4.2: “To prevent unnecessary processing load…a BGPsec speaker SHOULD NOT produce multiple consecutive Secure_Path Segments with the same AS number.  This means that to achieve the semantics of prepending the same AS number k times, a BGPsec speaker SHOULD produce a single Secure_Path Segment – with pCount of k...”  Given pCount, I’m wondering why these SHOULDs are not MUSTs, especially given the expected additional load.

[Sriram] We had thought about it. Decided to leave it as SHOULD rather than MUST. Most implementations will do it right. We thought -- just make it a strong recommendation rather than enforce strictly.

m6. Section 4.3. (Processing Instructions for Confederation Members) explains the process of adding Secure_Path and Signature segments that may or may not be used at all (given that the verification is optional), only to remove them later.  Why isn’t the process of adding Secure_Path and Signature segments optional itself (instead of just the validation)?

[Sriram] In a confederation, the update is crossing AS boundaries and ASNs of the members are included (cannot be omitted).  It was felt that it is not a good idea to have mixed signed and unsigned segments. Also, the same security risks may exit (e.g. illegal path shortening) between confed ASes as do between regular ASes.

m7. In 4.4. (Reconstructing the AS_PATH Attribute), what should happen if the Confed_Segment flag is set to zero and the most-recently added segment in the AS_PATH is of type AS_CONFED_SEQUENCE?  Theoretically this can’t occur because it means that someone accepted an update that it shouldn’t have, but please include some text about this case being an error.

[Sriram] This comment is related to m11. Added a new error check in Section 5.2 that reads, “If the update message was received from a BGPsec peer that is a member of the BGPsec speaker's AS confederation, check to ensure that the Secure_Path segment corresponding to that peer contains a Flags field with the Confed_Sequence flag set to one.” This takes care of m7 and m11.



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.

m9. Section 5.1 contains these references: “…the trusted cache could deliver the necessary validity information to the BGPsec speaker using the router key PDU [I-D.ietf-sidr-rtr-keying] for the RPKI-to-Router protocol [I-D.ietf-sidr-rpki-rtr-rfc6810-bis].”   The reference to I-D.ietf-sidr-rtr-keying seems to be related to the “router key PDU”, but that is defined in I-D.ietf-sidr-rpki-rtr-rfc6810-bis, so it looks like the first reference is not needed.

[Sriram] Yes. Fixed.

m10. In 5.1: “As discussed in Section 4, when a BGPsec speaker chooses to forward…, it SHOULD be forwarded with its BGPsec_Path attribute…”   That “SHOULD” is pointing at a fact, not acting normatively in this sentence so please change it to “should”.

[Sriram] Yes. Fixed.

m11. Section 5.2. (Validation Algorithm) mentions this check: ‘update..from a peer that is not a member of the BGPsec speaker's AS confederation, check to ensure that none of the Secure_Path segments contain a Flags field with the Confed_Sequence flag set to one.”  I’m sure that the check for the flag set if the peer is a Confederation peer is also needed, but not mentioned in this section (where the normative MUST for the validation algorithm) is present.  Section 4.3. (Processing Instructions for Confederation Members) does say this: “…when a confederation member runs the algorithm in Section 5.2, the confederation member, during processing of a Signature Segment, first checks whether the Confed_Sequence flag in the corresponding Secure_Path segment is set to one.”   I would like to see the full algorithm specified in one place (even if, like in Section 4.3, pieces of it are explained elsewhere).  Also, the text in 4.3 says that the check is performed “during processing of a Signature Segment”, which is fine, but probably late in the process (compared to the text in 5.2 that seems to require the check when the updates are received).

[Sriram] Thanks for catching this. It is all fixed now. Wording changes have been made in Section 4.3 to say that the check is made during error checking rather than “during processing of a Signature Segment”. Also, in the error checking list in Section 5.2, we have added a new check that reads: “If the update message was received from a BGPsec peer that is a member of the BGPsec speaker's AS confederation, check to ensure that the Secure_Path segment corresponding to that peer contains a Flags field with the Confed_Sequence flag set to one.”

m12. In Section 7.4. (Additional Security Considerations), please add a reference to point at “appropriate transport security mechanisms.”, and/or point at the Security Considerations from rfc4271.

[Sriram] Done.

Nits:

n1. In several places the text uses “we” (for example: “we expect…”).  It’s just a matter of style, but using the third person may be more appropriate (for example: “it is expected…”).

[Sriram] Done. Changes made per your suggestion.

n2. The use of “Target AS Number” is inconsistent: sometimes the “number” is capitalized, others it isn’t, and sometimes it is not even mentioned.

[Sriram] Now the document used ‘Target AS Number’ consistently.

n3. I don’t think we need Section 6.2. (Extensibility Considerations).

[Sriram] Leaving it as is for now. Can be removed later if there seems greater conviction about it during the IESG review process.