Subject: AD review of draft-ietf-httpbis-alt-svc-10

From: Barry Leiba <[email-masked]>
Date: Thu, 31 Dec 2015 01:38:35 -0500

Some comments, none very serious.  Let's have a quick chat about the
non-editorial ones before I send out the last call request.  And thanks,
Mike Bishop, for the really good and helpful shepherd writeup.

-- Section 2.1 --

   Clients MUST NOT use alternative services with a host that is
   different than the origin's without strong server authentication;

Total nit: make it "different from", not "different than".

-- Section 2.4 --

   By their nature, alternative services are OPTIONAL: clients do not
   need to use them.  However, it is advantageous for clients to behave
   in a predictable way when they are used by servers (e.g., for load
   balancing).

The antecedent to the last "they" is unclear: it looks like it's "clients",
when it should be "alternative services".  Best to re-word, as "...in a
predictable way when alternative services are used by servers...."

   If a client becomes aware of multiple alternative services, it MAY
   choose the most suitable according to its own criteria (again,
   keeping security properties in mind).

I don't think this is a 2119 "MAY": what *else* can it do?  You have no
other guidance about which alternative alternative to pick, so....  I think
this should just say, "it chooses the most suitable...."

   Furthermore, if the connection to the alternative service fails or is
   unresponsive, the client MAY fall back to using the origin or another
   alternative service.  Note, however, that this could be the basis of
   a downgrade attack, thus losing any enhanced security properties of
   the alternative service.  If the connection to the alternative
   service does not negotiate the expected protocol (for example, ALPN
   fails to negotiate h2, or an Upgrade request to h2c is not accepted),
   the connection to the alternative service MUST be considered to have
   failed.

I don't understand how this stops a downgrade attack if the alternative
service has better security than the existing connection.  The attacker
prevents me from establishing the better security, so I consider the
alternative service to have failed and fall back to the existing
connection... and the attack has succeeded, blocking me from upgrading the
security.  No?

-- Section 3 --
Please consider using RFC 7405 for the ABNF for "clear".

   alt-authority = quoted-string ; containing [ uri-host ] ":" port

This seems poor.   Why not this?:

NEW
   alt-authority = DQUOTE [ uri-host ] ":" port DQUOTE
END

   The field value consists either of a list of values, each of which
   indicating one alternative service, or the keyword "clear".

Typo: "indicates"

-- Section 3.1 --
For the persist ABNF, why 1DIGIT, and not just DIGIT?  Or, for that matter,
why not simply "1" ?  Other specifications might then add other values
using << persist =/ "2" >>, for example.

-- Section 9.1 --
The third paragraph assumes that system ports are inherently more secure
than user ports, and, as discussion during the development of RFC 6335
raised, that hasn't been the case for some time.  Many systems no longer
make a distinction, and no longet require root authority to listen on
system ports.

-- Section 9.4 --

   Clients concerned by the additional fingerprinting can choose to
   ignore alternative service advertisements.

Is there really any chance of this being exposed to users as an option?
Maybe it'd be good to add to this something like, "In particular, clients
configured for anonymous usage SHOULD NOT use alternative services." ?

--
Barry

---

From: Julian Reschke <[email-masked]>
Date: Thu, 31 Dec 2015 15:29:32 +0100

Hi Barry,

thanks for the feedback; I'm tracking the changes in 
<https://github.com/httpwg/http-extensions/issues/130>.

> Mike Bishop, for the really good and helpful shepherd writeup.

Indeed.

> Total nit: make it "different from", not "different than".

<https://github.com/httpwg/http-extensions/commit/f4d972afc73257940390834cf566aac2a898a8ed>

> The antecedent to the last "they" is unclear: it looks like it's
> "clients", when it should be "alternative services".  Best to re-word,
> as "...in a predictable way when alternative services are used by
> servers...."

<https://github.com/httpwg/http-extensions/commit/ba8920eac4d40252fe43b0d4511c18ebb825c9fa>

> other guidance about which alternative alternative to pick, so....  I
> think this should just say, "it chooses the most suitable...."

Agreed. I haven't changed that yet as it affects normative language but 
I will unless somebody wants to defend it soonish.

> connection... and the attack has succeeded, blocking me from upgrading
> the security.  No?
>
> -- Section 3 --

That would replace

   clear         = %x63.6C.65.61.72; "clear", case-sensitive

with

   clear         = %s"clear"; case-sensitive

(and add a dependency to the ABNF extension).

I'm not super-excited about this notation, and it seems we would be the 
first ones to actually use it (implying lack of validation tools etc).

What do others think?

>     alt-authority = DQUOTE [ uri-host ] ":" port DQUOTE

In HTTP specs, we don't like to use DQUOTE unless the thing being quoted 
used quoted-string syntax.

>     indicating one alternative service, or the keyword "clear".

That wasn't a typo, but as you are at least the second one complaining I 
changed it in 
<https://github.com/httpwg/http-extensions/commit/cc7946abe12ce8c4cd3db1bd1f8a7c2e71eb9943>.

> -- Section 3.1 --
> For the persist ABNF, why 1DIGIT, and not just DIGIT?  Or, for that
> matter, why not simply "1" ?  Other specifications might then add other
> values using << persist =/ "2" >>, for example.

I believe the intent was that new values do not imply changing the 
parser (which would be implied by changing the ABNF), but simply would 
allow new values here.

> make a distinction, and no longet require root authority to listen on

I have no opinion on this. More feedback appreciated.

> Maybe it'd be good to add to this something like, "In particular,
> clients configured for anonymous usage SHOULD NOT use alternative
> services." ?

That's an interesting proposal. What do the client implementers think?

Best regards, Julian

---

From: Barry Leiba <[email-masked]>
Date: Thu, 31 Dec 2015 11:38:55 -0500

Hi, Julian, and thanks for the quick reply on a holiday extended weekend.

Only on the items for which I have further comment...

>> connection... and the attack has succeeded, blocking me from upgrading

I didn't see a response to this.  I expect we might see comments about
this from the Security Directorate or ADs.

> first ones to actually use it (implying lack of validation tools etc).

It's a small thing, and it's up to the working group, of course.  I
would prefer the change, because (1) I think it makes it more
readable, and (2) we have put 7405 on the Standards Track, so we
should use it.  It wouldn't be a bad thing for us to break ground on
it.

> used quoted-string syntax.

I don't understand that.  I particularly don't understand why we'd
prefer to specify the content of the string only in the comment, when
it's perfectly easy and clear to put it into the ABNF, and have it be
verifiable.

I'm generally not a fan of putting substantive ABNF information into
ABNF comments, and prefer to reserve that sort of thing for cases when
it's *not* reasonable to do otherwise.

> <https://github.com/httpwg/http-extensions/commit/cc7946abe12ce8c4cd3db1bd1f8a7c2e71eb9943>.

Thanks.  To clarify, it could go either of these ways, and they're equally good:

- "The field value consists either of a list of values, each of which
indicates one alternative service"

- "The field value consists either of a list of values, each
indicating one alternative service"

...but "each of which indicating" doesn't work.

> I believe the intent was that new values do not imply changing the parser
> (which would be implied by changing the ABNF), but simply would allow new
> values here.

Three questions here, really, bundled into one:

1. Why "1DIGIT", rather than "DIGIT"?  Purely editorial, of course...
what's the benefit of using the "1"?

2. Why does "persist" have to be digits at all?  I'm generally not a
fan of unnecessarily coding concepts into numbers, rather than using
short words.  If it's necessary (or useful), that's fine.  I don't see
why here.

3. Do you expect a lot of additional values?  Clearly not, if you're
limiting it to ten possible ones.  In that case, as above, I prefer
when the ABNF is specific about it.  Consider, for example, RFC 2045's
definition of Content-Transfer-Encoding (in Section 6.1):

     encoding := "Content-Transfer-Encoding" ":" mechanism

     mechanism := "7bit" / "8bit" / "binary" /
                  "quoted-printable" / "base64" /
                  ietf-token / x-token

Here, "mechanism" could have just been defined as << ietf-token /
x-token >>, but enumerating the existing values in the ABNF is useful.
And the fact is that "persist=2" is not currently valid.

There's also, I suppose, the question of whether you'd want
"persist=2" to be considered acceptable (and ignored), and "persist=a"
to cause the whole field to be rejected as invalid.  If so, maybe
this, to specify what's valid now and how to extend?:

   persist = "1" / persist-ext
   persist-ext = DIGIT ; extensions can be specified in future

> I have no opinion on this. More feedback appreciated.

There was, of course, nothing actionable in my comment, so let me add
something here: Because I think there's little value in telling
clients to be more strict about authentication when switching from a
system port to a user port, I think it could give a false sense of
security if implementors do that... particularly if they think they
can be lax about authentication on system ports, because only trusted
software can listen on system ports.  We should be careful about
giving that impression.

Yes, more feedback from others, please.

> That's an interesting proposal. What do the client implementers think?

And I'm leaving this in the message just to encourage further
discussion as people <strike>sober up</strike> return from their new
year's celebrations.

Barry

---

From: Julian Reschke <[email-masked]>
Date: Thu, 31 Dec 2015 18:00:30 +0100

> Hi, Julian, and thanks for the quick reply on a holiday extended weekend.

Dito :-)

> ...

I might invest a bit of time to teach bap (Bill's ABNF Parser) to accept 
this; and once I can validate it I could be convinced to actually use it.

> it's perfectly easy and clear to put it into the ABNF, and have it be

There's a two-step process here.

First you need to parse the field value according to HTTP's 
quoted-string rules. The *result* of that parsing needs to validate as

   [ uri-host ] ":" port

There's (IMHO) absolutely no point to force this into a single ABNF 
expression.

> it's *not* reasonable to do otherwise.

It's in the prose as well:

"The "alt-authority" component consists of an OPTIONAL uri-host ("host" 
in Section 3.2.2 of [RFC3986]), a colon (":"), and a port number."

> short words.  If it's necessary (or useful), that's fine.  I don't see

I'll pass this to those who suggested the syntax :-)

> x-token >>, but enumerating the existing values in the ABNF is useful.

I respectfully disagree. For me, (ab)using the ABNF to enumerate values 
(in particular when they are extensible) is an anti-pattern.

>     persist-ext = DIGIT ; extensions can be specified in future

That's a good question.

 > ...

Best regards, Julian

---

From: Barry Leiba <[email-masked]>
Date: Thu, 31 Dec 2015 12:40:45 -0500

> First you need to parse the field value according to HTTP's quoted-string
> rules. The *result* of that parsing needs to validate as
>
>   [ uri-host ] ":" port
>

OK: We'll go ahead and disagree on this, but I won't argue the point
further.  If anyone else has a comment, let's hear it.  Otherwise,
we'll go with status quo.

> I respectfully disagree. For me, (ab)using the ABNF to enumerate values (in
> particular when they are extensible) is an anti-pattern.

OK here too: as with above, status quo unless there are comments from others.

b

---

From: Mike Bishop <[email-masked]>
Date: Thu, 31 Dec 2015 17:45:15 +0000

As to the system versus user port, that's an artifact of one of those security wranglings I mentioned in the write-up.  I remember working on servers where two things were true:

  - I could host arbitrary content at host.example.com/~username
  - I could run a process to listen on another port

I was not the server administrator, but there's no way for Alt-Svc to be scoped to a portion of the domain if the content owner is allowed to affect the headers.  An Alt-Svc client talking to such a (legacy, unmodified) server would permit the content owner to hijack traffic intended for other content owners in the future.  Scenarios where new capabilities on the client can be attacked when talking to unmodified servers make me twitch.

The original text in the draft was that servers should either prohibit non-admins from adding an Alt-Svc header or prevent non-admins from accepting incoming connections.  One is a retroactive requirement imposed on legacy servers set up before this draft existed; the other doesn't seem realistic for certain classes of machine.  Those aren't effective mitigations, in my opinion.

My contention was that we should require strong auth for any change of endpoint, to close attacks like this.  However, the Opp-Sec draft relies on Alt-Svc *not* requiring strong auth to change ports on the same host, and that change would break Opp-Sec in the form that consensus is/was pushing it.  The system/user port split was a compromise that I'm still only marginally happy with, but that's where we are.

If there's less consensus now on keeping a hole open for Opp-Sec to crawl through, I'd be thrilled to see that tightened back up.

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Barry Leiba
Sent: Thursday, December 31, 2015 8:39 AM
To: Julian Reschke <[email-masked]>
Cc: [email-masked]; HTTP Working Group <[email-masked]>
Subject: Re: AD review of draft-ietf-httpbis-alt-svc-10

>> I don't understand how this stops a downgrade attack if the 
>> alternative service has better security than the existing connection.  
>> The attacker prevents me from establishing the better security, so I 
>> consider the alternative service to have failed and fall back to the 
>> existing connection... and the attack has succeeded, blocking me from 
>> upgrading the security.  No?

I didn't see a response to this.  I expect we might see comments about this from the Security Directorate or ADs.

> I'm not super-excited about this notation, and it seems we would be 
> the first ones to actually use it (implying lack of validation tools etc).
>
> What do others think?

It's a small thing, and it's up to the working group, of course.  I would prefer the change, because (1) I think it makes it more readable, and (2) we have put 7405 on the Standards Track, so we should use it.  It wouldn't be a bad thing for us to break ground on it.

>>     alt-authority = DQUOTE [ uri-host ] ":" port DQUOTE END
>
> In HTTP specs, we don't like to use DQUOTE unless the thing being 
> quoted used quoted-string syntax.

I don't understand that.  I particularly don't understand why we'd prefer to specify the content of the string only in the comment, when it's perfectly easy and clear to put it into the ABNF, and have it be verifiable.

I'm generally not a fan of putting substantive ABNF information into ABNF comments, and prefer to reserve that sort of thing for cases when it's *not* reasonable to do otherwise.

> That wasn't a typo, but as you are at least the second one complaining 
> I changed it in 

- "The field value consists either of a list of values, each of which indicates one alternative service"

- "The field value consists either of a list of values, each indicating one alternative service"

>> matter, why not simply "1" ?  Other specifications might then add 
>> other values using << persist =/ "2" >>, for example.
>

2. Why does "persist" have to be digits at all?  I'm generally not a fan of unnecessarily coding concepts into numbers, rather than using short words.  If it's necessary (or useful), that's fine.  I don't see why here.

3. Do you expect a lot of additional values?  Clearly not, if you're limiting it to ten possible ones.  In that case, as above, I prefer when the ABNF is specific about it.  Consider, for example, RFC 2045's definition of Content-Transfer-Encoding (in Section 6.1):

Here, "mechanism" could have just been defined as << ietf-token / x-token >>, but enumerating the existing values in the ABNF is useful.
And the fact is that "persist=2" is not currently valid.

There's also, I suppose, the question of whether you'd want "persist=2" to be considered acceptable (and ignored), and "persist=a"
to cause the whole field to be rejected as invalid.  If so, maybe this, to specify what's valid now and how to extend?:

>> -- Section 9.1 --
>> The third paragraph assumes that system ports are inherently more 
>> secure than user ports, and, as discussion during the development of 
>> RFC 6335 raised, that hasn't been the case for some time.  Many 
>> systems no longer make a distinction, and no longet require root 
>> authority to listen on system ports.
>

There was, of course, nothing actionable in my comment, so let me add something here: Because I think there's little value in telling clients to be more strict about authentication when switching from a system port to a user port, I think it could give a false sense of security if implementors do that... particularly if they think they can be lax about authentication on system ports, because only trusted software can listen on system ports.  We should be careful about giving that impression.

> That's an interesting proposal. What do the client implementers think?

And I'm leaving this in the message just to encourage further discussion as people <strike>sober up</strike> return from their new year's celebrations.

Barry

---

From: Mike Bishop <[email-masked]>
Date: Thu, 31 Dec 2015 17:54:32 +0000

"persist" could as easily be a toggle; either present or not, no value.  However, the existing syntax doesn't permit that, so we defined it to be =1.  In this situation, I don't see a problem with hard-coding the value into the syntax.

Fundamentally, the question is, "If I see persist=2, what should I do with it?"  If I treat it as an unrecognized value, then it's equivalent to not being present, which may or may not be what the sender wanted.  That means whoever is defining persist=2 would probably have done better to define morerefinedpersist=1-4, and leave persist intact for legacy clients to understand.

If you're going to have to define a new token for other values to be useful anyway, let's formalize that and hard-code that there's only one acceptable value for this one.

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Barry Leiba
Sent: Thursday, December 31, 2015 9:41 AM
To: Julian Reschke <[email-masked]>
Cc: [email-masked]; HTTP Working Group <[email-masked]>
Subject: Re: ABNF related feedback to: Re: AD review of draft-ietf-httpbis-alt-svc-10

> There's (IMHO) absolutely no point to force this into a single ABNF 

OK: We'll go ahead and disagree on this, but I won't argue the point further.  If anyone else has a comment, let's hear it.  Otherwise, we'll go with status quo.

>> when the ABNF is specific about it.  Consider, for example, RFC 
>> 2045's definition of Content-Transfer-Encoding (in Section 6.1):
>>
> I respectfully disagree. For me, (ab)using the ABNF to enumerate 
> values (in particular when they are extensible) is an anti-pattern.

b

---

From: Barry Leiba <[email-masked]>
Date: Thu, 31 Dec 2015 12:54:20 -0500

Thanks for the details, Mike.  Maybe it'd be good to loop Stephen
Farrell in here, and see whether he's OK with what's there, or has
other suggestions.

Barry