Last Call Review of draft-ietf-ace-coap-est-15
review-ietf-ace-coap-est-15-secdir-lc-sheffer-2019-10-13-00

Request Review of draft-ietf-ace-coap-est
Requested rev. no specific revision (document currently at 18)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2019-10-18
Requested 2019-10-04
Authors Peter Van der Stok, Panos Kampanakis, Michael Richardson, Shahid Raza
Draft last updated 2019-10-13
Completed reviews Genart Last Call review of -15 by David Schinazi (diff)
Secdir Last Call review of -15 by Yaron Sheffer (diff)
Secdir Telechat review of -17 by Yaron Sheffer (diff)
Assignment Reviewer Yaron Sheffer
State Completed
Review review-ietf-ace-coap-est-15-secdir-lc-sheffer-2019-10-13
Posted at https://mailarchive.ietf.org/arch/msg/secdir/pD-jJhb79q4faIn9ekoYXY7oAx4
Reviewed rev. 15 (document currently at 18)
Review result Has Issues
Review completed: 2019-10-13

Review
review-ietf-ace-coap-est-15-secdir-lc-sheffer-2019-10-13

This document defines a new profile for EST (simple certificate enrollment) for restricted devices, to be run on top of CoAP and DTLS, instead of the usual HTTP and TLS.

Overall, I tend to be suspicious of "restricted" profiles, and this is a case in point. An implementation of DTLS is no simpler than TLS and most would probably support both protocols. And basically anything supports HTTP. So why would it make sense to define a whole new profile just to avoid using TCP very rarely (say, for daily certificate enrollment), when this profile even needs to include its own fragmentation/buffering mechanisms because the ASN.1 payloads are too long? In other words, we are introducing new and additional complexity with little or no performance gain.

- 2. Only certificate-based client authentication is supported. Hopefully some time soon we'll be able to use PAKE here, to bootstrap the PKI.

- 4. "Crypto agility is important" - this statement is somewhat meaningless: do we require more than one cipher suite, which would ensure some level of agility?

- Also, when we say "Sec. 4.4 of RFC 7925" do we mean *only* Sec. 4.4 or also the subsections: 4.4.1 etc.

- "in DTLS 1.3 the Supported Elliptic Curves extension has been renamed to Supported Groups". This is probably true for an older version of the draft, I couldn't find anything to support it in -32.

- "the Finished message must be computed" - this whole paragraph is unclear. Are we changing the Finished/MAC computation in DTLS (if so, this must be pointed out very clearly)? Or are we explaining a point about channel binding (if we are, this doesn't come across).

- Similarly for the following paragraph: is this a performance  optimization, or a change to DTLS? And by the way, why are standard session resumption mechanisms not used?

- I don't see value in the short EST URI paths given that most of the "weight" is in ASN.1 data, and the paths are negligible in comparison. Moreover, if the paths are different, shouldn't this document formally UPDATE RFC 7030? I think it is no longer a profile.

- Non-default server port: the examples include an IPv6 address in addition to the port number. Is there a way to use relative URIs (omitting the IP address) but include a port number? The server may not know its own IP address (IPv4 with NAT) or the address may be dynamic.

- The server MUST support the default /.well-known/est root resource, but then in the next sentence	we discuss non-default URIs. In the case of the server having multiple "identities" (the main purpose of the "arbitrary label", AFAIU), the root resource is confusing - which identity is it associated with? And then how is discovery managed for each identity, and is there a way to discover the identities?

- There is nowhere in this document a full formal definition of content type TBD287 (a single cert).

- Content type negotiation: I can see how it works for enrollment. But in the case of /cacerts, if the server has a list of certs in its explicit TA store (e.g. to support rollover), how can TBD287 (a single cert) make sense?

- It is mighty confusing to denote "content format" by "ct" (presumably, this stands for "content type").

- "ASN.1 encoded in binary format" - I think this should be, "DER-encoded ASN.1, in its native binary form".

- Please don't use "he" and "she" for the server and the client. Both are merely "it".

- "The Registrar MUST authenticate and optionally authorize the client requests" - this statement is too weak. The Registrar must also ensure that clients only send CSRs that pertain to their authenticated identity (channel binding), even when POP-linking is not used. I think this is worthy of its own subsection, describing the validation required for each particular EST flow.

- The following paragraph is not clear: is PoP-linking useful/recommended/mandated in this scenario? IMO there should be some 2119 word regarding server-side validation of the tls-unique.

- "Table 1 contains the URI mappings between EST-coaps and EST that the Registrar MUST adhere to." But the immediately preceding paragraph describes a case where a server-side generation on the EST-coaps side is mapped to client-side generation on the EST-HTTPS side, which is not a Table 1 mapping!

- "the encrypted CMS EnvelopedData blob MUST be converted at the Registrar" - can this be done without decrypting the blob, IOW must the Registrar be privy to the key wrapping key?

- The Registrar must support resource discovery: does it mean that resource discovery messages are simply proxied upstream, or that the Registrar presents a simpler resource structure (maybe with no discovery) to its clients while performing discovery upstream?

- Security Considerations: there's a long discussion about replacing the implicit TAs with explicit ones. If we (rightly) mandate that the client re-verify the server's cert after getting the /certs response, we are losing most of the minor performance gain that keeping the DTLS connection open might have given us. So why not unambiguously mandate the simpler "MUST close the connection after /certs" instead? Besides, /cacerts is a very rare operation. Why optimise it at all?

- "Improper CSR requests" - what are they? What's the server supposed to do? Please give a reference.

- A.3, response: I may be missing something, but the binary blob "3081...9fd9" does not parse as PKCS#8 to me.