Hi,

So I have some review comments based on my experience with writing a
client implementation from the current spec.  I was going to wait on
sending this until that was completed and I had working code to show,
but if folks are keen to move this toward WGLC soon (and I'd like that
too), then I figured I should probably fast track sending some of these
to the list asap now :)

I was glad to see that quite a few of the things I'd noted to mention
have already been addressed in the most recent update though.

Feel free to bust these up into separate threads on the list if any of
them need more than trivial further discussion.

So, in no particular order, other than what I had jotted them down in
my notes:


 - Account key roll-over is underspecified.

This section does now talk about what the server should return, but it's
still not clear on which JWS profile should be used for the newKey object.

Earlier in the document it says "JWS objects sent in ACME requests MUST
... include the nonce field" - and the example request shows both newKey
and the containing object as "signed as JWS" - but my understanding is
that only the outer JWS actually requires the nonce here.

I don't recall exactly where I drew that inference from now, it was a
while ago, but either way the correct behaviour should be noted there.



 - Account recovery.

I was going to note that if you'd lost the account key, chances are good
that you'd have lost the 'base' registration URI too ...

But if that section is really gone for good now, we can leave that until
it's revisited, if it ever is.  (it bounced in and out of git several
times in the last few minutes, so I wasn't sure if it was going to stay
gone yet :)  I agree with removing it entirely for now.



 - "status" fields for Authorisation objects and Identifier Validation.

The possible values are defined, but the precise meaning of those values
and the action that a client should take upon receiving them are not.

The best definitions I could find were in the source for boulder.

In particular, I see that "unknown" was removed from Identifier Validation
in a recent edit, but it remains for Authorisation Objects.

I was originally going to suggest it should be removed from both, since
it really just seems to be an internal state of boulder which leaked out
into the spec.  I can't see any good reason for that state to be returned
to a client (and if there is one, it should probably get a better name :)

Now that "invalid" and "revoked" have been removed as challenge responses
the remaining definitions are a bit more self-evident, but things like
the distinction between "pending" and "processing" could probably use some
more detail for people who didn't look them up in boulder's source.



 - Typo fix.

"but not one labeled "http-04", since challenge in version -04 was
compatible with one in version -04"

I assume that was meant to be "compatible with the one in version -03".



 - JWK thumbprints.

We mandate the use of SHA-256 and only SHA-256.  Should the hash used be
specified in a field somewhere?

As much as I'd like to think SHA-256 will never be broken, there is the
whole attacks only get better thing and encouraging people to hardcode
this selection in their code doesn't seem optimal.

I don't object to saying "only SHA-256 is supported by this version of
the spec", but it does seem wise to communicate the hash that was used
explicitly here.



 - rel=next

This one is more of a question about what I'm missing here ...

What is the value of the rel=next links for new-reg and new-authz?

They aren't actually the "next" operation to be performed and they
should already be available from the directory ...

Am I missing some important detail there, or are they really superfluous
and something we can/should prune out?



 - Challenge responses.

The reply format examples are inconsistent between what is shown in
"Responding to Challenges" and what is shown for the explicit challenge
types.  If you go back and forth enough you can figure out what is meant
but we can probably be explicit there instead of leaving part of what is
actually needed as an exercise for the reader.

e.g.:

 POST /acme/authz/asdf/0 HTTP/1.1
 Host: example.com

 {
  "resource": "challenge",
  "type": "http-01",
  "keyAuthorization": "IlirfxKKXA...vb29HhjjLPSggwiE"
 }
 /* Signed as JWS */

  vs'

 {
  "keyAuthorization": "evaGxfADs...62jcerQ"
 }
 /* Signed as JWS */


Also it says:

 "The client needs to respond with information to complete the challenges.
 To do this, the client updates the authorization object received from the
 server by filling in any required information in the elements of the
 "challenges" dictionary."

Should that be "indicated in the elements of"?  Because you don't fill in
the challenge dictionary to do this ...



 - Identifier Authorisation.

I see that "The "expires" field MUST be absent." was removed a couple of
days ago in the 'Editorial pass' changes Richard pushed.

I was going to query that because boulder included one.  I think that
it's right to allow that - but it's unclear to me why it was previously
prohibited so explicitly though.  Is there some detail which needs a
mention here about that?


Likewise the spec says:

 "The server updates the authorization document by updating its
 representation of the challenge with the response fields provided by the
 client. The server MUST ignore any fields in the response object that
 are not specified as response fields for this type of challenge.
 The server provides a 200 (OK) response with the updated challenge
 object as its body."

But boulder currently returns 202 Accepted for this.

Is that a bug in boulder, or the spec?



 - Complexity of challenge permutations.

It's nice that the Authorization Object can offer almost arbitrary
permutations for what sets of challenges might satisfy the proof of
control for a particular identifier, but I wonder if we do need to
limit this a little more in a few practical ways.

In particular, I wonder if it is reasonable to say something like:

 The challenges array MUST NOT contain more than one challenge of the
 same "type".

Otherwise the complexity at clients can increase significantly.
Most sites will configure to support just particular challenge types,
and will only be interested in those.  Having to then still pick from
a multiple choice set based on "combinations" seems like added pain
for no real gain.

A human might look at that set and find it easy to just pick the best
one, but a fully automated client that has to pick between two "equal"
options leaves more room for chaos than seems ideal.

For example, such a client might be configured to support http-01 and
dns-01, with a preference for dns-01.  If it has a choice to do just
dns-01, it would pick that, if only has a choice for http-01, then it
is still a simple choice.  If its simplest offered option requires it
to do both, it can cope with that too.  Otherwise, it is simply unsat
and can fail out until human intervention teaches it more methods.

But if it's offered the choice between dns-01 with token 'A' and dns-01
with token 'B', things start to get messy for no real benefit to the
protocol or its security.



 - Deleting things.

I was glad to see options to revoke account keys and identifier
authorisations got added.  That was on my list of important things that
were missing :)

The other one that might need noting explicitly which I tested was
removal of contact details.  In theory the protocol already allows that
by sending an empty array, but in practice boulder currently ignores
requests which do that.

There probably aren't many good reasons to do that in real use (I poked
a lot of junk requests at the staging server to see what it, and my code
would do), but having the only way to do it be to lie and send a "fake"
contact address seems less than ideal.

I'm likewise curious about why boulder dropped tel: contact URIs.
I can understand why the LetsEncrypt folk don't care to support that as
a contact method - but removing it broke access to existing accounts that
had one set, and it does seem like a valid "if all else fails" method for
out of band verification.

The commit log that dropped it didn't say much, but if there was some
good reason for that, maybe some guidance here is appropriate too?



 - More strictly specify valid identifier strings.

It would be nice to have some constraints on the allowable characters in
protocol identifier names, i.e. for things that are left open to later
extension like the challenge type names.

Applications may need to pass those around, sometimes in delimited lists
as strings etc., so it would be nice to have some guidance on what may
be an "illegal but unknown" identifier, as opposed to "clearly garbage".

Having to sanity check and untaint them in every place to a strict list
of precisely known identifiers is probably more of a maintenance burden
than being able to say "this must be alphanum, hypen or underscore" in
the places that don't care what they actually mean but must pass them
on to something which does - possibly as shell environment variables
or command line parameters etc.



  - Certificate renewal.

The spec says:

 "From the client's perspective, there is no difference between a
 certificate URI that allows renewal and one that does not. If the
 client wishes to obtain a renewed certificate, and a GET request
 to the certificate URI does not yield one, then the client may
 initiate a new-certificate transaction to request one."

This probably should be indicated somehow, because there *is* a real
difference here.  Namely what operation needs to be scripted for an
automated renewal.

This is another case where "a human can probably figure it out"
pretty quickly, but an unattended renewal process shouldn't need an
AI oracle to know what the renewal process should be.  It should be
told that explicitly in a protocol reply from the server in some way.



 - Kudos to everyone who has worked on this!

That's about all that's on my current list, though I'm not quite done
yet with implementing everything I want from a client, and I've still
got some reading to do to come up to speed with all the changes to this
document that landed in the last few days (mostly I've just scanned it
for changes that were already on my "report them here" list).

I was really happy to see that almost all the more major things I had
to raise have already been addressed in the latest document update
(even though I hadn't seen any discussion about them here previously :)

There might be couple more I've forgotten here, or that I might run into
before I'm all done on my side, but it's looking pretty close to ticking
off all my boxes now.

Well done, and many thanks. :)

  Ron


_______________________________________________
Acme mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/acme

Reply via email to