Re: [netconf] AD review of draft-ietf-netconf-sztp-csr

[Kent Watsen <[e-mail]>]

Hi Rob,

Thank you for your review!


> AD Review of draft-ietf-netconf-sztp-csr-03
> 
> Disclaimer, I am not a security expert and the security specific aspects
> (e.g., the precise details of how the CSRs are encoded) is beyond my knowledge.
> Hopefully this will be sufficiently checked by SEC DIR reviews and SEC AD,
> whilst also acknowledging the expertise of the authors!

Ack.


> My main questions really relate to REST semantics.
> 
> 1. It is always the case that the SZTP-client must do the handshake with
> the server (i.e, populating the csr-support container, then getting a 400 error back,
> before making the actual bootstrap request), or is it acceptable for the SZTP-client
> to guess on acceptable parameters, and send a single request?  Obviously, this cannot
> work in all cases, but I want to check whether it is allowed (or explicitly disallowed)
> in any cases.

A handshake is not strictly always required, as:

1) it’s possible the device could automatically generate a CSR using the same certificate information as present in its IDevID certificate.  This would allow the signature on the resulting LDevID to be from the deployment’s CA infrastructure, which has value in and of itself, but the LDevID would not contain any new/different/interesting fields, and hence might have limited appeal.

2) it’s possible that the device could automatically generate unique keys and associated p10, cmc, cmp structures (i.e., every permutation of key-algorithm and request-structure the device supports), but this would be expensive on the device, especially without knowledge if the deployment’s infrastructure even cares to set an LDevID on the device.

For these reasons the authors felt it best to introduce the dialog presented in the draft.



> 2. Section 2.2:
>   Assuming the SZTP-server wishes to prompt the SZTP-client to provide
>   a CSR, then it would respond with an HTTP 400 (Bad Request) error
>   code:
> 
> I wonder whether returning a 400 "Bad Request" error is really the best return code, i.e., 
> it wasn't clear to me whether this requesting the capabiltiies is really an error.
> Did you consider potentially using other return codes?  Possibly:
>  300 Multiple Choices,
>  403 Fobidden,
>  406 Not Acceptable

I did before look at all the 4xx codes.  I was initially drawn to 412 Precondition Failed, but noted that it is specific to HTTP request header fields.   As for the others you mention, the semantics of 403 Forbidden is that the request should not be repeated, which isn’t out case, and 406 Not Acceptable regards the use of the HTTP “Accept” headers, which aren't in play here either.  That said, 300 Multiple Choices does appear to be a better if not perfect option, so I made that change in my local copy.



> 3. Does it make sense to recommend a default certificate-request-format that clients should
> support, or does it make sense to not prioritize any particular certificate format, effectively
> requiring that a generic server needs to support them all?

No default format is possible, as it’s completely dependent on what CA infrastructure the particular deployment has.

The impact to generic servers is minimal as, for the most part, they simply pass the structure to the deployment’s CA and subsequently relay the response structure to the device.  Only if the SZTP-server wishes to inspect that contents would it then need to step up to understanding all the formats.


> 4. References.  RFC 3688 probably should be normative for the XML registry, noting that
> RFC 6020 is normatively referenced.

Really?  I tend to disagree as I thought the litmus test for if something were normative was if it had to be understood in order to implement the RFC.

You raise an interesting point about RFC 6020 being Normative, as I see that it is NOT normative in at least one of my other drafts.  I think that it is a reasonable position that, if the RFC 6020 reference is only present in the IANA Considerations section, then it is INFORMATIVE as far as the draft as a whole goes.  Thoughts?



> Nits:
> 
> 5. 
> In the example data, would it helpful to add a sentence to explain
>               "base64encodedvalue1=",
>               "base64encodedvalue2=",
>               "base64encodedvalue3="
> 			
> Actually, given this convention is used in various places, there is a choice as to whether
> to add a sentence to each example, or perhaps in the introduction.

Adding a sentence to each example would be ideal, but a ton of work, especially considering my needing to propagate it to the suite of client-server drafts as well.

Adding something like the following to the Introduction section wouldn’t be too much work:

1.3.  Conventions

   Various examples used in this document use a placeholder
   value for binary data that has been base64 encoded (e.g., 
   "base64encodeddata==“).  This placeholder value is used
    as real base64 encoded structures are often many lines
    long and hence distracting to the example being presented.

Thoughts?


> 6.
> 3.1.3.  Replay Attack Protection
> 
>   This RFC enables an SZTP-client to announce an ability to generate
>   new key to use for its CSR.
> 
> "a new key"

Fixed


> 7. In Security Considerations:
>   Generating a new key each time enables the random bytes used to
>   create the key to serve the dual-purpose of also acting like a
>   "nonce" used in other mechanisms to detect replay attacks.
> 
> I wasn't clear to me what the two purposes are here.  One is acting like
> a "nonce", but what is the other purpose?

:laugh:  The original first purpose is, of course, to generate a new key at all, as opposed to each time.   Please provide suggested text if you still think it can be improved.



> 8.
>   In the case the SZTP-client must choose between the asymmetric key
>   option versus a shared secret for origin authentication, it is
>   RECOMMENDED that the SZTP-client choose using the asymmetric key
>   option.
> 
> "In the case the" => "In the case that the ...

Fixed.


> Terminology:
> Should you add LDevID or IDevID

Maybe, but note that this draft (as with RFC 8572) carefully refers to “identity certificates”, of which IEEE 802.1AR DevIDs are mere examples of such, but by no means the only possibility.   It’s unclear if promoting such an ancillary reference to a top-level term is appropriate.  Currently, as I’m sure you’re aware, the draft has a reference (i.e., an <xref>) where these values are used…admitted, likely more times than necessary, such that I wouldn’t be surprised if the copyeditor were to eliminate all but the first references.  Thoughts? Please advise.


> Potential spelling warnings:
> descendent

Fixed.


> Grammar Warnings (from tool):
> Section: 2.3, draft text:
> This module augments an RPC defined in [RFC8572], uses a data type defined in [I-D.ietf-netconf-crypto-types], has an normative references to [RFC2986] and [ITU.
> Warning:  The plural noun "references" cannot be used with the article "an". Did you mean an normative reference or normative references?
> Suggested change:  "an normative reference"

Fixed.


> Section: 3.1.4, draft text:
> Consistent with the recommendation presented in Section 9.6 of [RFC8572], SZTP-clients SHOULD NOT passed the "csr-support" input parameter to an untrusted SZTP-server. 
> Warning:  The modal verb 'SHOULD' requires the verb's base form.
> Suggested change:  "pass"

Fixed.


> Section: 3.1.5, draft text:
> All of the certificate request formats defined in this document (e.g., CMC, CMP, etc.), not including a raw PKCS#10, support origin authentication.
> Warning:  Consider using all the.
> Suggested change:  "All the"

Fixed…and I learned something new :)


> Section: 3.1.5, draft text:
> Typically only one possible origin authentication mechanism can possibly be used but, in the case that the SZTP-client authenticates itself using both TLS-level (e.g., IDevID) and HTTP-level credentials (e.g., Basic), as is allowed by Section 5.3 of [RFC8572], then the SZTP-client may need to choose between the two options.
> Warning:  Did you forget a comma after a conjunctive/linking adverb?
> Suggested change:  "Typically,"

Fixed.


> Thanks,
> Rob

K.