Re: [TLS] TLS1.3 + PSK with multiple identities

2016-10-03 Thread Olivier Levillain
Hi list,

I have been working in the labs at ANSSI (the French Network and
Information System Agency) for several years and I just defended my PhD
thesis on the TLS ecosystem (documents are available at
http://paperstreet.picty.org/~yeye/2016/phdthesis-Levillain16/).

>> On Mon, 2016-09-19 at 10:29 +0200, Nikos Mavrogiannopoulos wrote:
>>> On Mon, 2016-08-08 at 11:28 +0300, Ilari Liusvaara wrote:
 More compact and makes the option where server sends some bad option
 more clear.
>> Note that if we really want to be more compact, we might also observe
>> that there is a redundant length field in the extension as sent by the
>> client:
>>case client_hello:
>>PskIdentity identities<6..2^16-1>;
>>
>> Each extension has at least four bytes on the wire — the extension_type
>> itself, and the length field of the extension_data in a uint16.
>>
>> If I am interpreting the spec correctly, then the data for the
>> PreSharedKeyIdentity extension in the ClientHello then follows that
>> with another uint16, which is *always* a value two lower than the one
>> which immediately precedes it.
>>
>> e.g.
>>
>>   0x00 0x29  // ExtensionType extension_type == 41
>>   0x00 0x14  // opaque extension_data<0..2^16-1>
>> // This length field is entirely redundant:
>> 0x00 0x12  // PskIdentity identities<6..2^16-1>
>>  // First identity:
>>   0x00 0x00  // PskKeyExchangeMode, PskAuthenticationMode
>>   0x00 0x04  // opaque identity<0..2^16-1>
>> 0x44 0x61 0x76 0x65 // "Dave"
>>  // Second identity:
>>   0x00 0x00  // PskKeyExchangeMode, PskAuthenticationMode
>>   0x00 0x06  // opaque identity<0..2^16-1)>
>> 0x43 0x68 0x6c 0x6f 0xc3 0xab // "Chloë"
>>
>> Do we care that the '0x00 0x12' bytes on my third line above are
>> entirely redundant on the wire? Or have I interpreted it wrong?
>>
> Not enough to fix it, this is just the way TLS rolls.

Sorry if I am a little late to the party, but I noticed that even if
this is generally true, I believe it has not always been enforced in TLS
extensions.

In 2006, the IETF standardised the session tickets extension, allowing
for session resumption without server-side state (RFC 4507).
However, no TLS stack implements the specification correctly: even
if the specification described the _content_ of the extension as
a variable-length object (that is an opaque object prefixed by its
length), every implementation ignores this second (useless) length
field.  The RFC 5077, published in 2008, fixes the gap
between the specification and the implementations.

RFC 4507 :

The SessionTicket extension has been assigned
the number 35.  The format of the SessionTicket
extension is given at the end of this section.

  struct {
opaque ticket<0..2^16-1>;
  } SessionTicket;

00 23  Ticket Extension type 35
01 02  Length of extension contents
01 00  Length of ticket
FF FF .. ..Actual ticket

RFC 5077

The SessionTicket extension has been assigned
the number 35.  The extension_data field of
SessionTicket extension contains the ticket.

00 23  Ticket Extension type 35
01 00  Length of extension contents (ticket)
FF FF .. ..Actual ticket


Best regards,
olivier

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Draft 18 review : Hello Retry Request and supported groups cache

2016-11-23 Thread Olivier Levillain
Hi,

>> Being able to send supported_groups does allow a server to choose to make
>> a tradeoff between an extra round trip on the current connection and its
>> own group preferences. One example where a server might want to do this is
>> where it believes that X25519 is likely a more future-proof group and would
>> prefer that, but still believes the client's choice of P256 is suitable for
>> this connection. Always requiring an extra round trip might end up being
>> expensive depending on the situation so some servers might prefer to avoid
>> sending an HRR for a slightly more preferred group.
>>
>> I think that requiring the client to maintain state from the
>> supported_groups puts undue requirements on the client, since not all
>> clients keep state between connections, and those that do usually only keep
>> session state for resumption.
>>
> This matches my view as well.
>
> I agree that the client should not be require to keep state. I do not
> believe that the draft requires this, but if someone thinks otherwise,
> please send a PR to fix.
>

There were actually two points in my message:
 - I was not convinced by this way of signalling a preference without
enforcing it, but I understand that, if we keep supported_groups, it
does not cost much and the client can safely ignore the server sent
extension;
 - however, I found strange that the specification stated that the
client could update its view when seeing this extension, but that it was
not stated in the case of an HRR where updating its views of the
servers' preference would clearly be useful for the future. I only
proposed to add the same text "The client MAY update its view of the
server's preference when receiving an HRR, to avoid the extra round trip
in future encounters".

olivier

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Draft 18 review: Downgrade protection

2016-11-23 Thread Olivier Levillain
Hi,

> Thanks for your comments. I do want this section to be clear.
>
> It would be very helpful if you formatted this as a PR. That would make it
> easier to understand the changes in this text.

The text has been proposed as PR 775
(https://github.com/tlswg/tls13-spec/pull/775).

olivier

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Draft 18 review : Message splitting and interleaving

2016-11-23 Thread Olivier Levillain
Le 23/11/2016 00:24, Eric Rescorla a écrit :
> On Tue, Nov 22, 2016 at 2:18 PM, David Benjamin <david...@chromium.org>
> wrote:
>
>> On Tue, Nov 22, 2016 at 2:05 PM Olivier Levillain <
>> olivier.levill...@ssi.gouv.fr> wrote:
>>
>>> Hi list,
>>>
>>> I am sorry for the very late answer concerning draft 18, but we
>>> (ANSSI) have several remarks after proof-reading the current
>>> specification.
>>>
>>> We are sorry for the multiple long messages.
>>>
>>> If the WG is interested by some of our concerns/proposals, we would be
>>> glad to propose some PRs.
>>>
>>>
>>> = Message splitting and interleaving =
>>>
>>> How to split and interleave subprotocols in TLS has not been clearly
>>> specified in the past, and it would be useful to be crystal clear on
>>> this point.
>>>
>>> In the specification, the subject is first presented in 4.5 (P.61):
>>>Handshake messages sent after the handshake MUST NOT be interleaved
>>>with other record types.  That is, if a message is split over two or
>>>more handshake records, there MUST NOT be any other records between
>>>them.
>>> But I am wondering why this text only concern "messages after the
>>> handshake".  It should always be the case!

I don't know what is your take on this first point, but I think I will
propose something in the PR.

I will try to add/move some text so the information is all present in
section 5.

>> +1. We (BoringSSL) and I believe NSS already forbid this for alerts at all
>> versions.
>>
>> This rule is actually implied by TLS 1.3 already, because we've gotten rid
>> of warning alerts. All alerts terminate the connection, except for
>> end_of_early_data, and end_of_early_data immediately signals a key change.
>> So it is not legal to send two alerts in the same epoch, much less in the
>> same record. (Being explicit about this is good, of course.)
>>
> This seems fine. I would take a PR for this.

Will do.


>  - incidentally, the default behaviour would apply to Heartbeat, as
>>>was the intent of the specification;
>>>  - ApplicationData should be considered as a stream with possibly
>>>0-length records
>>>  - Handshake messages should either come as a sequence of multiple
>>>*entire* messages, or as a fraction of *one* message.  That is,
>>>the number of HS messages inside one record should either be a
>>>round number or strictly less than 1.
>>>
>> What simplifications were you expecting out of this? It seems to me this
>> would be a nuisance to both enforce as a receiver and honor as a sender.
>>
>> Our implementation doesn't try to pack handshake messages into records,
>> but I believe NSS does. NSS folks should confirm, but I expect such
>> implementations just buffer the messages up and flush when the buffer
>> exceeds the records size. That means all kinds of splits are plausible:
>>
>>   [EncryptedExtensions Certifi]
>>   [cateRequest Certificate Cer]
>>   [tificateVerify Finished]
>>
> Yeah, that's how this works in NSS.
>
> I'm not seeing a real benefit in prohibiting this behavior.

The expected benefit was that it was a way to enforce the rule from
section 4.6 (P.64), stating that a Handshake message should not span key
changes. That is why the receiver already had to do some work, and my
proposal was to restrict it even more. Yet I see your point that from
the sender's point of view, this is more complex.

After re-reading the 4.6 part, I find it sufficient. Sorry for the noise
about this.


olivier

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Draft 18 review : Extensions and message reuse

2016-11-23 Thread Olivier Levillain
Le 22/11/2016 22:19, Eric Rescorla a écrit :
> On Tue, Nov 22, 2016 at 11:02 AM, Olivier Levillain <
> olivier.levill...@ssi.gouv.fr> wrote:
>> The first example of such a problem is the signature_scheme and
>> signature_algorithms enums.  In practice, the signature_algorithm must
>> contain information valid for TLS 1.2 and TLS 1.3, but the exact
>> meaning is not the same in both cases.  It would be cleaner to have a
>> new, separate extension called signature_scheme, defining the new
>> enum.  This way, a TLS 1.2/1.3 client implementations would send both
>> extensions.  We would spend 10 bytes on the wire, but it is not that
>> important for the first message which can be kept reasonably long
>> (and which is sometimes padded via the corresponding extension...)
>>
> We certainly could do this, but I don't think it's the right answer, for
> two reasons:
>
> 1. From a protocol perspective, some of the things you could say with
> signature_algorithms weren't coherent. For instance: I support ECDSA_SHA512
> but only with P256 (via supported_curves). So it's actually a retroactive
> improvement to TLS 1.2 to use the code points this way. NSS, at least
> interprets these the same way in TLS 1.2 and TLS 1.3. I believe BoringSSL
> does as well.
> 2. From an implementation perspective, it's easier to just have a single
> external facing surface, which should be the more sensible one, namely
> signature schemes.

I understand your point, but there were different curves with the same
security parameter in TLS 1.2, and advertising some combinations in a
TLS 1.2 / 1.3 (supporting Brainpool curves but not the NIST equivalent)
implementation might be a pain. Considering the current deployment, we
certainly can live with that.

We also were wondering why not reuse the values for hash algorithms for
PSS: if we use 0x04XX for PSS-sha256 and 0x05XX for PSS-sha384, etc. it
would align with existing TLS 1.2 behaviour, and make it easier to
include PSS support in these implementations, as is recommended by the
specification.

>> Another example is the key_share extension: it is in fact made of
>> three different extensions, depending on the message where it is
>> included.  It would be simpler for a (almost-)stateless parser* to
>> either use three different extensions or to use the same content each
>> time.  In the second case, the extension would each time consist of a
>> list of (group + optional keyshare).  The advantages of such a unified
>> keyshare extension would be:
>>  - stateless parsing*
>>  - no reuse of supported_groups which can be merged with key_share
>>  - no more validation complexity since higher-level checks were
>>already required (the groups sent by HRR did not came with a key
>>share for example).
>> The major con one might see is that the server can not advertise its
>> supported groups in an encrypted manner anymore (since
>> supported_groups is not used anymore).  However, we do believe
>> simplifying the implementation is worth it.
>>
>> * Of course the parser still has to use the extension id to know how
>>   to parse the value, but it would not need to have broader context
>>   (the message where the extension is included).  In other words, we
>>   would like select syntax in structs to only use local information.
>>
>> I can try and propose a PR for each extension if there is an interest
>> in the WG.
>>
> It seems to me you are making two points here:
>
> 1. That it would be good to unify supported_groups and key_share
> 2. That we should use separate code points wherever extensions differ
> between the messages they appear (this is far from the only one).
>
> We've had some conversation about the first approach, which seems like kind
> of a stylistic/taste issue whether to have the "I can do" and "here is" in
> one
> place or two, and on balance I think it's better to not change this at this
> point
> without a really compelling reason.

I have no compelling reason, but I don't remember why the WG chose to
stick with two extensions where there was a working design with only one.

> I can see the appeal of having one parser per extension ID, but it isn't
> actually that much easier from an implementation perspective. You already
> need a mapping table from extension ID to handler and that table
> also needs to be able to detect common problems (e.g., extension reuse
> or extensions appearing in the wrong place) and it's not hard at that
> point to also have it point to message-specific parsers (this is how NSS
> mostly works). ISTM that the alternative you propose would either result
> in a proliferation of extensions (which confuses the matching rule

Re: [TLS] Draft 18 review : 0-RTT

2016-11-23 Thread Olivier Levillain
Le 23/11/2016 01:28, Martin Thomson a écrit :
> On 23 November 2016 at 06:07, Olivier Levillain
> <olivier.levill...@ssi.gouv.fr> wrote:
>> In 4.2.8 (P.47), the server receiving early_data "can behave in one of
>> two ways"... followed by three cases.  Beside the typo, the first case
>> could be phrased differently.  Actually, it reads
>>
>>-  Ignore the extension and return no response.  This indicates that
>>   the server has ignored any early data and an ordinary 1-RTT
>>   handshake is required.
>>
>> Since an ordinary 1-RTT handshake will require the server to actually
>> send a response (the ServerHello), it might be better to put it this
>> way:
>>
>>-  Ignore the extension and return a standard 1-RTT ServerHello.
>>   This indicates that the server has ignored any early data and
>>   an ordinary 1-RTT handshake is required.
> Here's a PR: https://github.com/tlswg/tls13-spec/pull/773
>
> I've gone a little bit further than what Olivier suggests and pointed
> out in each of these that the server is required to ignore early data.

Thank you for the rewrite.

olivier

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Draft 18 review : Hello Retry Request and supported groups cache

2016-11-23 Thread Olivier Levillain
>> There were actually two points in my message:
>>  - I was not convinced by this way of signalling a preference without
>> enforcing it, but I understand that, if we keep supported_groups, it
>> does not cost much and the client can safely ignore the server sent
>> extension;
>>  - however, I found strange that the specification stated that the
>> client could update its view when seeing this extension, but that it was
>> not stated in the case of an HRR where updating its views of the
>> servers' preference would clearly be useful for the future. I only
>> proposed to add the same text "The client MAY update its view of the
>> server's preference when receiving an HRR, to avoid the extra round trip
>> in future encounters".
>>
> This is is unsafe, because the HRR is unauthenticated. We could update it
> after the handshake completes, but I think this is obvious enough that it
> doesn't
> need to be stated.

Unless I am mistaken, EncryptedExtensions is not authenticated either
(even if it is sent in the same flight as the authentication messages),
so updating the client cache can not be done immediately after
interpreting the supported_groups extension.

However, if you believe this does not need to be stated, I am fine with
that.

olivier

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Draft 18 review : Message order

2016-11-23 Thread Olivier Levillain
Hi,

> On Tue, Nov 22, 2016 at 11:08 AM, Olivier Levillain <
> olivier.levill...@ssi.gouv.fr> wrote:
>
>> = Message order =
>>
>> I believe the message P.27 section 4 is important, but not
>> sufficient. As already expressed on the list, a formal automaton
>> should be provided in the spec.
>>
>> I think Ekr said there was some work in progress in this area.  Is
>> this a goal for the final specification?
>>
> Yes, I will put some sort of more complete state machine in the final spec
> (probably in -19)

I have been working on my black board on the simplest case (pure TLS, no
PSK support), and I stumble upon the late client authentication issue
that was presented in the thread starting with
https://www.ietf.org/mail-archive/web/tls/current/msg21500.html. I agree
with the thread that NST and KU are easy to deal with.

As stated in the thread, as it is currently specified, the mechanism is
kind of a pandora's box :
 - the client must keep the handshake transcript (or its running /
forkable hash) because of the Finished message that must eventually be sent
 - the server can send multiple CertificateRequest, and the client must,
for each, keep the content of the message (for the label, and also for
the hash context required for the Finished message) => the unboundedness
problem
 - there is no way to reject the request gracefully.

As I understand the thread, one idea was to say that the mechanism is
forbidden by default, unless it is activated by some profile. However,
as stated in
https://www.ietf.org/mail-archive/web/tls/current/msg21561.html, since
there are voices to allow it in the HTTP profile, this is problematic
because HTTPS can be used in constrained environments where we would
like to avoid late client auth. I believe the late client auth mechanism
should explicitly be negotiated, either at the TLS level with an
extension, or at the application level via an API callback (not a
profile) and an upper layer nego.


Sorry to dig this up (and sorry for the re-discovery explained at
length), but where does the WG stand on this issue? PR 680 is currently
parked, but I believe the current situation is not acceptable from the
client's point of view, and I fail to see whether unbounded parallel
certificate requests would be used in practice or not.

Best regards,
olivier

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


[TLS] Draft 18 review: Downgrade protection

2016-11-22 Thread Olivier Levillain
Hi list,

I am sorry for the very late answer concerning draft 18, but we
(ANSSI) have several remarks after proof-reading the current
specification.

We are sorry for the multiple long messages.

If the WG is interested by some of our concerns/proposals, we would be
glad to propose some PRs.


= Donwgrade protection =

On P.32 (section 4.1.3), the part about downgrade protection mechanism
is not clear enough.  As I understand it, the modified server_random
only occurs with a ServerHello indicating TLS 1.2 or below.  Moreover,
a TLS 1.2 client should only abort the handshake with the TLS 1.1
value, which is not clear in the explanation.  Finally, the
ServerKeyExchange is only defined in TLS 1.2 or below, so it would be
better to add some precision.  Here is a proposal to make these points
more explicit:

   TLS 1.3 has a downgrade protection mechanism embedded in the server's
   random value.  TLS 1.3 server implementations MAY respond to a
   ClientHello indicating only support for TLS 1.2 or below with a
   ServerHello containing the appropriate version field.

   TLS 1.3 server implementations which respond with a TLS 1.2
   ServerHello, MUST set the last eight bytes of their Random value
   to the bytes:

 44 4F 57 4E 47 52 44 01

   TLS 1.3 server implementations which respond with a ServerHello
   indicating support for TLS 1.1 or below SHOULD set the last
   eight bytes of their Random value to the bytes:

 44 4F 57 4E 47 52 44 00

   TLS 1.3 clients receiving a TLS 1.2 or below ServerHello MUST check
   that the last eight octets are not equal to either of these values.
   TLS 1.2 clients SHOULD also check that the last eight bytes are not
   equal to the second value if the ServerHello indicates TLS 1.1 or
   below.  If a match is found, the client MUST abort the handshake
   with an "illegal_parameter" alert.  This mechanism provides limited
   protection against downgrade attacks over and above that provided
   by the Finished exchange: because the ServerKeyExchange, a message
   present in TLS 1.2 and below, includes a signature over both random
   values, it is not possible for an active attacker to modify the
   randoms without detection as long as ephemeral ciphers are used.
   It does not provide downgrade protection when static RSA is used.

I can propose a PR if this makes sense to the WG.


Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


[TLS] Draft 18 review : Extensions and message reuse

2016-11-22 Thread Olivier Levillain
Hi list,

I am sorry for the very late answer concerning draft 18, but we
(ANSSI) have several remarks after proof-reading the current
specification.

We are sorry for the multiple long messages.

If the WG is interested by some of our concerns/proposals, we would be
glad to propose some PRs.


= Extensions and message reuse =

We were sorry to find that some TLS 1.2 extensions were reused with a
different meaning, or that some TLS 1.3 extensions were context
dependent, which does not allow for a clear separation between the
parsing step (which might lead to a decode_error alert) and the
message validation step (which might lead to an invalid_parameter
alert).  It is especially disappointing since the choice made for the
ciphersuites is a very clean one.

The first example of such a problem is the signature_scheme and
signature_algorithms enums.  In practice, the signature_algorithm must
contain information valid for TLS 1.2 and TLS 1.3, but the exact
meaning is not the same in both cases.  It would be cleaner to have a
new, separate extension called signature_scheme, defining the new
enum.  This way, a TLS 1.2/1.3 client implementations would send both
extensions.  We would spend 10 bytes on the wire, but it is not that
important for the first message which can be kept reasonably long
(and which is sometimes padded via the corresponding extension...)

Another example is the key_share extension: it is in fact made of
three different extensions, depending on the message where it is
included.  It would be simpler for a (almost-)stateless parser* to
either use three different extensions or to use the same content each
time.  In the second case, the extension would each time consist of a
list of (group + optional keyshare).  The advantages of such a unified
keyshare extension would be:
 - stateless parsing*
 - no reuse of supported_groups which can be merged with key_share
 - no more validation complexity since higher-level checks were
   already required (the groups sent by HRR did not came with a key
   share for example).
The major con one might see is that the server can not advertise its
supported groups in an encrypted manner anymore (since
supported_groups is not used anymore).  However, we do believe
simplifying the implementation is worth it.

* Of course the parser still has to use the extension id to know how
  to parse the value, but it would not need to have broader context
  (the message where the extension is included).  In other words, we
  would like select syntax in structs to only use local information.

I can try and propose a PR for each extension if there is an interest
in the WG.


Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


[TLS] Draft 18 review : PSK and PSK Binder

2016-11-22 Thread Olivier Levillain
Hi list,

I am sorry for the very late answer concerning draft 18, but we
(ANSSI) have several remarks after proof-reading the current
specification.

We are sorry for the multiple long messages.

If the WG is interested by some of our concerns/proposals, we would be
glad to propose some PRs.


= PSK and PSK Binder =

P.35 (4.2), it is specified that the pre_shared_key extension must be
the last extension in the ClientHello, because of the way the PSK
Binder is computed.  This seems very hackish and will most certainly
lead to implementation errors.  However, I understand that it was not
easy to propose a cleaner mechanism while keeping a common flow
between *DHE and PSK modes.

Yet, we would have preferred that PSK would not be MTI as stated in
the end of the document (P.81, section 8.2).  In previous versions PSK
and resumption was not MTI, so it might be logical to keep it this
way.  Alternatively, we might propose a profile defining a simpler TLS
subset.

Regarding the definition of the PSK Binder (P. 45 and 46, section
4.2.6.1), we found it very hard to read, since the binding value is
said to be computed as the Finished message, but differently.  In
particular, it would be useful to give the relevant information (I
believe the Handshake Context is called transcript in this section,
and it would be important to explicit the Hash to use), instead of
implying it.  It would not take much space in the document, but it
would ensure implementers know how to compute the binder.


Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


[TLS] Draft 18 review : 0-RTT

2016-11-22 Thread Olivier Levillain
Hi list,

I am sorry for the very late answer concerning draft 18, but we
(ANSSI) have several remarks after proof-reading the current
specification.

We are sorry for the multiple long messages.

If the WG is interested by some of our concerns/proposals, we would be
glad to propose some PRs.


= 0-RTT =

In 4.2.8 (P.47), the server receiving early_data "can behave in one of
two ways"... followed by three cases.  Beside the typo, the first case
could be phrased differently.  Actually, it reads

   -  Ignore the extension and return no response.  This indicates that
  the server has ignored any early data and an ordinary 1-RTT
  handshake is required.

Since an ordinary 1-RTT handshake will require the server to actually
send a response (the ServerHello), it might be better to put it this
way:

   -  Ignore the extension and return a standard 1-RTT ServerHello.
  This indicates that the server has ignored any early data and
  an ordinary 1-RTT handshake is required.


Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


[TLS] Draft 18 review : Message splitting and interleaving

2016-11-22 Thread Olivier Levillain
Hi list,

I am sorry for the very late answer concerning draft 18, but we
(ANSSI) have several remarks after proof-reading the current
specification.

We are sorry for the multiple long messages.

If the WG is interested by some of our concerns/proposals, we would be
glad to propose some PRs.


= Message splitting and interleaving =

How to split and interleave subprotocols in TLS has not been clearly
specified in the past, and it would be useful to be crystal clear on
this point.

In the specification, the subject is first presented in 4.5 (P.61):
   Handshake messages sent after the handshake MUST NOT be interleaved
   with other record types.  That is, if a message is split over two or
   more handshake records, there MUST NOT be any other records between
   them.
But I am wondering why this text only concern "messages after the
handshake".  It should always be the case!

It is next present in section 4.5.3 (P.64):
   Handshake messages MUST NOT span key changes.  Because the
   ServerHello, Finished, and KeyUpdate messages signal a key change,
   upon receiving these messages a receiver MUST verify that the end of
   these messages aligns with a record boundary; if not, then it MUST
   terminate the connection with an "unexpected_message" alert.

And then in section 5 (mostly P.64 and P.65) plus a repetition P.69.


It is worth noting that this specific point was (at least partially)
the origin of several security issues:
 - the alert attack (https://www.mitls.org/pages/attacks/Alert);
 - Heartbleed (where OpenSSL answered with a fragmented record whereas
   it was not the spirit of the spec);
 - the OpenSSL Downgrade attack in 2014.

Some of this is covered in the current specification, but it might be
worth being even stricter:
 - regarding splitting/merging, they should be forbidden by default;
 - the default would apply to alerts (currently, the spec states that
   alerts MUST not be split, but they should not be merged either
   for simplicity's sake and since it is useless);
 - incidentally, the default behaviour would apply to Heartbeat, as
   was the intent of the specification;
 - ApplicationData should be considered as a stream with possibly
   0-length records
 - Handshake messages should either come as a sequence of multiple
   *entire* messages, or as a fraction of *one* message.  That is,
   the number of HS messages inside one record should either be a
   round number or strictly less than 1.

These constraints can be expressed in 5.1 and cover most of the
problematic cases.  They are easy to check and to enforce.


However, the problem of the OpenSSL Downgrade flaw (where the first
record containing the beginning of the ClientHello was cut before the
ClientHello.version field) will still stand, but there is no simple
way to fix this, since version negotiation can now happen anywhere in
the ClientHello extensions, so it might be impossible to enforce that
the client versions are sent in the first record.

I might volunteer some text if there is an interest of the WG.


Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


[TLS] Draft 18 review : Message order

2016-11-22 Thread Olivier Levillain
Hi list,

I am sorry for the very late answer concerning draft 18, but we
(ANSSI) have several remarks after proof-reading the current
specification.

We are sorry for the multiple long messages.

If the WG is interested by some of our concerns/proposals, we would be
glad to propose some PRs.


= Message order =

I believe the message P.27 section 4 is important, but not
sufficient. As already expressed on the list, a formal automaton
should be provided in the spec.

I think Ekr said there was some work in progress in this area.  Is
this a goal for the final specification?


Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


[TLS] Draft 18 review : Minor remarks and typos

2016-11-22 Thread Olivier Levillain
 only true when the
   client authenticates itself with a certificate.  This point is
   related to a point sent by Ben Kaduk
   (https://www.ietf.org/mail-archive/web/tls/current/msg21829.html):
   "In section 4.5.1 we note that when client auth is not used,
   servers MAY compute the remainder of the client-sent messages for
   the transcript so as to issue a NewSessionTicket immediately after
   the server Finished.  Do we want to say anything about why a server
   might wish to do so?"
   (https://www.ietf.org/mail-archive/web/tls/current/msg21829.html).
   to which Ekr answered with "I think I would rather not."

 - P.63 (4.5.3): The definition of the "request_update" field is
   unclear. It is said to "indicate that the recipient of [...] should
   respond".  Should not it say "Indicates *whether* the recipient"

 - P.65 (5.1): "Endpoints supporting other versions" => "earlier"?

 - P.66 (5.2): the content type if the TLS InnerPlaintext must be non
   null for the padding to work.  This is correctly stated in the end
   of the specification where 0 is invalid_RESERVED, but it might be
   useful to add a statement in the type field definition: "Valid
   content types all have non-null values."

 - P.67 (5.2): The exact same text is repeated twice in the page: "An
   endpoint that receives a record that exceeds this length MUST
   terminate the connection with a "record_overflow" alert."

 - P.81 (8.1): RSA signature schemes are specified to be MTI for
   certificates and optionally for CertificateVerify.  The precision
   should also be added for ecdsa_secp256r1-sha256.

 - P.83 and 85 (8.2): "Client" is supposed to mean that "the server
   shall not send them".  Yet, the cookie extension is marked as
   Clear, and can be sent in the HRR message.

 - P.84 (8.2): Since it is said afterwards that truncated_hmac is a
   dangerous extension, it might be logical to mark it as *not*
   recommended?

 - P.84 and 85: I suppose that the order of the extensions in this
   table is given by the extension id.  Yet it might be more practical
   to sort the table by extension name?

 - P.105 (B.4): "Do you verify that the RSA padding doesn't have
   additional data after the hash value?  [FI06]" => the attack was
   about PKCS#1 v1.5, not PSS.  Does it still apply, or should the
   text be amended?

 - P.105 (B.4): I believe the part about ECDSA "k" parameter states
   opposing statements, since deterministic ECDSA leads to a
   non-random k:
  Do you use a strong and, most importantly, properly seeded random
  number generator (see Appendix B.2) when generating Diffie-Hellman
  private values, the ECDSA "k" parameter, and other security-
  critical values?  It is RECOMMENDED that implementations implement
  "deterministic ECDSA" as specified in [RFC6979].
   Would not it be more logical to say: "Alternatively, it is
   RECOMMENDED..."

 - P.113 (D.2): There are missing references concerning the analysis
   of the TLS record layer

= Typos =

 - P.28 (section 4.1.1): a quote is missing after '"psk_key_exchange_modes'
 - P.46 (4.2.7): "A clients MUST provide" => "client"
 - P.54 (4.4.1): "If an extension applies the the entire chain" => "to the"
 - P.58 (4.4.2): "A single 0 byte which servers as the separator" => "serves"
 - P.63 (4.5.3): "If the request_udate" => request_update
 - P.86 (8.2): "and the TLS SignatureAlgorithm Registry to list values 4-233 as 
"Reserved"" => It should be 223 (as 224-255 are reserved for private use)


Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


[TLS] Draft 18 review : Signature in certificates

2016-11-22 Thread Olivier Levillain
Hi list,

I am sorry for the very late answer concerning draft 18, but we
(ANSSI) have several remarks after proof-reading the current
specification.

We are sorry for the multiple long messages.

If the WG is interested by some of our concerns/proposals, we would be
glad to propose some PRs.


= Signature in certificates =

The two paragraphs in 4.4.1.2 P.56 starting with "All certificates"
are very far from clear.  They require (MUST) some behaviour, which is
later reformulated with an unless part.  I am not sure of the intent
here, but we believe the current text should be rewritten to clearly
express the intent of the WG.

My comprehension is that the server MUST use only signature schemes
described in signature_algorithms, except for the following cases:
 - for checking the signature in self-signed or trust anchors (since
   this check is useless, the trust coming from an out-of-band
   mechanism in this case)
 - when the only available chains use signature scheme are not known
   to be supported by the client
 - the case of SHA-1 is special

The same confusion can be found in 4.4.2 P.59 ("If sent by a
server...")


Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Deprecating alert levels

2016-10-26 Thread Olivier Levillain
Hi list,

I recently saw a related CVE regarding OpenSSL on oss-security mailing
list: CVE-2016-8610. The original mail is
http://seclists.org/oss-sec/2016/q4/224. As I understand it, the idea is
to send a continuous flow of unauthenticated, warning-level alerts in
the middle of the initial handshake. This leads OpenSSL to keep the
connection open (and apparently to consume some CPU).

Even if this might not be the attack of the century, it may also be
another reason to reconsider how a TLS stack should handle
[multiple|warning|unauthenticated] alerts.

olivier


Le 25/10/2016 03:14, Kyle Nekritz a écrit :
> +1 to both Martin and ekr, I think simplifying these alerts with clearly 
> defined behavior for each alert description is the best way forward.
>
> Kyle 
>
> -Original Message-
> From: TLS [mailto:tls-boun...@ietf.org] On Behalf Of Martin Thomson
> Sent: Wednesday, October 19, 2016 10:18 PM
> To: Eric Rescorla 
> Cc: tls@ietf.org
> Subject: Re: [TLS] Deprecating alert levels
>
> On 20 October 2016 at 05:28, Eric Rescorla  wrote:
>>> 2.  Are there cases, such as unrecognized name. where it is useful to 
>>> indicate that an alert is not fatal?  If so how should this case be handled?
>>
>> I think this alert was a mistake :)
> In NSS is to tolerate it, but it's an exception.  I'm happier with a lone 
> exception than with atrophied and redundant alert levels continuing as they 
> are.  I'd prefer to take the PR, with a minor amendment noting the hazard 
> caused by unrecognized_name(112).  Clients that intend to accept TLS 1.2 and 
> lower probably have to ignore warning alerts until they see that the server 
> is doing TLS 1.3 or higher.
>
> ___
> TLS mailing list
> TLS@ietf.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_tls=DQICAg=5VD0RTtNlTh3ycd41b3MUw=l2j4BjkO0Lc3u4CH2z7jPw=1svSdxAuionbHyrUN4ThSCRLZ1pCQuLaO0qtgQ8Dk7A=jWxxDB9uWwT6kP_7TcZ4isUa_Z5LNWOhgMX_O1s3oaw=
>  
>
> ___
> TLS mailing list
> TLS@ietf.org
> https://www.ietf.org/mailman/listinfo/tls

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] WGLC: draft-ietf-tls-tls13-19

2017-03-27 Thread Olivier Levillain
Hi list,

I think there is at least another issue that still needs to be
discussed: how to properly handle post-handshake handshake messages.

The subject has also been raised several times on GitHub
(https://github.com/tlswg/tls13-spec/pull/680,
https://github.com/tlswg/tls13-spec/pull/676,
https://github.com/tlswg/tls13-spec/issues/572) and on the mailing list
(https://www.ietf.org/mail-archive/web/tls/current/msg22038.html).

Bottom line is:
- handling client late authentication requires a lot of state in the
client stack
- currently, handling client late authentication is mandatory


For a longer version, post-handshake records of type Handshake can be of
three kinds:
- NewSessionTicket (sent by the server, and that can safely be ignored
entirely by clients)
- KeyUpdate (sent by either party, requiring only a bit of state)
- CertificateRequest (sent by the server, an arbirary number of times,
and requring the client to keep some state *for each request*)

Of course, this last item makes the post-handshake client state machine
explode, whereas the first two items can ben implemented in a trivial
way. The client can not indeed ignore all this state to answer, since it
is supposed to answer at least with a Finished message, which will cover
the CertificateRequest message. Moreover, since each of these Finished
messages must cover the initial handshake and the current
CertificateRequest message, it requires a forkable hash implementation,
which requires more memory.

A client _could_ ignore CertificateRequest and never answer them, but
this would not really be conformant, and it would be problematic as soon
as the parties need to send Handshake messages.


Thus, I believe the current text is inadequate. Different solutions are
possible :
- remove client late authentication entirely (this would have my
preference, since it introduces other issues*)
- make client late authentication optional (compatible clients would
signal it as an extension)
- rethink the client late authentication, as was done with KeyUpdate,
to limit the state required on the client side.

Best regards,
Olivier Levillain


* One of these issues is that requiring Client Authentication upon an
applicative request may lead a client to have to write records on a
SSL_read call :
1) client and server complete the handshake
2) client sends a request (SSL_write)
3) server reads the request, decides an authentication is required, and
sends a CR
4) client reads the response (SSL_read), but has first to comply with
the CR (that is write packets) to get the response

This violation of the network layers can be dealt with (and has been,
since Apache allows eactly this kind of flow) but it introduces
complexity in the code, which I belive we should remove altogether : an
application request should not trigger an SSL authentication request.
What was done with KeyUpdate sanitized a similar situation.

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] WGLC: draft-ietf-tls-tls13-19

2017-03-31 Thread Olivier Levillain
Hi,

> I think there is at least another issue that still needs to be
> discussed: how to properly handle post-handshake handshake messages.
> 
> The subject has also been raised several times on GitHub
> (https://github.com/tlswg/tls13-spec/pull/680,
> https://github.com/tlswg/tls13-spec/pull/676,
> https://github.com/tlswg/tls13-spec/issues/572) and on the mailing list
> (https://www.ietf.org/mail-archive/web/tls/current/msg22038.html).
> 
> Bottom line is:
> - handling client late authentication requires a lot of state in the
> client stack
> - currently, handling client late authentication is mandatory

[...]

> Thus, I believe the current text is inadequate. Different solutions are
> possible :
> - remove client late authentication entirely (this would have my
> preference, since it introduces other issues*)
> - make client late authentication optional (compatible clients would
> signal it as an extension)
> - rethink the client late authentication, as was done with KeyUpdate,
> to limit the state required on the client side.


Martin Thomson proposed a PR (thank you) corresponding to the second
point, using a simple extension in the ClientHello :
https://github.com/tlswg/tls13-spec/pull/921/

I believe this proposal is a good trade-off allowing simpler
implementation designs when late client authentication is not needed.


Best regards,
Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] I-D Action: draft-ietf-tls-certificate-compression-02.txt

2018-02-14 Thread Olivier Levillain
Hi list,

Le 26/01/2018 à 11:18, internet-dra...@ietf.org a écrit :
> 
> A New Internet-Draft is available from the on-line Internet-Drafts 
> directories.
> This draft is a work item of the Transport Layer Security WG of the IETF.
> 
> Title   : Transport Layer Security (TLS) Certificate 
> Compression
> Authors : Alessandro Ghedini
>   Victor Vasiliev
>   Filename: draft-ietf-tls-certificate-compression-02.txt
>   Pages   : 7
>   Date: 2018-01-26


I am sorry I am late in the process of commenting this draft, but I have
a question about the CompressedCertificate message: why does it contain
an uncompressed_length field?

What is the point of this field which can not be trusted? If the idea is
to give a hint of how long a certificate is, we already know that.

Since the whole goal of this draft is to reduce the size of the message,
I would strongly suggest this field be removed. It is too much a
temptation to be considered as a reliable information for shaky
implementations.

Best regards,
Olivier Levillain

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls