Subject: [TLS] Heartbleed / protocol complexity

From: Hanno Böck <[email-masked]>
Date: Wed, 9 Apr 2014 23:25:05 +0200

Hi,

It's kinda surprising that nobody yet started a thread on the biggest
issue in TLS these days on the TLS WG list. So I make a start.

First thought might be "no protocol issue, software bug, not an issue
for the TLS standardization process". But I think there is an issue at
hand here: We have a severe bug in a rarely used TLS extension.

And: This is the second time in just a few days a TLS extension gives
headache. We just had the Dual EC paper exposing possible security
issues with the Extended Random extension (which luckily never came to
life).

I see a number of issues here:
* Heartbeat extension is enabled in cases where it most likely will
  never be needed or used (HTTPS), but it still causes problems. That
  shouldn't be.
* Extensions make the protocol more complex. Complexity adds attack
  surface. Someone recently said "TLS 1.3 sounds like TLS 1.2 minus
  some features". Probably that's a good way to go forward. IMHO the
  lesson learned should be: TLS extensions shouldn't be added
  carelessly, need good justification and shouldn't be overly complex.
  Same goes for extra ciphers and other things that blow up the
  protocol variations.
* Heartbeat adds some completely unneccessary complexity by having a
  payload with an arbitrary length. There's no point in that. Fefe
  wrote something about it (german only):
  http://blog.fefe.de/?ts=adba343f
  (I don't like his name blaming but he has a point on heartbeat and
  payload)
  Lesson to learn: If it is decided that a new extension is needed it
  should be as simple as possible.

Thoughts?

-- 
Hanno Böck
http://hboeck.de/

mail/jabber: [email-masked]
GPG: BBB51E42

---

From: Martin Thomson <[email-masked]>
Date: Wed, 9 Apr 2014 14:28:20 -0700

On 9 April 2014 14:25, Hanno Böck <[email-masked]> wrote:
> If it is decided that a new extension is needed it

That's a motherhood statement.  Yes, RFC 6520 could have been simpler,
but it does provide a valuable function.  The payload included.

---

From: Nico Williams <[email-masked]>
Date: Wed, 9 Apr 2014 17:23:34 -0500

> issue in TLS these days on the TLS WG list. So I make a start.

Standard IDL + standard encoding + tooling == this type of problem
mostly goes away and it's easier to test and fuzz test unit
components.

TLS has an ad-hoc IDL and encoding, and it IIUC doesn't adhere to its
own conventions tightly enough that we could now standardize a
compatible IDL+encoding and develop tooling for it.

To all who have ever poo-poo'ed ASN.1 and friends I now say: you've
definitevly lost the argument.  If you don't like one IDL, use
another, or develop a new a new one.

> I see a number of issues here:

I don't agree with all that you say, but even if I did, all of these
issues pale by comparison to the IDL/encoding issue.

>   surface. [...]

Maybe, but we need extensibility, sometimes to get us out of a hole.
No extensibility == insecure fallbacks in the future.

There are no right answers in some cases.

>   wrote something about it (german only):

I don't think it's unnecessary: at least for DTLS it is probably
necessary for PMTUD.  If we have no use for it in TCP it might be
worth turning it off there, but that's fighting the last war by moving
complexity elsewhere -- the more DTLS and TLS diverge the less common
code can be shared.

Nico
--

---

From: "Salz, Rich" <[email-masked]>
Date: Wed, 9 Apr 2014 18:42:59 -0400

> TLS has an ad-hoc IDL and encoding, and it IIUC doesn't adhere to its own conventions tightly enough that we could now standardize a compatible IDL+encoding and develop tooling for it.

I wrote a parser for "TLS IDL" and posted it to the list.  There are a handful of corrections that need to be made in order for the definitions to match the defined syntax.  The biggest one being "ASN.1Cert" looks like a field name, not a type.  The posts are in the archives, if anyone cares.  I'll mail the code to anyone who cares.

	/r$

--  
Principal Security Engineer
Akamai Technology
Cambridge, MA

---

From: Geoffrey Keating <[email-masked]>
Date: 09 Apr 2014 15:43:03 -0700

> issues with the Extended Random extension (which luckily never came to

I can add a few more examples: compression and client-initiated
renegotiation are two things that were optional to implement, and
that some implementations didn't implement and were saved security
vulnerabilities by not doing so.

I seem to remember that there have also been bugs with NULL or
'export' cipher suites being unexpectedly enabled.

I think perhaps the lesson to learn from this is that implementations
should probably not aim to be 'complete' or 'full-featured' TLS
implementations, but rather only implement what is needed for their
target audience; and if they do implement features that only some users
will need, disable them by default.

---

From: Nico Williams <[email-masked]>
Date: Wed, 9 Apr 2014 18:06:38 -0500

> I wrote a parser for "TLS IDL" and posted it to the list.  There are a handful of corrections that need to be made in order for the definitions to match the defined syntax.  The biggest one being "…

That's great news.  Can you say anything about my "IIUC" above?

Nico
--

---

From: Nico Williams <[email-masked]>
Date: Wed, 9 Apr 2014 18:13:16 -0500

> vulnerabilities by not doing so.

I agree as to compression: we know well now that compression decisions
must not be done at any layer other than that which knows if it's safe
to do it, and that layer is never TLS.  (This doesn't mean that the
app couldn't request that some TLS records be compressed while others
not, but that would be needless complexity.)

I don't agree as to client-initiated renegotiation.  The problem there
is that the semantics of renegotiation were not well specified early
on, so the APIs that got built too easily ended up failing to be
capable of doing the necessary channel binding, so the channel binding
didn't get built.  (Causality actually went the other way here, I
know, but in so far as running code was the source of
specification...)

> will need, disable them by default.

If you said that apps should get to specify which TLS features they
need I'd agree.  Non-implementation with no choice in the matter
simply leads to non-deployment of important features.  There's no need
to throw out the baby with the bathwater.

Nico
--

---

From: "Salz, Rich" <[email-masked]>
Date: Wed, 9 Apr 2014 21:42:54 -0400

>> TLS has an ad-hoc IDL and encoding, and it IIUC doesn't adhere to its own conventions tightly enough that we could now standardize a compatible IDL+encoding and develop tooling for it.

I'm not quite sure what you mean.  Are you saying that you think the IDL doesn't match what is actually put out on the wire?  Or the IDL->wire mapping is broken/wrong in places, or what?

I haven't looked at generating "stubs" from the IDL, so I guess I really can't comment.
	/r$

---

From: Watson Ladd <[email-masked]>
Date: Wed, 9 Apr 2014 18:44:38 -0700

> That's great news.  Can you say anything about my "IIUC" above?

Parsing an IDL isn't the problem. The problem is developing a
validated parser generator for the language which the IDL expresses.
The further problem is that the semantics of the Hello messages are
extremely ugly. Peter Gutmann compared it to ordering from a Chinese
menu at one point when discussing ECC ciphersuites, and I don't think
that's the only extension with that property.

Even if you succeed in parsing, the extensibility of TLS is (largely)
ridiculous. Security protocols and their implementations are part of
the TCB. They need to be simple.

Sincerely,
Watson Ladd

-- 
"Those who would give up Essential Liberty to purchase a little
Temporary Safety deserve neither  Liberty nor Safety."
-- Benjamin Franklin

---

From: "Manger, James" <[email-masked]>
Date: Thu, 10 Apr 2014 12:25:42 +1000

> another, or develop a new a new one.

I agree Nico.

The heartbeat message involved in this bug is specified in RFC6520 "TLS and DTLS Heartbeat extension" as:

   struct {
      HeartbeatMessageType type;
      uint16 payload_length;
      opaque payload[HeartbeatMessage.payload_length];
      opaque padding[padding_length];
   } HeartbeatMessage;

I think the exact same bytes on the wire would be achieved with the following alternative.

   struct {
      HeartbeatMessageType type;
      opaque payload<0..2^16-1>;
      opaque padding[padding_length];
   } HeartbeatMessage;

Explicitly indicating a variable-length field with <0..2^16-1> means the a value starts with a length.
I wonder is this change would have made a difference? Do most implementations support variable-length fields automatically with some lower-layer bounds checks? Or do most implementations leave this to heartbeat-specific code?

--
James Manger

---

From: Nikos Mavrogiannopoulos <[email-masked]>
Date: Thu, 10 Apr 2014 09:57:29 +0200

> issue in TLS these days on the TLS WG list. So I make a start.
[...]
>   never be needed or used (HTTPS), but it still causes problems. That

Implementations are free to enable any extensions by default or not. I
wouldn't expect the heartbeat extension to be enabled in most
implementations.

>   wrote something about it (german only):

There is. The heartbeat extension can be used under DTLS to perform path
MTU discovery.

>   should be as simple as possible.

Indeed, and in that case the heartbeat extension is as simple as
possible.

regards,
Nikos

---

From: Hannes Tschofenig <[email-masked]>
Date: Thu, 10 Apr 2014 11:26:41 -0700

Hi Hanno,

I agree with Martin regarding your the statement about simplicity. We
always try to make extensions as simple as possible. Questions that come
to my mind are: Have you provided comments at the time when RFC 6520 was
written? Have you helped to improve the OpenSSL implementation of RFC 6520?

There is an underlying problem that needs to be addressed, namely that
we produce specifications but once we are done with the specifications
we (as an organization) don't seem to care about what happens
afterwards. Someone there is the impression that the developer community
implements our protocols quickly and in the way we want. Assuming the
existence of code that is available to the public does not necessarily
mean that it is bug-free.

We have many examples of this behaviour throughout the IETF (i.e., there
is nothing unique to TLS).

In the past I had argued that we need to take the transition from the
specification (paper) to the actual implementation a bit more serious
but, unfortunately, I found very little support for it. Just to give you
a pointer to my most recent write-up on this topic have a look at my
STRINT paper: https://www.w3.org/2014/strint/papers/62.pdf

Ciao
Hannes

On 04/09/2014 04:28 PM, Martin Thomson wrote:
> _______________________________________________
> TLS mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tls
>

---

From: Dan Brown <[email-masked]>
Date: Fri, 11 Apr 2014 00:20:30 +0000

Could some test vectors have helped prevent this bug?

Sent from my BlackBerry 10 smartphone on the Rogers network.
  Original Message
From: Hannes Tschofenig
Sent: Thursday, April 10, 2014 7:55 PM
To: Martin Thomson; Hanno Böck
Cc: [email-masked]
Subject: Re: [TLS] Heartbleed / protocol complexity

Hi Hanno,

Ciao
Hannes

---

From: Watson Ladd <[email-masked]>
Date: Thu, 10 Apr 2014 19:24:45 -0700

> Could some test vectors have helped prevent this bug?

Systematically ensuring that all invalid length combinations would
have triggered an error would have worked. That's more than some test
vectors. Furthermore, it's not clear how you supply test vectors to a
protocol implementation: it can be done, and should be, but it is
quite a bit of work.

What could have mitigated this bug would be not disabling OS
protection measures in malloc. Had OpenSSL not used their own malloc,
then on OpenBSD the malloc flags "FGJZ" would have significantly
reduced the impact of the attack, and quite possibly crashed the
process if exploitation was tried. (OpenSSL has bugs that are only
found when you disable their own malloc wrapper, which is why they
didn't do it).

Various proposals for taint analysis in C have floated around for a
number of years. However, qmail demonstrates that one can write secure
C code without this assistance. Even a typedef int untrust len,
followed by a compiler that warns of aliasing and a perl script that
brings special attention onto casts from this type (okay, not so easy
to write: you need to track declarations) would have been useful.

Furthermore, there was no need to implement heartbeats over TLS for
web servers. The TLS wire format is a pain to parse, and you need to
handle it even after ending the handshake.

However, the fundamental problem isn't anything more than the
developers of OpenSSL being lackadaisical about security. Entire
multiuser operating system kernels (xv6 to be precise) can be fitted
into one fifth the code size of OpenSSL.  PolarSSL is one tenth the
size, and does the same job. miTLS does a much better job, is in a
high level language, costs 10x as much in computation time, and is
guaranteed to be correct. (It doesn't do X509 verification however,
and I don't believe side channels are reasoned about: both are fixable
without much more code).

Sincerely,
Watson Ladd

---

From: Hanno Böck <[email-masked]>
Date: Fri, 11 Apr 2014 16:47:46 +0200

On Wed, 9 Apr 2014 14:28:20 -0700

> but it does provide a valuable function.  The payload included.

While we're at it:
Can anyone name me what real-world applications use TLS Heartbeat with
TCP? Or any use at all?
I'm really asking out of curiosity.
Because I read so much about the whole thing and nowhere was
anything mentioned at all.

-- 
Hanno Böck
http://hboeck.de/

---

From: [email-masked] (Martin Rex)
Date: Fri, 11 Apr 2014 17:49:52 +0200 (CEST)

> Can anyone name me what real-world applications use TLS Heartbeat with

Stuff like QUANTUMINSERT and FOXACID possibly.

-Martin

---

From: Nico Williams <[email-masked]>
Date: Fri, 11 Apr 2014 11:17:30 -0500

> Can anyone name me what real-world applications use TLS Heartbeat with

I can't think of existing applications that need it in TLS-over-TCP today.

For DTLS (over UDP, natch) this extension is needed for PMTUD.

Nico
--

---

From: Patrick Pelletier <[email-masked]>
Date: Sun, 13 Apr 2014 19:03:19 -0700

> protocol implementation: it can be done, and should be, but it is

Presumably in the form of a pathological client (or server) that sends 
nasty things to its peer, and checks how it responds to them.  Although 
it would be a lot of work, I think the first step would just be 
establishing such a test suite that could be used by all TLS 
implementations, and then many could contribute to it over time.

> found when you disable their own malloc wrapper, which is why they

Can you elaborate on this, or point me to a place where it has been 
discussed before?  Although crypto/mem.c is complicated enough that I 
could easily be missing something, I thought that by default, 
CRYPTO_malloc essentially just wraps the system malloc.  How does this 
wrapper make the system malloc behave any differently?

> C code without this assistance.

One *can* write secure code in any language, but it's a question of ease 
and probability.  Even though DJB can write secure C code, I'm of the 
opinion that on the whole we'd be a lot better off if security-critical 
code was written in a memory-safe language.

> web servers.

Yes.  It seems like TLS/DTLS could benefit from "profiles" indicating 
which features are recommended for which uses.  There could be a "web 
profile", a "mail profile", an "Internet of Things" profile. etc.

> developers of OpenSSL being lackadaisical about security.

I'm inclined to agree with you, but to be a bit more charitable and 
specific, I think the problem boils down to two main things:

1) Lack of process.  It's unclear who's in charge of OpenSSL, how they 
decide what features to put in or leave out, when they decide to fix 
bugs, and so forth.  Bugs often languish in the bug tracker, without 
ever being marked "yes, we will fix this" or "no, we won't."  For an 
outsider, it's unclear how to become involved in OpenSSL in a 
constructive way, since there seems to be a lack of structure, and 
almost a certain hostility to outsiders.

2) Trying to be all things to all people.  OpenSSL is a huge, 
general-purpose cryptography library, of which TLS is only a part, 
despite the name.  Besides having the kitchen sink, feature-wise, it 
also tries to be extremely portable.  Supporting obscure, obsolete, or 
embedded operating systems means the code is much more complicated than 
it needs to be to support typical use on mainstream desktop and server 
operating systems.  Not to mention that a lot of code is doubled due to 
the FIPS versus non-FIPS distinction.

> miTLS does a much better job, is in a
> guaranteed to be correct.

Yes, miTLS seems very tantalizing; it seems like it ought to be used 
more.  (Although I shouldn't be one to talk, since I'm using OpenSSL in 
my project at work, despite all of its horribleness.)

--Patrick

---

From: Bill Frantz <[email-masked]>
Date: Mon, 14 Apr 2014 20:26:34 -0700

>Can anyone name me what real-world applications use TLS Heartbeat with

When I designed the E language <www.erights.org> line protocol 
<http://www.erights.org/elib/distrib/vattp/index.html> back in 
the late 1990s, I implemented a heart beat function. At the 
time, we rejected TLS because its security model didn't support 
our "no central authority" needs. A few years later, some people 
figured out how to use TLS, but that has never been coded.

Cheers - Bill

-----------------------------------------------------------------------
Bill Frantz        |The nice thing about standards| Periwinkle
(408)356-8506      |is there are so many to choose| 16345 
Englewood Ave
www.pwpconsult.com |from.   - Andrew Tanenbaum    | Los Gatos, 
CA 95032