Re: [Acme] ACME draft is now in WGLC.

2017-02-12 Thread Anders Rundgren

On 2017-02-13 06:26, Martin Thomson wrote:


   In the examples below, JWS objects are shown in the JSON or flattened
   JSON serialization, with the protected header and payload expressed
   as base64url(content) instead of the actual base64-encoded value, so
   that the content is readable.  Some fields are omitted for brevity,
   marked with "...".

I didn't really understand this without an example to use for
reference.  Given that the first actual use of this form is 15 (!)
pages further down the document, maybe you could move this there.


JWS is great for what is was originally designed for.  ES6 normalization
nullifies the need for dressing JSON data in Base64Url.

Anders
https://cyberphone.github.io/doc/security/jsonsignatures.html

___
Acme mailing list
Acme@ietf.org
https://www.ietf.org/mailman/listinfo/acme


Re: [Acme] ACME draft is now in WGLC.

2017-02-12 Thread Martin Thomson
On 11 February 2017 at 05:56, Salz, Rich  wrote:
> ACME WGLC

It's been a while since I did a review of this, so I spent a few hours on it.

This document is in pretty good shape.  I have a lot of comments (a
LOT), and some of these are serious enough that I'd want to see
another WGLC, but they are relatively minor.  Overall the document is
in very good shape; most of my comments are of the nitpicking variety.

This is a pretty significant piece of work and that this is as good as
it is represents cause for congratulations to those involved (except
me, I'm just a hazard).


Meta

I found the use of lowercase versions of RFC 2119 keywords quite
distracting, and occasionally confusing.  It's often the case that the
lowercase version is used where a simple imperative statement would
work.  I haven't called out many of those in my review, except where
they caused what I think to be a genuine problem.

I have created an editorial pull request on the spec.  There were lots
of tiny fixes (including some opportunities to correct the above).  In
some cases, that PR will conflict with substantial changes that I
think need to happen, so you might want to attend to that first.

   https://github.com/ietf-wg-acme/acme/pull/243


S5.1

   Clients SHOULD
   support HTTP public key pinning [RFC7469], and servers SHOULD emit
   pinning headers.

and

   ACME clients SHOULD send a User-Agent header in accordance with
   [RFC7231], including the name and version of the ACME software in
   addition to the name and version of the underlying HTTP client
   software.

and

   Such servers SHOULD
   set the Access-Control-Allow-Origin header field to the value "*".

Why?  It's usually good practice with a SHOULD to provide some sort of
reason why choosing not to comply might be necessary, or to otherwise
motivate the choice.  All three seem relatively easy to fix with a
simple sentence (pinning good m'kay, client bugs(?), building a
web-based client).

S5.2

   For all other requests, there MUST be a "kid" field.  This field must
   contain the account URI received by POSTing to the new-reg resource.

MUST?

   Servers MUST NOT respond to GET
   requests for resources that might be considered sensitive.

Since this is a requirement, it might pay to do a little due diligence
on what might be sensitive.  I'm not sure that all implementers will
have the same view (or same motivations) when it comes to this.

  server MUST return an error

This would benefit from a reference to S5.7, as does all the other
instances of error handling.  Though this section includes an example
(that seems unnecessary), it's very hard to tell if this is normative
or not from context.

   In the examples below, JWS objects are shown in the JSON or flattened
   JSON serialization, with the protected header and payload expressed
   as base64url(content) instead of the actual base64-encoded value, so
   that the content is readable.  Some fields are omitted for brevity,
   marked with "...".

I didn't really understand this without an example to use for
reference.  Given that the first actual use of this form is 15 (!)
pages further down the document, maybe you could move this there.

S5.3

   JWK thumbprint [citation needed]

S5.4

I would reference RFC 7230, Section 2.7.3 "http and https URI
Normalization and Comparison" here and note the risk that
normalization could cause comparison failures.  A recommendation that
URLs generated by the server SHOULD be normalized according to that
section so that the risk of normalization is reduced.

S5.5

   The server MUST include a Replay-
   Nonce header field in every successful response to a POST request,
   and SHOULD provide it in error responses as well.

This seems to purposefully neglect to mention other requests, like
GET.  Does the "SHOULD" apply just to POST requests, or is it any
method?

This section says that nonces are mandatory with POST requests, but
does not offer a way to get a nonce that does not require a nonce.
Please make this observation and reference S6.2.

S5.6

Once again, mention is made of error descriptions without a forward reference.

   Additionally, the server
   SHOULD send a "Retry-After" header indicating when the current
   request may succeed again.

"may" or "could"?

S5.7

In a couple of places, a specific error URN is mandated (with MUST),
but this section only uses SHOULD for the error description.

   Authorization and challenge objects can also contain error
   information to indicate why the server was unable to validate
   authorization.

Where and how would this appear?  This needs more definition (I
looked, but I didn't find anything further down).

S6.1

The list of resources could use some references to the sections that
define them.

S6.1.2

   status (required, string):  The status of this account.  Possible
  values are: "valid", "deactivated", and "revoked".  The value
  "deactivated" should be used to indicate user initiated