Re: [dtn] AD review of draft-ietf-dtn-bpsec-default-sc-02

["Birrane, Edward J." <[e-mail]>]

Zahed,

  Thank you for this detailed review!

  I have included my responses in-line below and have enumerated your comments using the convention C# for clarity.

  I have also posted a -03 which should address comments below.


> * Abstract :
> 
>     I would suggest to replace "may be" with "intended to be".

C1: Agreed. This change was made in the -03 version of the document.


> * Terminology :
>      Section 4.6 : please check in normative text are correctly following
> RFC8174.
>      Section 6.1 : please check in normative text are correctly following
> RFC8174.

C2: Agreed. I have looked through these sections and made adjustments to the wording in the -03 version.


> * It would be good for the user of this document to capture the reason
> behind selecting HMAC-SHA and AES-GCM in respective overview section
> (section 3.1 and section 4.1).

C3: Agreed. I have added reasons for selection in 3.1 and 4.1 in the -03 version.


> * Section 3.1 :
>       Names does not match table names in RFC8152. I understand it might be
> desirable to use the longer form that makes it clear that this is
> SHA256/384/512 version of HMAC. However, in that case I think an explicit
> mapping needs to occur in this document.
> 
>       Again the name does not match how it is written in section 3.1 and 3.3.1.

C4: Agreed. I have changed the names in the -03 version of the document to correspond to the names used in RFC8152.

 
> * Section 3.3.2 and Section 3.5 :
> It seems like some sort of key-id might be required to resolve
> this. The BPsec does not define any generic key-id and it is neither defined
> here. It might become an issue for interoperability.

C5: Agreed this might become an issue for interoperability.  For example, if the policies for requiring security operations differ on different nodes there may be interop problems. Similarly, if the policies for key generation/communication differ on different nodes, there may be interop problems. The DTNWG consensus was this limitation is appropriate for now for this default security context and the DTN WG is looking to determine delay-tolerant key management strategies in the future. 


> * Section 3.3.3:
>    Two aspects here -
>     1. Why are so many bits used for flags? I tried to back track it to BPsec and
> BP documents but I am not sure I found what I was looking for.

C6: Agreed. We do not need this many options. It has been reduced in the -03 version of the document.


>     2. if the plan here is to assign in future (bits 8-15) then an procedure need
> to be in place to assign them (even in an abstract form). The future
> assignment will also impact 3.3.6 and 3.7 as the unknow bits will lead to
> failure as the flag value is part of the IPPT and receiver does not know what
> to add based on the bits. What is the thinking here?

C7: Agreed and do not mean to imply that the default security contexts should require assigned fields. This has been addressed in the -03 version of the document.


> * Section 3.3.4:
>     I would suggest to add a reference to BPsec 3.1 here.

C8: BPSec 3.1 describes the definition of the BIB and BCB block, but I think we can assume the reader understand these definitions.


>     Nit: ID 1 should have default value 6, or if the intention here is to use
> SHA256 then it should be 5 instead (as mentioned in the early SECART
> review).

C9: Agreed. I have adjusted this value in the -03 version of the document.

 
> * Section 3.6 :
>      Is there any particular reason for defining the flag bits in the order  -
> primary block flag, target header flag and security header flag but for
> canonical form process the order is primary block flag, security header flag
> and target header flag?

C10: No reason  and agree this is confusing.  I have addressed this in the -03 version of the document.


>     Nit: in the process description text "flag" is written in both uppercase and
> lower case.

C11: Agree that the capitalization is nonuniform for several items. I have made a pass through the -03 version of the document to try and normalize the use of capitalization.


>     I assume that security acceptor and verifiers will have to use the actual
> value provided in the Integrity scope flags unless they are correctly
> configured. This sounds like the local flags used to generate the IPPT MUST
> be placed in the Integrity Scope flags. Is there any reason why this is SHOULD
> not a MUST?

C12: The thinking is that if default flags are used, then there is no reason to include the flag regardless of the configuration of a verifier or an acceptor.


 
> * Section 3.7.2 :
>       This section should also say any error on verification should result in
> failure. Something similar to the text in the section 4.8.2

C13: I have updated the language in the -03 version to try and address this comment.

 
> * Section 4 :
>      Several of above mentioned issues are also applicable to subsections of
> section 4.

C14: Agreed. Comments from section 3.* that also relate to section 4.* have been addressed in the -03 version of the document.


> * Section 4.3.2 :
>      Does this "security policy" refers to local security policy?

C15: Yes.  Security policy at the source, verifier, or acceptor is meant to imply the local security policy at the node.


> * Section 5:
>       As mentioned before registries for Scope flags will be necessary.

C16: Similar to C7, there is no intent to define such a registry and the -03 version of the document corrects this.

 
> * Section 6.3
> 
>      To me this security considerations for fragmentation belongs to BPsec as
> the main technical issues on fragmentation are described there.

C17: I would support removing the fragmentation section from this document and including it in some other broader-scoped document on DTN security, though perhaps not the BPSec document itself because the issues with fragmentation can extend beyond the insertion and handling of security blocks. 


> I would also like to draw attention to the SECART review (Thanks to the
> SECART team and Christian Huitema) proposing test vectors to mitigate
> interoperability issue pointed out by the reviewer. This seems like a good
> proposal to include in this specification.


C18: Agreed and grateful for the review! As mentioned in my comments to the SECART review and similar to C17 above, I think that example encodings are a very useful concept, but would ultimately be useful at showing bundle structure and security block structure. Rather than repeating such encodings in all future security context documents, I propose the DTN WG consider the best way to produce this broader-scope documentation.