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

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Kent,

From: Kent Watsen <[e-mail]>
Sent: 24 June 2021 16:26
To: Rob Wilton (rwilton) <[e-mail]>
Cc: [e-mail]; [e-mail]; [e-mail]
Subject: Re: AD review of draft-ietf-netconf-sztp-csr

Hi Rob,

Pruning down to just interesting bits…



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.

Okay, I'm wondering whether a sentence to that effect would be helpful, since when I read the draft, my impression was that a handshake was always expected, but perhaps I'm missing something?

I had an offline chat with my co-authors.

As the YANG syntax stands, it’s possible for the SZTP-client to proactively send a CSR in it’s first message, but the value is so low that it’s almost a SHOULD NOT…and, if we’d caught it before, a “choice” statement may have been used to make it impossible (in order to simplify implementations).

That said, the first paragraph of the “description" statement for "container csr” states that a “csr” is sent in response to a “request-info” structure from the SZTP-server, which suggests that only a “csr-support” node can be proactively sent in the first message.  That only issue is that it doesn’t state that it is *only sent* in response to a “request-info” structure, leaving open a possibility that the first message MAY include both the “csr-support” *and* “csr” nodes.

Looking at just the draft-text (i.e., Ignoring the YANG), Section 2.1 says "In the order of their intended use”, which is suggestive, but the word “intended” leaves open the possibility for alternate uses.

My recommendation is to 1) remove the word “intended”, 2) add a choice statement to the YANG around the “csr-support” and “csr” nodes., and 3) strengthen the language in the last paragraph on page 4 to be more like the YANG, indicating that the “csr” node is sent only in response to a “request-info” from the SZTP-server.

Thoughts?

I’m not sure.  It feels less RESTful (at least to me) to not allow the data to be provided as a single message, even if in practice that might not make sense in most (almost all) cases.  If you were using a separate YANG RPC to send the crs-support and get the request-info then only including the crs node in the bootstrapping data RPC then I would have no complaints.  I.e., it only feels strange to handle a transactional dependency within the data structures of a single RPC on what is nominally a stateless API.

Also, worth noting that using a separate YANG RPC to transmit the capabilities avoids the need to return an error.  It could just return success with the required information.  Was this approach considered at all, and would this be too big a change?  I’m not trying to create extra work for you, or delay this document …

If a separate RPC to get the capabilities in undesirable or infeasible then changing to a choice seems reasonable, but I would probably suggest keeping “intended” and probably keeping the language on page 4 somewhat open, i.e., not strictly locking it down to doing a handshake.

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.

I guess the logic here is that you need to understand RFC 6020 to understand the registry that is having entries added (given that RFC 7950 doesn't define the registry).

But, as per below, I'll check with the IESG on this one.



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?

Let me check with the IESG and get back to you on this one.  Although, perhaps worth noting that both these references are normative in RFC 8343 (and they are only referenced from the IANA registry entries).

Ack, I will wait for your response on this item.

I didn’t really get a strong answer from other ADs on this.

Some of us felt that a normative reference is appropriate (because you need to understand the meaning of the registry that is being updated), and nobody objected to that characterization, but equally, some ADs didn’t think that Informative is necessarily wrong and it is probably not worth worrying about.

My recommendation is to make them normative, because that is what has been done for other RFCs with YANG modules (and consistency is good), and I don’t see any other ADs objecting to this approach.  I would regard this as the path of least resistance.

Thanks,
Rob




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?

Looks good.

Applied.



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.

When you put it that way, it is kind of obvious ;-)

You can either leave it as is, or perhaps a small tweak (moving also) might make it clearer:

  Generating a new key each time enables the random bytes used to
  create the key to also serve the dual-purpose of acting like a
  "nonce" used in other mechanisms to detect replay attacks.

Applied.



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.

Okay.  I'm happy for you to leave this as is, this is pretty minor.

Leaving as is for now, but on the fence and would welcome thoughts from others.




Thanks,
Rob

K.