Subject: [sidr] comments on draft-ietf-sidr-bgpsec-protocol-14 -- data covered by signature

From: "Sriram, Kotikalapudi" <[email-masked]>
Date: Mon, 4 Jan 2016 15:57:25 +0000

Happy New Year!

I have given it (version 14) a complete read. Thanks, Matt, once gain for all your efforts.
My comments are split over two posts. 
This is the first post that seeks to clarify a technical point. 
My second post will have comments/suggestions to help improve 
the clarity/presentation in the document, and also lists some typos and nits.

The following were the three possibilities for a protocol change to address the 
problem that David identified: 
1. Signature protects the Target ASN, {ASN, pCounts, Flags} of the signer, 
Previous Secure_Path, and {AFI, SAFI, NLRI}.   
2. Signature protects all of the data in #1 and the most recently added Signature_Segment 
(i.e. that of the eBGPsec speaker from whom the update was received).
3. Signature protects all of the data in #1 and the Previous Signature_Block 
(i.e. all previous Signature_Segments). 

David initially proposed #1.
http://www.ietf.org/mail-archive/web/sidr/current/msg07258.html  
Following on that, Rob made a case for #2.
http://www.ietf.org/mail-archive/web/sidr/current/msg07261.html  

I don’t think #3 was discussed in that thread. Nevertheless it is a solution candidate.
#3 is what Matt has included in the revised draft.
The relative merits of #2 and #3 are worth a little discussion.   
 
One thing to note is that ECDSA-P256 signature length is about 72 octets
(including DER (ASN1) format overheads); plus 22 octets for SKI and Sig length.
So a total of 94 octets of additional data (per signature) is to be included 
in the hashed data at BGPsec routers.
In the case of #2, the *additional data* to be hashed would be a constant at 94 octets,
(independent of the AS path length), while in the case of #3,  
it would be variable given by #AS (i.e. previous path length) x 94 octets.
So, for example, 940 octets more (for #3 over #2) if the previous AS path length is 10.
Is this of concern? One good thing is that the Signature_Segments to be added
to the hashed data are adjacent (that helps in consideration of #3). 
Adjacency helps with lowering # CPU cycles consumed for marshaling the data for hash (for #3).

We have preliminary measurement data which shows that the performance for 
SHA-256 hash operations on Intel NUC 3GHz (single threading) is as follows:
(we will be conducting this study also on a 3.5 GHz server with a lot more RAM):
Hash input data size |  time per SHA-256 hash operation   | hash operation speed |  
50 octets |  0.34 usec  |  2,923,662 hash ops/s
100 octets | 0.58 usec  |  1,716,556 hash ops/s 
500 octets | 2.02 usec  | 493,969 hash ops/s
1000 octets |  3.93 usec  | 254,462 hash ops/s
5000 octets |  17.21 usec  | 58,109 hash ops/s
(Averaged over 1000 iterations;  usec = microseconds)

For a long previous-AS-path length of 10, with the choice of #2, 
we’ll be in 300 octets ballpark for hashed data size, and
with the choice of #3, we’ll be around 1200 octets ballpark for hashed data size.
So, to me it looks like #3 choice does not have a significant penalty
over #2 choice in terms the performance of hashing operations
for the range of hashed data sizes of our interest.
It is true that the hash processing time more than doubles for #3 compared to #2 in the 
hash data size ranges of interest. However, since all other BGPsec update processing 
is likely to dominate over the hash processing time, the concern with regard to 
hash performance may not be huge when comparing #3 with #2.
The adjacency assumption of the multiple Signature_Segments for hashing is important
for #3, and should hold (hopefully) in the implementations.
 
Your thoughts/inputs on the merits/trade-offs of #2 vs. #3? Please share them.

Thank you.
Sriram

---

From: "Sriram, Kotikalapudi" <[email-masked]>
Date: Mon, 4 Jan 2016 16:40:41 +0000

Matt,

I hope you find these comments helpful.

Comments/Suggestions to help improve clarity/presentation:
-----------------------------------------------------¬--------------------------

Page 3, 3rd para: add [20] next to [19]; both are relevant references.

Page 5, 1st para: s/…for the same AFI combination…/…for the same AFI …/

Page 8, 1st para:  s/(i.e., the number of  AS numbers in the path)/
(i.e., the number of distinct AS numbers in the path)/

Page 14, 1st para: At the end of this para, within the parentheses, I suggest adding: 
See Section 4.4 for an algorithm for reconstruction of AS_PATH attribute.

Page 15: 2nd last para: 
s/…populated with the length (in octets) of the Signature field/
…populated with the length (in octets) of the value in the Signature field/

Page 17, 2nd para from bottom: 
s/ …if the next most recently added Secure_Path segment…/
…if the immediately following Secure_Path segment…/

Note that "next most recently added Secure_Path segment" connotes 
"immediately preceding Secure_Path segment" (i.e. going backward in time). 
It seems to me, that is not what was intended here.

Page 18, 1st para: At the end, add the following: (see Section 5.2).

Page 18, please append the following at the end of the 1st para:
Before attempting to reconstruct an AS_PATH for the purpose of forwarding 
an unsigned (non-BGPsec) update to a peer, a BGPsec speaker MUST
perform the basic integrity checks listed in Section 5.2 to ensure that the received
BGPsec update is properly formed.

Page 19, last para: s/ via the RTR protocol/ via the RPKI-to-Router (RTR) protocol [15]/

Pages 20-21, Section 5.1: The mention of ROA and the second bullet item (top of page 21) 
are not needed here, because ROA is not used in BGPsec validation (origin validation does that).

Page 22, last para:
"If there is no Signature_Block
   corresponding to an algorithm suite that the BGPsec speaker supports,
   then the BGPsec speaker MUST treat the update message in the same
   manner that the BGPsec speaker would treat an (unsigned) update…"

Note that the former (without supported algorithm suite) is not the
same as unsigned, so they cannot be treated in the same manner.
So I think it would be more precise to say it the following way:
"If there is no Signature_Block corresponding to an algorithm suite that 
the BGPsec speaker supports, then the BGPsec speaker MUST 
process the update message as follows: First, convert the Secure_PATH
attribute to AS_PATH attribute, and then treat the update
as if it was received unsigned (non-BGPsec)."

Page 23, in the Figure: s/Rest of Secure_Path/Preceding portion of Secure_Path/

Note that "Rest of Secure Path" implies Secure_Path segments before and after the current one. 
Minor modifications in the text on page 24 also need to be made,
wherever "Rest of Secure_Path" is mentioned. 

Page 25 (1st full para):
"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'.  (That is, if a BGPsec update message contains
   two Signature_Blocks then the update message is deemed 'Valid' if the
   first Signature_Block is marked 'Valid' OR the second Signature_Block
   is marked 'Valid'.)"

I think we cannot say here "…BGPsec update message is deemed to be 'Valid'."
This document only specifies validation for Signature_Block (or Secure_Path).
It is up to the operator to combine the result of RFC6811 (origin validation)
with the results of this document (path validation) to decide about
the overall validity of the BGPsec update message.  
So, instead of "is deemed to be 'Valid' ", should we say "is deemed to be 'Path Valid' "?

Page 27, 1st paragraph (below the bullet): 
"That is, the recipient of a valid BGPsec Update message is assured
   that the Secure_Path portion of the BGPsec_Path attribute corresponds
   to a sequence of autonomous systems who have all agreed in principle
   to forward packets to the given prefix along the indicated path.  (It
   should be noted that BGPsec does not offer any guarantee that the
   data packets would flow along the indicated path; it only guarantees
   that the BGP update conveying the path indeed propagated along the
   indicated path.)"

I find the first sentence problematic. In essence, it contradicts the second sentence.
The first sentence says BGPsec assures something about the data plane.
The second sentence says BGPsec does not guarantee anything about the data plane!
I would reword the first sentence as follows:
"That is, the recipient of a valid BGPsec update message is assured that 
the update propagated via the sequences ASes listed in the Secure_Path 
portion of the BGPsec_Path attribute."

Page 27, 1st paragraph (below the bullet), in the last sentence:
s/ the recipient is assured that this path terminates.../ the recipient is assured,
due to origin validation, that this path terminates…/

Note: I suggest this wording because BGPsec does not provide this 
particular assurance; origin validation does.

Page 30, 2nd para (last para of Section 7.4): 
This appears to be a carryover from v.13 draft since it refers to old Sections 4.1 and 4.2. 
It is also worth noting that the extremely unlikely attack 
being mentioned here is made even more unlikely due to 
more data that needs to be signed over at the second AS (based on this v-14 draft).
     
===========
Typos, Nits:
---------------

Page 12, 3rd para: s/…update message has not been that has not successfully validated/
…update message has not been successfully validated/

Page 15, 1st para: same sentence twice in the same para … 
"the Target AS Number is the AS to whom the BGPsec speaker intends to send the update message"
Similar sentence appeared earlier in the same para at bottom of page 14).

Page 16, 1st para: The following phrase is repeated twice in the same para:
"when a confederation member sends a BGPsec update message to a peer 
that is a member of the same confederation"

Page 16, 1st para: The following phrase is repeated twice in the same sentence (last sentence):
"in a non-BGPsec update message"

Page 25, 2nd para in Section 6.1:
s/a mandatory algorithm suites document will be created/  
a mandatory algorithm suites document exists/ 

References: [9] should have date of November 2015 (it shows Nov. 2014).

Page 17 and p.22:    s/RFC WXYZ/RFC 7606/

Global substitution: s/AS_Path/AS_PATH/g

Global substitution: s/BGPSEC/BGPsec/g

Thank you.
Sriram