Subject: [http-auth] review of (most of) mutualauth

From: Stephen Farrell <[email-masked]>
Date: Sun, 01 Sep 2013 15:49:03 +0100

Hi,

I didn't manage to get through the whole draft yet, but
I'll finish my review on a later version (assuming that
one will pop out soonish).

Note that while I think some fairly major changes are
needed, and while I'm also quite skeptical of the value
of any new password-based schemes, I do think this work
should be published.

Cheers,
S

general:

(1) The IPR declaration on the earlier non-WG drafts
should be updated for this draft and/or the
"replaced-by" information needs to be updated. It is not
enough to include a section within the draft since that
may not be noticed at IETF LC time. And FWIW, I'd
personally guess that the existence of the IPR will be a
negative in terms of the probability that this gets
wider adoption.

(2) draft-oiwa-httpauth-mutual-algo needs to be at least
a normative ref and a wg draft as well.  That draft
should get as much review as this one, however that's
done, since the whole thing depends on the security of
that draft. Its not possible to evaluate the security of
this draft without reviewing that one. (Which I've not
yet done since I'm not sure of its current status.)

(3) The draft has far too much sales-pitch and that
needs to be fixed. I fully understand why that text is
there and including it was appropriate when this draft
was facing a lonely uphill battle to become an RFC. But
that's no longer the case - now we have a WG all such
text ought be removed I wouldn't be surprised if this
takes more than one iteration to fix as there is a lot
of sales-pitch text.  I note some but not all of these
in the detailed comments below.

(4) The claim that this "prevents" phishing is not
justifiable - one can still mount phishing attacks on
any password based scheme, regardless of the protocol
used to transmit the password-derived authenticator.
(Including "common" in "prevents common phishing
attacks" does not make this ok.)

(5) Following up from (4), it doesn't matter if the user
can detect the phish after the password has been
compromised - that's too late.

(6) I don't think "Mutual" is a good name for this HTTP
auth scheme - "mutual" describes a property of many
potential schemes and isn't really specific enough.
Note: This is a minor point, easily outweighed by any
substantive deployment and/or multiple implementations
(but not by one implementation).

(7) I don't get how the HTTP server maintains state
between requests 3 and 6 in Figure 1. How does that
work? It seems counter to normal HTTP to me at least,
but then I don't know much about HTTP.

(8) I would worry that attackers could exploit the
potential for playing about between messages 3 and 6 in
Figure 1. Do we have any papers or other reason to think
they cannot?

(9) 4.1: the choices for pwd-hash are outdated - do we
really want md5 and sha1 as choices in new protocols?
The comment applies even if these are not security
criticial - the only acceptable reason to have those IMO
would be to meet some legacy interop requirement which I
don't believe applies here.

(10) Defining an "authenticaion realm" concept that is
not the same as the standard web-origin seems like a
mistake. In particular, the wildcard concept, if it
differs from web-origin is a bad plan and if its the
same, then why not just use what exists already?

(11) 5.1 talks about "path" parameters but those were
not mentioned in the earlier text in section 5. I don't
know how to intepret that. (I also find 5.1 quite
unclear.)

(12) Section 7 should really use TLS channel binding and
not re-invent that. In any case, code implementing this
might not have visibility inside the TLS library and so
should not require such layering violations.

(13) 7: Why just the server cert and not the entire cert
path? What about TLS raw public keys? What about DANE?
editorial/nits:

nits/minor comments:

- abstract: getting the HTTP acronym correct here would
be good. (s/Transport/Transfer/) Could be this is due to
the same error having being made in the WG charter
earlier.

- abstract: "intended to replace" is not appropriate for
the abstract - its enough to provide motivation
elsewhere. (And the other experimental schemes also
presumably have this same goal - better not to start a
bidding war on which abstract has better sales-pitch:-)

- abstract: claiming to replace HTML forms is unwise IMO

- TOC: 44 pages of content is too long, this should only
require <20 pages really.

- intro: what is "true" mutual auth? I'd delete that
sentence.

- intro: "currently available" is no longer basic and
digest - this wg is developing a bunch

- intro: "people have begun to think" is not appropriate

- intro: "solves these problems" para - this is just
sales talk, suggest deleting all but last sentence of
this para

- intro: "much stronger" is not justifiable IMO, nor
would it be for e.g. EKE or SPEKE

- intro: "It is secure" - Blatant assertion is not good
enough.

- intro: users *cannot* distinguish between good and bad
web servers on the basis of the browser's UI - for any
good web site, a bad one, can act as a MITM and grab the
password. Once entered, the password is compromised.

- I didn't (yet) review the httpauth-extension draft,
but that also seems to have an inappropriate title. I'm
not sure that the claims in the bullet list just before
1.1 are justified, as I've not read that draft.

- 1.1.1 - title is wrong - "this" will be experimental
and won't  "supercede" anything. The entire section
should be deleted really.

- 1.1.2 - this section also seems misplaced at best, I'd
also delete it.

- 1.2, I'd say its unwise to define new 2119-like terms,
so I'd drop the definition and uses of encouraged and
advised.

- 2.1 - what is 401-INIT? and 401-STALE etc? Mixing 401
and 200 and other message names is just confusing and
doesn't help.

- 3.1.2 sounds wrong to me, but should be ok for an
experimental draft, be good to get someone who knows
more about i18n to read it through.

- 3.1.3, I didn't check the ABNF, someone probably
should

- 4.1: version parameter - not a great idea IMO but ok
for exp stuff

- 4.1: the forward references to later sections and to
draft-oiwa-httpauth-mutual-algo make this v. hard to
read.

- 4.1, algorithm: you can't have a MUST for draft-oiwa
*or* something else undefined.

- 4.1, the "reason" responses seem a bit verbose - why
do you need to provide so much detail here and why isn't
HTTP status sufficient?

- 4.2 (and elsewhere), multiple version fields seems
like a bad plan

- 4.3, saying things MUST be the same as when received
isn't clear enough - saying which bits of s/w MUST do
what would be better.

- 4.5, rekey-method: I have no idea what this is about
based on the current text.

- 5: "SHOULD NOT automatically reuse" seems odd - when
would it be ok to do that? If never, then s/SHOULD NOT/
MUST NOT/ would be right.

- 6: sessions being local to a "server" seems like an
odd thing to say - why can't servers load balance? If
this were to gain traction, that'd have to be changed.