I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please wait for direction from your document shepherd or AD before posting a new version of the draft. For more information, please see the FAQ at . Document: draft-ietf-sidr-bgpsec-protocol-20 Reviewer: Russ Housley Review Date: 2016-12-07 IETF LC End Date: 2016-12-16 IESG Telechat date: Unknown Summary: Almost Ready I was involved in the design of BGPsec, but I have not reviewed the document for a long time. I read it for Gen-ART today. I hope these comment will improve the document before. Question I do not see any guidance on the handling of private AS numbers. I'm not sure whether it belongs in the document or not. I am thinking about a BGP stub customer that employs a private AS. If I understand properly, that stub customer cannot publish a ROA for the private AS number and the prefixes that it uses. So, the stub customer cannot become a BGPsec speaker. So, my question is about the BGPsec speaker that receives an announcement from the stub customer. Does that BGPsec speaker strip the private AS and sign an announcement? Will their ROA that cover the prefixes used by the stub customer? Major Concerns Section 2.2 says: ... A BGP speaker MAY announce BGPsec capability only if it supports the BGP multiprotocol extension [RFC4760]. ... This needs to be worded as a MUST NOT statement. The current wording does not align with the definition of MAY in RFC 2119. I think an additional consideration is needed in Section 6.2. This protocol design assumes a signer will compute a message digest and then digitally sign that digest. If someone wants to use a digital signature that works differently, there may be a significant change to this protocol. It is very unusual for the IANA Considerations to contain a figure, and Figure 10 really seems out of place. The version number can be put in an IANA registry, but there really is no need until a second value is needed. There is no reason to put Dir in an IANA registry, there are only two values and both are fully specified in this document. The unassigned bits must be zero until a new version is specified. Minor Concerns I find the Abstract misses some important information. Also, the old wording suggests that the attribute contains a single digital signature. I suggest: This document describes BGPsec, an extension to the Border Gateway Protocol (BGP) that provides security for the path of autonomous systems (ASes) through which a BGP update message passes. BGPsec is implemented via an optional non-transitive BGP path attribute that carries digital signatures produced by each autonomous system that propagates the update message. The digital signatures provide confidence that every AS on the path of ASes listed in the update message has explicitly authorized the advertisement of the route. Section 2.1 says: ... The BGP speaker sets this bit to 0 to indicate the capability to receive BGPsec update messages. The BGP speaker sets this bit to 1 to indicate the capability to send BGPsec update messages. It seems a bit wasteful to repeat the whole capability for each direction. Wouldn't it be better to follow the example used in other capability definitions (such as RFC 7911) by using one of the unassigned bits? The Send/Receive pair of bits would have these semantics: This field indicates whether the sender is (a) able to receive BGPsec update messages from its peer (value 1), (b) able to send BGPsec update messages to its peer (value 2), or (c) both (value 3) for the address family identified in the AFI. For completeness, please add a paragraph to the end of Section 3.1 that describes the 4-octet AS Number field. Please state how an old 16-bit AS number is carried. In Section 3.2, the Signature Length within the Signature Segment does not count the length field itself. It seems that all of the other length values in this specification count the size of the length too. Consistency will avoid implementation errors. Section 4.2 references the MP_REACH_NLRI attribute; please add a citation to the RFC that defines that attribute. Nits Section 2.1 says: The second and third octets contain the 16-bit Address Family Identifier (AFI) which indicates the address family for which the BGPsec speaker is advertising support for BGPsec. This document only specifies BGPsec for use with two address families, IPv4 and IPv6, AFI values 1 and 2 respectively. BGPsec for use with other address families may be specified in future documents. Please reference RFC 4760 for a place to learn about AFI. In Section 4.1, the comma is in the wrong place, but when I tried to offer a correction, I thought that a slight rewording would also improve the clarity: OLD: If a BGPsec router has received only a non-BGPsec update message (without the BGPsec_Path attribute), containing the AS_PATH attribute, from a peer for a given prefix then it MUST NOT ... NEW: If a BGPsec router has received only a non-BGPsec update message containing the AS_PATH attribute (instead of the BGPsec_Path attribute) from a peer for a given prefix, then it MUST NOT ... Please reword the first paragraph of Section 4.2 to avoid the phrase "said route advertisement". While it is not wrong, it does feel very awkward. This is not a patent claim. The acronym "ASN" is not used until Section 4.4, and it is not expanded there. I suggest that all uses of ASM should be expanded to AS number.