Re: [websec] AD review of draft-ietf-websec-key-pinning-17

[Barry Leiba <[e-mail]>]

Yoav, thanks for the comments.  I hope the authors will also comment
tout de suite, so we can move this document along.

> Authors: you seem to have your work cut out for you.   And to the rest of
> the working group: there is a good reason why Barry has sent this to the
> list. Dealing with the AD review is a task for the working group, not only
> the authors and chairs. So everyone, please feel free to jump in.

Indeed: please do jump in.

> The below responses are my own and have no hats.  I’m skipping the language
> ones, because me not American, me no talk English good.

.גם לי בעברית

>> Is "we believe that" in the middle of the second paragraph something
>> that should stand as it is in the final RFC?
>
>    We believe that, with care, host operators can greatly
>    reduce the risk of main-in-the-middle (MITM) attacks and other false-
>    authentication problems for their users without incurring undue risk.
>
> I think this should stay.

OK, I'll let that drop, then.  I do wish we could find a better way to
say it than "we believe", but if no other ADs bring that up I'll say
no more about it.

>> I don't think it's clear enough that this document doesn't define that
>> registry, and that this is giving instruction for future registry
>> creation.  Please talk with me about why you feel the need *now* to
>> say that a future registry will have to use IETF Review as its policy.
>> Why would Specification Required or Expert Review not be OK?  Why
>> shouldn't the decision be left for the time the registry is created?
>
> I agree with that. How about:
>
> NEW:
>    Additional directives extending the semantic functionality of the
>    header fields can be defined in other specifications. The first
>    such specification will need to define a registry for such
>    directives.

That sounds fine.

>>    In the pin-directive, the token is the name of a cryptographic hash
>>    algorithm, and MUST be "sha256".  (In the future, additional hash
>>    algorithms MAY be registered and used.)  The quoted-string is a
>>    sequence of base 64 digits: the base 64-encoded SPKI Fingerprint
>>    ([RFC4648]).  See Section 2.4.
>>
>> I don't like the MUST there (nor the MAY).  As to "registered and
>> used", is this intended to be in the same registry as above?  So the
>> directive "pin-sha256" would be registered, along with a new
>> "pin-shaXYZ"?
>>
>> Assuming that's right, I'm really starting to think that the registry
>> should be created now.  The reason to defer creation is if we think
>> that no new directives will ever really come along, so we won't need
>> the registry.  But if the hash algorithm names will be in the
>> registry, we can be pretty sure we'll need new ones.
>
> This I will push back on. The day SHA-256 becomes not good enough for this
> purpose is when there is a second pre-image attack against SHA-256. We don’t
> have that yet even for MD5. I don’t think we’re going to *need* a pin-SHA512
> or a pin-SHA3-256.  Somebody might *want* a pin-GOST-R34.11-94, but in that
> case I’m happy to leave the work of setting up a registry to them.

Again, I'll let this go... but I'm not terribly happy about it.  It's
not a question of whether or when we'll get *rid* of 256, but whether
and when we'll add a new one as an alternative.  As soon as we do,
whether it be SHA-512, some SHA3 variant, or some GOST thing, it'll be
up to someone else to create the registry.  That someone might not be
in as good a position to do it as we are now.

But if you're really confident that we're likely never to actually
need the registry, I'll drop this one as well.

>> NEW
>>    When a connection passes Pin Validation using the UA's noted pins
>>    for the Host at the time, the Host becomes a Known Pinned Host.
>>
>>    According to rule 5, above, the UA MUST ignore pin-directives with
>>    tokens naming hash algorithms it does not recognize.  If the set of
>>    remaining effective pin-directives is empty, and if the Host is a
>>    Known Pinned Host, the UA MUST cease to consider the Host as a Known
>>    Pinned Host (the UA should fail open).  The UA SHOULD indicate to
>>    users that the Host is no longer a Known Pinned Host.
>> END
>>
>> On the last sentence, though, I'm really unsure: are we really giving
>> UI advice in a protocol document?  Is that a good idea?  Do w have any
>> sense that my mother will have the first idea what you're talking
>> about when you indicate this to her?
>
> We do often write sentences like “This event should be logged.” I don’t
> think either your mother or mine regularly run Console.app or eventvwr.exe
> to check logs. I guess this is the equivalent statement for user agents. It
> makes sense that if there is some indication that a website is pinned, that
> indication SHOULD be gone when the website is unpinned. It’s not for us to
> say how a particular vendor will indicate pinning in their UI, but I think
> it’s reasonable to say that the pinning indication should be gone.

As straight advice, I think "You ought to log this," is vastly
different to "You ought to tell this to the end user."  The former is
something we know a lot about: what information is critical for
problem determination.  We know little to nothing about what's helpful
or not in user interfaces.  (I also don't think our advice on logging
should be using 2119 language, as it's not a protocol interoperability
issue.)

You say that these indicators make sense; I contend that they don't.
First, users have no idea what pinning means.  Second, from the user's
point of view, if you've confirmed the identity of the server, the
user doesn't care whether you did that with key pinning or not,
whether you did it with DANE or not, whether you did it by checking
the CAs or not, or whatever.  My sense (as a non-UI-designer) is that
if you're going to tell the user anything, it's purely that the site's
identity has been confirmed (or not).  Nothing about the mechanism
matters.

I don't see why there's any value in indicating pinning in a UI.

>> -- Section 4.3 --
>>
>>    Because having a backup key pair is so important to recovery, UAs
>>    MUST require that hosts set a Backup Pin. (See Section 2.5.)
>>
>> Unless I misunderstand, this requirement means that if my server
>> doesn't send a backup pin, my server doesn't benefit from key pinning
>> (it's as though I never included the header in the first place).  If
>> that's not true, then Section 2.5 needs to be fixed to make it clear
>> that failure to include a backup pin constitutes a validation failure.
>>
>> If that is true, then you're trading off recovery for security, and I
>> expect the Security ADs to question that.  To head that off, it might
>> be good to acknowledge that here, and perhaps to say a few more words
>> about it.
>
> How about:
> The down side of keeping a not-yet-deployed key pair
> is that if an attacker gains control of the private
> key she will be able to perform a MitM attack without being
> discovered. Care must be taken to avoid leaking the key, such
> as keeping it offline.

That's fine to add, but it doesn't get to the point of my question.
The point wasn't about securing the backup key.

The point is this:

Suppose I give you a pin, but not a backup pin.  There are two things
you can do:

1. Accept the pin, and proceed along happily unless and until I do
something that makes that pin invalid.  At that point, lacking a
backup, you start denying access to my server.

2. Do not accept the pin because there's no backup, and proceed as
though I hadn't sent the pin.

The spec has chosen path 2.  I'm questioning whether that's really the
right path.  It seems to me that (2) is less secure (because I don't
get pinning), and you're choosing it just in case (1) might happen.

Why wouldn't the better approach be to say that you SHOULD send a
backup pin, and warn what can happen if you don't.  And then if a
server doesn't, and becomes inaccessible for a time as a result, it's
their own fault for not paying attention to the advice?

Barry