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

Reply via email to