Hi Esko, Many thanks for sending this review! I am responding with some delay due to the vacation period. I have converted your points below into github issues and plan on handling and discussing them on github. I tagged you so you should have received a bunch of notifications related to it. For reference, here is a list of issues I created:
https://github.com/ace-wg/est-oscore/issues/55 https://github.com/ace-wg/est-oscore/issues/56 https://github.com/ace-wg/est-oscore/issues/57 https://github.com/ace-wg/est-oscore/issues/58 https://github.com/ace-wg/est-oscore/issues/59 https://github.com/ace-wg/est-oscore/issues/60 https://github.com/ace-wg/est-oscore/issues/61 https://github.com/ace-wg/est-oscore/issues/62 https://github.com/ace-wg/est-oscore/issues/63 https://github.com/ace-wg/est-oscore/issues/64 https://github.com/ace-wg/est-oscore/issues/65 https://github.com/ace-wg/est-oscore/issues/66 https://github.com/ace-wg/est-oscore/issues/67 https://github.com/ace-wg/est-oscore/issues/68 https://github.com/ace-wg/est-oscore/issues/69 https://github.com/ace-wg/est-oscore/issues/70 https://github.com/ace-wg/est-oscore/issues/71 https://github.com/ace-wg/est-oscore/issues/72 https://github.com/ace-wg/est-oscore/issues/73 https://github.com/ace-wg/est-oscore/issues/74 https://github.com/ace-wg/est-oscore/issues/75 https://github.com/ace-wg/est-oscore/issues/76 https://github.com/ace-wg/est-oscore/issues/77 https://github.com/ace-wg/est-oscore/issues/78 https://github.com/ace-wg/est-oscore/issues/79 https://github.com/ace-wg/est-oscore/issues/80 Again, many thanks for doing this review! Mališa > On Aug 10, 2024, at 16:21, Esko Dijk <esko.d...@iotconsultancy.nl> wrote: > > Hi all, authors (cc), > > Here’s a first review of the document draft-ietf-ace-coap-est-oscore-05. This > is mostly based on my first read of the document; I didn’t look yet into all > the details or possible implementations of this technology. > Overall it looks like a useful addition to the constrained-networks toolbox, > where IoT devices already using OSCORE / EDHOC can benefit from EST without > needing additional security protocols/code. > > Detailed comments per section below: > > General (entire document) > > - There could be one line of whitespace between table content and table > caption. This would be visually more clear. > - Spelling “enrolment” vs “enrollment” - unify > > Abstract > > >> … is a certificate provisioning protocol over HTTPS. > It’s currently defined over HTTPS or CoAPS, not just HTTPS. (RFC 7030 / 9148 > respectively.) Best to mention both these transports already here in the > 1stsentence. > > >> … also leverages the certificate structures defined in > >> [I-D.ietf-cose-cbor-encoded-cert]. > Sentence could be clarified to say that the CBOR encoded certs are optionally > used! Now it’s not clear at high level if CBOR replaces X509 certificates > always or that it’s optionally used. > > 1. Introduction > > Paragraph 3 explains that EST can be carried over HTTP protected with OSCORE. > This part could still be clarified: I initially thought the intention was to > enable HTTP transport without needing CoAP at all i.e. in none of the > communication legs. This interpretation then conflicts with many following > parts of the document that only mention CoAP and not HTTP used by the client. > For example, paragraph 5 mentions “… context, the CoAP exchange carrying … “ > which then triggers the question whether this is really a CoAP exchange, or > whether it can also be a HTTP exchange? > Maybe the intention was to support only CoAP for the client side; which might > be carried as HTTP for a particular network segment? (e.g. from proxy to EST > server). > Not sure how to resolve this; it depends on the intention of how HTTP is > supposed to be used and this is not yet fully clear to me. > > >> … translating between between > (remove double word) > > >> * Compact representations f X.509 … > Compact CBOR representations of X.509 … > > Operational Differences with EST-coaps > > >> … the following respects: > the following aspects: > > There is some repetition in the text. E.g. paragraph 1 overlaps with bullet 1 > content. And bullet 2 subbullet 3 overlaps with bullet 2 of Section 1. I > think that Section 1 and 1.1 could be easily joined together and compressed. > (Or maybe I misunderstood the purpose of 1.1.) > > Bullet 2, subbullet 1: “is complemented with” -> unclear exactly what > “complemented” means here. Is using raw public keys an optional thing the > client can invoke? Or does it need to do both? > Similar questions for subbullet 2 and 3. > > 2. Terminology > > Section may expand acronym “DH” when used first time. > > >> Apart from enrolling signature keys, … > This is unclear to me! The purpose of EST was explained to be “certificate > provisioning” or “certificate enrollment” (assumed to be synonyms). Now it’s > signature key enrollment ? Doesn’t sound familiar and may need some more > details. Maybe it points to the fact that an identity (consisting of a > device’s public key and associated private key) gets enrolled into a domain, > at the same time that a certificate is provisioned into the device? Could we > say “apart from enrollment based on signature keys, … “ ? > > >> recipients public DH key > recipient’s public DH key > > >> Therefore this document … procedures defined. > Last sentence is complex and could be improved (grammar-wise). What does > “its” refer to? “defined” refers to something defined where? > > 3. Authentication > > >> During initial enrollment … payloads. > Is EDHOC only run during the initial / first enrollment – and then never > again? Or does it in some cases need to be “refreshed” before doing a > re-enrollment? > > >> … conveying EST payloads. > “carrying EST payloads” is maybe more clear? > > >> … to the CA for decision about … > “to the CA to support its decision about” ? Or “to the CA for the decisions > about” ? > > 3.1 EDHOC > >> certificates/raw public keys > Maybe better to clarify “certificates or raw public keys” ? > > >> … URI to the credential … > “URI of the credential” is perhaps more accurate? Since the I stands for > Identifier. It identifies the credential. > > 3.2 Certificate-based Authentication > > >> … client SHOULD populate its Explicit TA database … > There’s a requirement on the client here, but what should the client do > concretely? Are there some details needed on how to do this? And with whom > are the “subsequent authentications” made, only with EST servers or with any > servers/peers in the domain? (If it concerns EST server then it’s in scope of > this spec.) > And what are the exception cases of the SHOULD ? It’s also not so clear here > how this requirement differs from RFC 9148 requirements on Explicit and > Implicit TA database. > > >> … EST client certificate SHOULD conform to [RFC7925] … > That holds also for the server certificate, I think? If the server’s cert is > not lightweight, then just optimizing the client’s cert helps only for 50%. > RFC 7925 defines the profile for both client and server. > > >> … EST server certificate MAY be a (natively signed) CBOR certificate … > This MAY would imply that all clients MUST support the CBOR certificate – was > that intended? It would be better if the client could select either option > and the EST server would support both and adapt to the client’s choice. > If that’s not possible then having a defined “profile” can be useful – e.g. a > system implementation would need to pick either the X509 cert profile or the > CBOR cert profile and then all devices in the system including the EST server > support that choice according to the profile. > > 3.4 Optimizations > > There should be some introductory text before the bullets. E.g. explain there > will follow some optional behaviours that a client can be configured / > programmed to use, in order to reduce one of message size / round-trips / … . > Perhaps split into optimizations that a client can decide to use, and > optimizations that a server can decide to use? > > Difference between bullet 3 and 4 is not very clear yet. Bullet 3 mentions > “enrolled client certificate” – is this the domain certificate that’s created > due to the enrollment process? I suppose the client can’t use this > optimization when it’s not yet enrolled? > In bullet 4 > – is the advantage here that the client never has to actually access/download > its own domain certificate? E.g. it keeps using a reference to it, instead of > the actual certificate? Does this mean that all future/potential servers > and peers will have to support this “cert reference” instead of the real cert? > - when would the EST server return the reference rather than the full > certificate? It seems the client is also obliged to support the cert > references then, adding to its codebase. > > All of the “certificate reference” optimizations seem to complicate matters > in terms of interoperability. If device 1 MAY send a reference, this forces > device 2 to support (MUST) a method to get the certificate using this > reference to e.g. check it. (Is this correct?) > There should also be more detail about what from RFC 9360 is used for this – > e.g. specify the x5u header as the default option for this? (Now it’s just > mentioned as example.) > > 4. Protocol Design and Layering > > >> Block-Wise > -> “Block-Wise transfer” > > Figure 1 caption: best to mention the term “stack diagram” in the caption. > > 4.1 Discovery and URI > > The server uses the “osc” attributes to signal OSCORE support. This is fine, > however, the re-use of the existing resource type name “ace.est.sen” also > means it would support the original EST-coaps protocol. > This gives a clash in resource definitions when the EST server wants to > provide EST-with-OSCORE-over-DTLS which is stated as one of the options of > this protocol. > In this case, a regular EST-coaps client will see a resource of type > “ace.est.sen” which it recognizes and which is hosted at a URL that has a > coaps:// protocol, which it recognizes, and so it would assume this EST > server would support EST-coaps. (It would ignore the “osc” attribute, not > knowing it.) So this gives a conflict in resource type name use, because the > EST server maybe doesn’t intend to support EST-coaps classic and only intends > to support EST-OSCORE. But now it’s impossible to express this using link > format. (Or do we intend to make EST-coaps support mandatory for such case?) > > 4.2.1 /crts > > >> … which is subsequently installed in the Explicit TA. > “TA” here should be “TA database”. And it could be clarified that the > “installed in” part only happens of course if the server has been properly > authenticated. I think the original EST protocol in Section 4.1.1 allows a > client to request /cacerts without having yet authenticated the EST server. > > Second paragraph: “could be just the CA public key … or … without the > signature” -> in this case the Content-Format would be a different one, I > presume – a format that can encode this reduced information instead of the > regular Content-formats defined by EST-coaps. Now it reads as if the server > can just leave away information but that’s not correct I think. The client > might request a specific Content-Format for the /crts resource, one with all > bells and whistles included, and the server needs to oblige to this request. > > 4.3.2 CBOR-encoded Objects > > >> In the case of CBOR-encoded request … also CBOR encoded. > There’s also the client’s Accept option that comes into play. There could be > a future content-format that the client would specify in the Accept option; > in which case the response could be another format than 62. > But agree that when no Accept option is specified, then 62 is returned with > CBOR-encoded contents. > > 4.4 Message Bindings > > >> endpoints support delayed responses > Text here could refer to Section 4.7 of RFC 9148. > > Final bullet: “EST URLs based on https:// are” -> this seems incorrect, it > would probably need to be “EST-coaps URLs based on coaps:// are translated to > coap:// , but with … “ > > 4.6 Message fragmentation > (Use caps ‘F’ here) > > >> … to prevent IP fragmentation … > to prevent 6LoWPAN fragmentation > > Last paragraphs Block1/Block2 requirements: these are repeats of section 4.4 > requirements. Should try to avoid duplicate normative requirements. > If needed, refer back to the requirements made elsewhere. > > 4.8 Enrollment of Static DH Keys > > >> … how the EST client enrolls a static DH key > See earlier comment about enrolling keys vs enrolling devices or > certificates. Terminology is a bit unclear to me. > > Paragraph 2: there are 2 SHOULD requirements here – it seems it’s in fact > only 1 requirement? Reduce to one SHOULD, if possible. Also if the SHOULD is > not followed, what are the alternatives? > > >> In some cases, it may be beneficial … > Which cases? Maybe this is further detailed in the SP- references given later > on? > > 5. HTTP-CoAP Proxy > > >> … use of TLS and DTLS is optional … > Use of TLS or DTLS is optional > > It’s optional for the client to use: it determines which protocol is > initiated. Is it mandatory for the Proxy to support all of the options > NoSec, OSCORE, TLS and DTLS for the first leg from the client ? Or do we > assume “profiles” where the deployer of the Proxy knows what all the EST > clients will support/use? > In case of CoAP-over-TLS, what scheme would be used? (would we have to > mention in that case that discovery would yield e.g. “coap+tls” scheme in the > links? That seems to add more complexity / options to this specification. ) > > 6.1 Server-generated Private Keys > > >> … it has been shown that many available hardware modules … > Is there some reference for this claim? Maybe not needed in this doc if it’s > generally known. But I didn’t know it. > > 6.2 Considerations on Channel Binding > > >> … PKCS#10 requests OPTIONAL > Is this OPTIONAL for the client? Does the server need to support this channel > binding always? > > 7.1 EDHOC Exporter Label Registry > It’s not clear to me what this is and how/when TBD4 is used. It’s not > mentioned anywhere else in the draft. > > 8.2 Informative References > > It looks like some references need to be normative. There’s some IETF > guidance for this case that any optionally implemented function requires the > spec for that function to be Normatively referenced. Because the one > implementing this option needs to follow that specification and normatively > implement it. “Informative” is purely background info that an implementer > would not have to implement even with all options enabled. See > e.g.https://datatracker.ietf.org/doc/statement-iesg-iesg-statement-normative-and-informative-references-20060419/ > > [I-D.ietf-core-oscore-edhoc] > > [I-D.ietf-cose-cbor-encoded-cert] > -> normative, because using these CBOR encoded certs is an option in the > implementation. > > [RFC8446] > -> normative, because it looks like the client could use TLS, and for sure > the EST server could use TLS. > > [RFC7627] > -> Normative, in case using TLS is OPTIONAL for an implementation. (E.g. so > far in the draft it looks like an EST client could use TLS, or the leg > between Proxy and EST server could use TLS.) > > [RFC9147] > -> normative since DTLS can be optionally used as extra transport security? > > [RFC9360] > -> normative since it looks like a client or server can optionally use a > certificate by reference. Assuming we want x5u to be the default option for > this. > > Some references are not used it seems: > > [RFC2985] -> not used? > [RFC2986] -> not used? > [RFC5280] -> not used? > [RFC5869] -> not used? > [RFC5914] -> not used? > [RFC7627] -> not used? > > > Best regards, > Esko > > IoTconsultancy.nl <http://iotconsultancy.nl/> | Email/Teams: > esko.d...@iotconsultancy.nl <mailto:esko.d...@iotconsultancy.nl>
_______________________________________________ Ace mailing list -- ace@ietf.org To unsubscribe send an email to ace-le...@ietf.org