Subject: Re: [Acme] Specify account by kid (reg URL) rather than key. #193

From: Richard Barnes <[email-masked]>
Date: Sun, 2 Oct 2016 11:40:05 -0400

I can live with this, but I'm not enthusiastic.

tl;dr: Makes things more complicated, security benefit debatable.

It makes me sad to lose the clean layering between cryptographic
verification and verification of identity.  Before, the need to provide a
valid signature provided some minimal validation of the request that could
be performed totally statelessly by the server.  Now, the request
validation logic needs access to the account DB.  I guess whether you see
this as a loss depends on whether you regard signature verification or DB
access as more expensive; I am far more wary of DB access, and state
coupling in general.

I also don't think the attribution gains here are quite as big as are being
suggested.  For all requests where the account tied to the key matters, the
account is identified by the key.  So the error case here isn't like an
HTTPS client that doesn't check the server's cert and proceeds with the
connection anyway; an ACME server that doesn't look up the account for a
key is unable to proceed.

I'll grant that the "kid" approach is somewhat harder to attack than the
"jwk" approach.  In order to get the server to recognize a bad key pair,
the attacker needs to get a confusable account URI, and account URIs are
issued by the server.  So you basically need two bugs instead of one (URI
allocation + comparison, vs. key comparison).  Nonetheless, key comparison
does not seem that risky to me -- it's what undergirds every TLS and SSH
session you've ever engaged in, after all, and I don't recall ever hearing
about a major vuln in an implementation of these protocols due to a key
comparison flaw.  I don't have the same confidence in URL allocation and
comparison routines, which will have much more variability.

On Tue, Sep 27, 2016 at 9:24 AM, Daniel McCarney <[email-masked]>
wrote:

> I'm also strongly in favour of this change. I think the minimal increase
> in client
> state/complexity is offset by the gain of knowing that the category of
> potential
> bugs that Jacob mentions are avoided.
>
>
> On Tue, Sep 27, 2016 at 1:01 AM, Martin Thomson <[email-masked]>
> wrote:
>
>> I am inclined to think that this is a good change, on the basis that
>> it means that the server is minting the identifiers that the client
>> uses.  I think that Jacob is probably understating the potential for
>> bugs here.  And key canonicalization is a bad smell.
>>
>> On 27 September 2016 at 14:51, Jacob Hoffman-Andrews <[email-masked]>
>> wrote:
>> > I understand the concern, but I think that clients already have to store
>> > a significant amount of state: the ACME directory URL, the private key,
>> > and the domain names, certificates, and private keys of existing
>> > certificates. I think that one more item, the account URL, is not a
>> > heavy burden, especially when weighed against a real flaw in the
>> > protocol. You could consider it akin to storing a username and password
>> > for a more traditional login.
>> >
>> > All that said, for clients that find it to be a big savings, there is
>> > always the method of finding the account URL by POSTing again to new-reg
>> > with the same key.
>> >
>> > On 09/24/2016 06:16 PM, Hugo Landau wrote:
>> >> I'm somewhat against this on the grounds that it introduces unnecessary
>> >> state into clients (the registration URI), increasing their complexity.
>> >>
>> >> _______________________________________________
>> >> Acme mailing list
>> >> [email-masked]
>> >> https://www.ietf.org/mailman/listinfo/acme
>> >>
>> >
>> > _______________________________________________
>> > Acme mailing list
>> > [email-masked]
>> > https://www.ietf.org/mailman/listinfo/acme
>>
>> _______________________________________________
>> Acme mailing list
>> [email-masked]
>> https://www.ietf.org/mailman/listinfo/acme
>>
>
>
> _______________________________________________
> Acme mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/acme
>
>

---

From: Eric Rescorla <[email-masked]>
Date: Sun, 2 Oct 2016 08:46:15 -0700

I am inclined to agree with Richard here.

In particular, I'm surprised to see Jacob's arguments that key comparison
is a threat because of hash collisions [0]. It seems like if we are
computing fingerprints using hash functions with collision attacks, we've
got bigger problems than this.

-Ekr

[0] Nit: it seems like most useful attacks would be because of pre-images.

On Sun, Oct 2, 2016 at 8:40 AM, Richard Barnes <[email-masked]> wrote:

> I also don't think the attribution gains here are quite as big as are
> being suggested.  For all requests where the account tied to the key
> matters, the account is identified by the key.  So the error case here
> isn't like an HTTPS client that doesn't check the server's cert and
> proceeds with the connection anyway; an ACME server that doesn't look up
> the account for a key is unable to proceed.
>
>> On Tue, Sep 27, 2016 at 1:01 AM, Martin Thomson <[email-masked]
>> > wrote:
>>

---

From: Hugo Landau <[email-masked]>
Date: Mon, 3 Oct 2016 01:34:49 +0100

>    comparison routines, which will have much more variability.
I've already come out against this, but I'd like to echo Barnes's point
about URL comparison being a bit messy. There are multiple possible URLs
which represent a resource - "https://example.com/acme/reg/123",
"https://example.com/acme/reg/123?",
"https://example.com/acme/reg/123#",
"https://example.COM./acme/reg/123", maybe even
"http://example.com/acme/reg/123". Specifying the "right" URL isn't as
simple as it seems. What if an implementation considers only the path
and ignores the hostname, etc. Clarifying this would probably be a
decent-sized paragraph to add to the spec.

Avoiding key comparison also seems like crypto-superstition more than a
problem demonstrated in practice. Can we point to any historical
vulnerabilities caused by an implementation error of this kind?

---

From: Jacob Hoffman-Andrews <[email-masked]>
Date: Tue, 25 Oct 2016 18:44:41 -0700

On 10/02/2016 08:40 AM, Richard Barnes wrote:
> the need to provide a valid signature provided some minimal validation
> of the request that could be performed totally statelessly by the server.
This would only filter out requests that are otherwise well-formed, but
have a bad signature, which are a vanishingly small fraction of
requests. I just sampled 100k POSTs from Let's Encrypt's logs, and
exactly 0 of them have that property. So I don't think this "stateless
validation" has an advantage.

> Nonetheless, key comparison does not seem that risky to me -- it's
> what undergirds every TLS and SSH session you've ever engaged in

The main concern is not key comparison, it's "verify, then lookup" vs
"lookup, then verify." Your SSH example actually supports my point: In
SSH, the client sends a user id to the server, which the server then
uses to look up the public keys with which to authenticate the user.

On 10/02/2016 05:34 PM, Hugo Landau wrote:

> Can we point to any historical vulnerabilities caused by an
implementation error of this kind?

The specific kind of bug that the "verify, then lookup" pattern makes
possible is one where the "lookup" phase looks up the wrong thing. That
type of bug happens a lot. For instance, one of these Rails snippets has
a security flaw that would allow misissuance, because of the "verify,
then lookup" pattern. Can you tell which one just by looking at them?

-----------------------------------------------------------
header_key = jws_body.header.jwk
if !jws_body.verify(header_key)
  return "Bad signature"
account = Account.find_by jwk: header_key
authorizations = Authorization.find_by account: account.id
if authorizations.any? {|a| a.fqdn == requested_fqdn }
  issue_cert()
-----------------------------------------------------------

-----------------------------------------------------------
header_key = jws_body.header.jwk
if !jws_body.verify(header_key)
  return "Bad signature"
account = Account.find_by! jwk: header_key
authorizations = Authorization.find_by! account: account.id
if authorizations.any? {|a| a.fqdn == requested_fqdn }
  issue_cert()
-----------------------------------------------------------