Re: [Ace] AD review of draft-ietf-ace-coap-est-12

Benjamin Kaduk <kaduk@mit.edu> Sat, 07 September 2019 01:05 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: ace@ietfa.amsl.com
Delivered-To: ace@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 8C74A120E16; Fri, 6 Sep 2019 18:05:16 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.2
X-Spam-Level:
X-Spam-Status: No, score=-4.2 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 527J40xYvXUQ; Fri, 6 Sep 2019 18:05:13 -0700 (PDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 929EB120B38; Fri, 6 Sep 2019 18:05:13 -0700 (PDT)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id x87155jQ009903 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 6 Sep 2019 21:05:07 -0400
Date: Fri, 6 Sep 2019 20:05:04 -0500
From: Benjamin Kaduk <kaduk@mit.edu>
To: consultancy@vanderstok.org
Cc: Jim Schaad <ietf@augustcellars.com>, draft-ietf-ace-coap-est.all@ietf.org, ace@ietf.org
Message-ID: <20190907010504.GR78802@kduck.mit.edu>
References: <20190828233639.GI84368@kduck.mit.edu> <027701d55ebf$994184b0$cbc48e10$@augustcellars.com> <edcbc2a243cc7118e35aec77b2e1599c@bbhmail.nl> <20190901204340.GG27269@kduck.mit.edu> <6b482aaed0ce510c503984dfbac7286c@bbhmail.nl>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <6b482aaed0ce510c503984dfbac7286c@bbhmail.nl>
User-Agent: Mutt/1.10.1 (2018-07-13)
Archived-At: <https://mailarchive.ietf.org/arch/msg/ace/NngD4qcFspv1c9Ja23bYE0u4S9o>
Subject: Re: [Ace] AD review of draft-ietf-ace-coap-est-12
X-BeenThere: ace@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Authentication and Authorization for Constrained Environments \(ace\)" <ace.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ace>, <mailto:ace-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ace/>
List-Post: <mailto:ace@ietf.org>
List-Help: <mailto:ace-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ace>, <mailto:ace-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 07 Sep 2019 01:05:17 -0000

On Mon, Sep 02, 2019 at 02:47:10PM +0200, Peter van der Stok wrote:
> Hi Ben,
> 
> Below some additional reactions to your review.
> In some parts the term "suggest" is used, meaning that I am not sure of
> the correctness of my reaction.
> A confirmation/denial would be appreciated in those cases.

Okay, I'll try to remark at those locations, and sorry for not trimming
stuff without comments.

> My co-authors can always improve my changes.
> 
> To-morrow I hope to do the follow-up.
> 
> many thanks,
> 
> peter
> 
> ________________________________________________________________
> 
> Section 4
> 
>    As per sections 3.3 and 4.4 of [RFC7925], the mandatory cipher suite
> 
> nit: I do see RFC 7925 in the subject heading, but the lead-in here is
> still a bit jarring.  Without some statement in this document to that
> effect, RFC 7925 is not binding on the protocol specified in this
> document, so I think it's better to say something like "In accordance
> with", or even to flat out state that "this document conforms to the
> DTLS 1.2 profile specified in RFC 7925". 
> 
> <pvds> done, used: In accordance with sections …  </pvds>
> 
>    DTLS 1.2 implementations must use the Supported Elliptic Curves and
>    Supported Point Formats Extensions in [RFC8422].  Uncompressed point
>    format must also be supported.  DTLS 1.3 [I-D.ietf-tls-dtls13]
>    implementations differ from DTLS 1.2 because they do not support
>    point format negotiation in favor of a single point format for each
>    curve.  Thus, support for DTLS 1.3 does not mandate point format
>    extensions and negotiation.
> 
> DTLS 1.3 uses the "supported_groups" extension in contrast to Supported
> Elliptic Curves for 1.2; we should mention that disparity as well. 
> 
> <pvds> added at the end of the paragraph </pvds> 
> 
>    o  a previously issued client certificate (e.g., an existing
>       certificate issued by the EST CA); this could be a common case for
>       simple re-enrollment of clients.
> 
> Is "re-enrollment" intended to cover renewal operations? 
> 
> <pvds>RFC7030 states: "An EST client can renew/rekey its existing client
> certificate by 
> 
> submitting a re-enrollment request to an EST server."
> 
> Do you want that emphasized more in the draft?

I don't think it's necessary; thanks for finding the right 7030 quote to
quiet me up.

> </pvds>
> 
>    o  a previously installed certificate (e.g., manufacturer IDevID
>       [ieee802.1ar] or a certificate issued by some other party); the
>       server is expected to trust that certificate.  IDevID's are
> 
> "trust" can cover a lot of things, many of which we don't really need
> here; would "expected to be able to validate" suffice? 
> 
> <pvds> s/is expected to trust that certificate/ can use the certificate
> for DTLS client Authentication/
> </pvds>
> 
>    As described in Section 2.1 of [RFC5272] proof-of-identity refers to
>    a value that can be used to prove that the private key corresponding
>    to the public key is in the possession of and can be used by an end-
>    entity or client.  Additionally, channel-binding information can link
> 
> nit: "the certified public key", I think, since the certificate is what
> binds the identity to the public key.
> Also, this sentence is a bit awkward, though I don't have any concrete
> rewording suggestions at present. 
> 
> <pvds>s/public key/certified public key/ 
> 
> Suggestion, is this better?: "a client provides proof of identity by
> proving that it possesses the private key corresponding with the
> certified public key" 
> 
>    Given that after a successful enrollment, it is more likely that a
>    new EST transaction will take place after a significant amount of
>    time, the DTLS connections SHOULD only be kept alive for EST messages
>    that are relatively close to each other.  In some cases, like NAT
>    rebinding, keeping the state of a connection is not possible when
>    devices sleep for extended periods of time.  In such occasions,
>    [I-D.ietf-tls-dtls-connection-id] negotiates a connection ID that can
>    eliminate the need for new handshake and its additional cost.
> 
> Do we also want to mention DTLS 1.3 session resumption here as less
> expensive than a full handshake?  It's not as cheap as "just keep using
> the same connection ID", of course, but has somewhat different other
> properties. 
> 
> <pvds> thanks, added: 
> 
> "additional cost; or DTLS 1.3 session resumption provides a less costly
> alternative than re-doing a full DTLS handshake."
> </pvds>
> 
> Section 5
> 
>    o  Simple enroll and re-enroll for a CA to sign public client
>       identity key.
> 
> nit(?): is this "public client identity key" or "client identity public
> key" or something else? 
> 
> <pvds>by reducing the txt's target, I suggest: 
> 
> s/public client identity key/client certificate/ 

That seems to work.

> </pvds>
> 
>    o  Certificate Signing Request (CSR) attribute messages that inform
>       the client of the fields to include in a CSR.
> 
> nit: "informs" 
> 
> <pvds> yep, thanks </pvds>
> 
>    o  Server-side key generation messages to provide a private client
>       identity key when the client choses so.
> 
> (similar nit(?) as above) 
> 
> <pvds>also reducing the text's target: 
> 
> s/provide a private client identity key/provide the client with a new
> private key/ 

Thanks

> </pvds>
> 
> Section 5.1
> 
> With the current text, I expect us to get several IESG questions about
> "why do you have both discovery and well-known URIs?".  I think we need
> to treat this more prominently in the first few paragraphs of the
> section, perhaps just after we discuss the short-est strings, so we
> could tie into the constrained nature of things and how some devices may
> need to hardcode assumptions about the endpoint location. 
> 
> <pvds> I suggest the following starting text: 
> 
> "EST-coaps is targeted for low-resource networks with small packets. Two
> types of installations are possible (1)rigid ones where the address and
> the supported functions of the EST server(s) are known, and (2) flexible
> one where the EST server and it supported functions need to be
> discovered. 
> 
> For both types of installations, saving header space….."

That looks great; thank you!

> </pvds>
> 
>    Figure 5 in Section 3.2.2 of [RFC7030] enumerates the operations and
>    corresponding paths which are supported by EST.  Table 1 provides the
>    mapping from the EST URI path to the shorter EST-coaps URI path.
> 
> Our table has the entries in a different order than 7030's table. 
> 
> <pvds> correct, has been modified </pvds> 

(It may be ever so slightly easier for readers that have both documents
open, if we keep the ordering the same for the entries that are common to
both documents.)

> We also don't say anything about the (lack of the) fullcmc endpoint. 
> 
> <pvds>/fullcmc is added in the table with "n.a." as short value </pvds> 
> 
> The serverkeygen endpoints could perhaps have some notation to indicate
> that the private key is always returned, in addition to the PKCS#7 vs.
> pkix-cert question that distinguishes skg and skc. 
> 
> <pvds>after "…..an /skc request.", the following text is added: 
> 
> "In both cases a private key MUST be returned." 

I was thinking we could have something in the table, though we may be too
short on horizontal space for that to actually work.

> </pvds>
> 
>    The /skg message is the EST /serverkeygen equivalent where the client
>    requests for a certificate in PKCS#7 format and a private key.  If
> 
> nit: s/requests for a/requests a/ 
> 
> <pvds>done</pvds>
> 
>    Clients and servers MUST support the short resource EST-coaps URIs.
> 
> Are they expected to also support the long EST URIs over CoAP? 
> 
> <pvds> It was undecided whether we should add: 
> 
> "The corresponding longer URIs from <xref target="RFC7030"/> MAY be
> supported." 

I have no strong preference here, but would not be surprised if we got a
question on it from the IESG.  (Which I guess means a weak preference for
adding the above text.)

> </pvds> 
> 
>   The first three lines of the discovery response above MUST be
>    returned if the server supports resource discovery.  The last three
> 
> It may be worth listing out ace.est.crts, ace.est.sen, and ace.est.sren
> explicitly for clarity (especially since line breaks are "only for
> readability") 
> 
> <pvds> after "the first three lines" added 
> 
> ",describing ace.est.crts, ace.est.sen, and ace.est.sren,"
> </pvds>
> 
>    lines are only included if the corresponding EST functions are
>    implemented.  The Content-Formats in the response allow the client to
>    request one that is supported by the server.  These are the values
>    that would be sent in the client request with an Accept option.
> 
> It seems that these specific values (or a subset thereof) are mandatory;
> a forward reference might be in order. 
> 
> <pvds> "Added : "(see table 2)"

Hmm, I think I was not very clear about what I meant: for the given
resources, the only defined content types for that resource are the ones
listed in the example request.  If a server tries to advertise anything
else, it seems like that would be out of spec (and maybe a client should
bail on seeing it?).  But maybe I'm misinterpreting things and we should
leave open space for future expansions to other content formats.

> Section 5.2
> 
> Did we consider merging this table with Table 1? 
> 
> <pvds> 
> 
> The functions of the tables are different. It fits the progress of the
> text. Merging will probably mean more forward references. Unless two
> tables are too boring, I suggest to leave as is. 

Okay.  Thank you for the explanation!

> </pvds>
> 
> Section 5.2
> 
>    While [RFC7030] permits a number of these functions to be used
>    without authentication, this specification requires that the client
>    MUST be authenticated for all functions.
> 
> Perhaps this divergence from 7030 should be noted more prominently,
> perhaps in the section title or a dedicated "Differences from RFC 7030"
> section? 
> 
> <pvds> 
> 
> Suggestion to move this phrase to section 5 just before section 5.1? 

That would be a big help, thanks..

> </pvds>
> 
> Section 5.3
> 
>    EST-coaps is designed for low-resource devices and hence does not
>    need to send Base64-encoded data.  Simple binary is more efficient
>    (30% smaller payload) and well supported by CoAP.  Thus, the payload
>    for a given Media-Type follows the ASN.1 structure of the Media-Type
>    and is transported in binary format.
> 
> This last sentence is only true when scoped to this document, for which
> all the content we're handling is specified using ASN.1.  I don't know
> whether we want to tweak the wording to reflect that or not, though.
> Also, we probably should say DER-encoded ASN.1 structure (or BER?  I'd
> have to check what the requirements are) since ASN.1 is just the
> abstract syntax and not the encoding rules. 
> 
> <pvds> 
> 
> s/payload/payload for DER-encoded ASN.1/
> </pvds>
> 
>    When the client makes an /skc request the certificate returned with
>    the private key is a single X.509 certificate (not a PKCS#7
>    container) with Content-Format identifier TBD287 (0x011F) instead of
>    281.  In cases where the private key is encrypted with CMS (as
>    explained in Section 5.8) the Content-Format identifier is 280
>    (0x0118) instead of 284.  The key and certificate representations are
>    ASN.1 encoded in binary format.  An example is shown in Appendix A.3.
> 
> I think these relationships might be more clear in tabular form; I
> dind't really understand the scheme at this point in the document, and
> needed to get a ways further in before it really "clicked". 
> 
> <pvds> the following table is added. Is that sufficient?
> 
>              +----------+-----------------+-----------------+ 
> 
>              | Function | Response part 1 | Response part 2 | 
> 
>              +----------+-----------------+-----------------+ 
> 
>              | /skg       | 284                 | 281                | 
> 
>              | /skc       | 280                 | TBD287          | 
> 
>              +----------+-----------------+-----------------+ 
> 
>              Table 3: response content formats for skg and skc 

I think so.  Just splitting out and naming "part 1" and "part 2" for the
comparison helps the clarity.

> </pvds>
> 
> Section 5.4
> 
>    o  EST-coaps servers sometimes need to provide delayed responses
>       which are conveyed with an empty ACK or an ACK containing response
>       code 5.03 as explained in Section 5.7.  Thus, it is RECOMMENDED
> 
> nit: the response itself (delayed or not) is not in the ACK, so maybe
> "the need for which is conveyed". 
> 
> <pvds> 
> 
> s/responses which are conveyed with an/which are preceded by an
> immediately returned/ 

That should work (but please check that the final text still has "delayed
responses", the expression here would have dropped the "responses" part)

> Section 5.6
> 
>    layer.  In addition, invokers residing on a 6LoWPAN over IEEE
>    802.15.4 [ieee802.15.4] network should attempt to size CoAP messages
>    such that each DTLS record will fit within one or two IEEE 802.15.4
>    frames.
> 
> Is this intended to be a normative SHOULD?  If not, it feels like we
> need a reference or justification. 
> 
> <pvds> 
> 
> s/should attempt/are recommended/ 
> 
> </pvds>
> 
> Section 5.7
> 
>    certificate to the client after a short delay.  If the certificate
>    response is large, the server will need more than one Block2 blocks
>    to transfer it.
> 
> nit: "Block2 block" singular 
> 
> <pvds> thanks, done </pvds>
> 
> POST [2001:db8::2:1]:61616/est/sen (CON)(1:0/1/256) {CSR (frag# 1)} -->
>    <-- (ACK) (1:0/1/256) (2.31 Continue)
> 
> Where is this notation documented?  (Is Appendix B.1 of this document
> the
> first place it's introduced?)  We need some kind of reference on first
> usage. 
> 
> <pvds> Just before figure 2 the following phrase is added: 
> 
> "The notation of figure 2 is explained in section B.1" 
> 
> </pvds> 
> 
>    If the server is very slow (i.e. minutes) in providing the response
>    (i.e. when a manual intervention is needed), he SHOULD respond with
> 
> nit: RFC style puts commas after (and before) "i.e." and "e.g.". 
> 
> <pvds>done everywhere </pvds>
> 
>    the identical CSR to the server.  As long as the server responds with
>    response code 5.03 (Service Unavailable) with a Max-Age Option, the
>    client SHOULD keep resending the enrollment request until the server
>    responds with the certificate or the client abandons for other
>    reasons.
> 
> nit: the transitive verb "abandons" has no direct object 
> 
> <pvds>abandons the request </pvds>
> 
>    response.  Note that the server asks for a decrease in the block size
>    when acknowledging the first Block2.
> 
> nit: From the trace, it looks like the server is the first one to use
> a 128-byte block size, and it does happen in an ack message, but that
> ack is not acking a block2 (though it contains one).  (That ack message
> also happens to contain part of the response.) 
> 
> <pvds> Ai, that confuses a lot. 
> 
> Should read: Note that the client asks again for a decrease
> </pvds>
> 
> POST [2001:db8::2:1]:61616/est/sen(CON)(1:N1/0/256){CSR (frag# 1)}-->
>   <-- (ACK) (1:0/1/256) (2.31 Continue)
> POST [2001:db8::2:1]:61616/est/sen (CON)(1:1/1/256) {CSR (frag# 2)}  -->
>   <-- (ACK) (1:1/1/256) (2.31 Continue)
> 
> The first line doesn't seem to match up -- doesn't the "N1" mean
> "fragment N1+1" but the descriptive text at the end say "frag# 1"?
> Or is this supposed to be 1:0/1/256? 
> 
> <pvds> Difficult to be terse and get it right: 
> 
> CSR (frag# n) means fragment n of the request 
> 
> Cert resp(frag# m) means fragment m of the response 
> 
> Like in figure 2, in figure 3, the line 
> 
> "... Immediate response when certificate is ready ...", 
> 
> Is added to separate the request sending from the response reception 

Thanks for adding that line, but I think you were looking at one chunk
later than I was looking at.  I was looking at:

(1:N1/0/256){CSR (frag# 1)}

and seeing that the "N1" in "N1/0/256" should correspond to "fragment
N1+1", but the parenthetical at the end says "frag# 1".

Thanks,

Ben

> </pvds>
> 
> Also, it surprised me somewhat that the client has to re-send the whole
> request (i.e., all fragments thereof) after the Max-Age interval when
> the server says it's ready, since that feels wasteful of bandwidth, but
> I assume that's just how CoAP works and not relevant for this document. 
> 
> <pvds>We did not see any other mechanism; but we are talking about many
> minutes, the relative bandwidth loss is not too high.
> This way, there is no need to keep much state, especially when many
> simultaneous requests are treated.</pvds>
> 
> Section 5.8
> 
>    In scenarios where it is desirable that the server generates the
>    private key, server-side key generation should be used.  Such
> 
> nit: suggest s/should be used/is available/ to avoid the appearance of
> tautology. 
> 
> <pvds>done</pvds>