67

"The fix for this bug is simple: check that the length of the message actually matches the length of the incoming request."

Why do we even have the client report the length at all?

If we can know the length of the incoming request, can't we just infer the length of the message from that?

(This is a programming and protocol design question.)

AviD
  • 72,138
  • 22
  • 136
  • 218
Elliot
  • 753
  • 5
  • 9

3 Answers3

37

I am not aware of any definitive, "official" answer on this subject, but this seems to be part of an attempt at genericity and coherence. In the SSL/TLS standard, all messages follow regular encoding rules, using a specific presentation language. No part of the protocol "infers" length from the record length.

One enlightening detail is the ClientKeyExchange message: in SSL 3.0, with an RSA key exchange, the contents of that handshake message are the "raw" encryption result (i.e. 256 bytes if the server uses a 2048-bit RSA key, plus the 4-byte handshake message header); in TLS 1.0 and subsequent versions, the encryption result is an "opaque vector" with an extra two-byte header containing the length, so that's 258 bytes, not 256, and still with an extra 4-byte handshake message header. This change illustrates the will of the standard designers to achieve strict regularity with as little inference from packet length as possible.

One gain of not inferring length from the outer structure is that it makes it much easier to include optional extensions afterwards. This was done with ClientHello messages, for instance.


If we look at things from a higher point of view, TLS is only half-finished. TLS, as a protocol, needs to exchange structured data; sender and receiver must be able to encode and decode such data with no ambiguity, and preferably no buffer overflow either. One strategy for that is to do the following:

  • Define strict, regular encoding rules for structured elements. For instance, define that every single element should be preceded by a header containing the length of the element.
  • Define an unambiguous language to express the structure and type of all messages as part of the protocol standard; that's the "presentation language" of the RFC.
  • Write a generic compiler software, into which you paste the presentation language from the RFC, and which produces an encoder/decoder implementation which handles the encoding and decoding of all messages, with systematic checks on all lengths and value ranges and so on.

An embodiment of this strategy is ASN.1. ASN.1 is used, for instance, in X.509 certificates; with an ASN.1 compiler, you could (theoretically) paste the ASN.1 syntax from the X.509 standard and obtain a complete encoder/decoder engine for X.509 certificates (I have done that, including writing my own ASN.1 compiler, and there are some "quirks", including the fact that the ASN.1 types for strings and dates appear to have been invented by a baboon on LSD; but the overall scheme still works).

Another embodiment is XML, with XML Schema as "presentation language": you push the schema and the input data into your XML parser, and if something other than an error code is produced, then you know that all the data complies with the schema, and you can walk it in a decoded, structured tree. Again, that's the theory, which is nice, as far as theories go.

The "presentation language" of TLS falls a bit short here: it does not have a compiler. That language looks like a "computer language", but it was meant for human reading. For that, it works well; however, this means that the developer will have to transcribe that language into an encoder/decoder. The developer must take care not to forget a step. And that's exactly what occurred with the "heartbleed" bug: a step was forgotten, which can have dire consequences.

Thomas Pornin
  • 320,799
  • 57
  • 780
  • 949
  • 4
    I imagine that if Netscape had invented SSL only about 5 to 8 years later, everything would have been encoded in XML instead of ASN.1 – tylerl Apr 13 '14 at 19:16
  • 2
    Never fear, ASN.1 and XML have spawned a bastard offspring: XER (for "XML Encoding Rules"). – Thomas Pornin Apr 13 '14 at 19:31
32

For TLS with the purpose of liveliness (keep-alive) checks, there's no reason to:

  1. Encode a payload size field in the heartbeat request/response header (the length of the payload comes from the record layer rrec.length in OpenSSL code -- you just have to subtract off the fixed HB header size from this),
  2. Allow HBs to be variable size -- a small HB size (in the range of ~4-32 bytes) would work perfectly -- just enough for sequence number,
  3. Add padding to the payload, OR
  4. Perform PMTU discovery (defined below)

So the design is flawed and overcomplicated in regards to ordinary TLS. Note TLS is the widely used protocol we really care about, encrypting all HTTPS traffic.

In the vulnerable OpenSSL commit all the generated Heartbeat requests have a small fixed payload (18 bytes) and when processing a received HB response, OpenSSL only checks the first two bytes of it which contain the HB sequence number. Source: t1_lib.c (containing all the TLS HB code) when generating a HB (only described in tls1_heartbeat ), it fixes the payload size at 18. Processing a HB response in tls1_process_heartbeat also only does any meaningful processing if the payload is exactly 18. Note processing of a request in TLS is the vulnerable part that undermined HTTPS.

Background

Before getting to the claimed justification, I have to introduce three concepts: DTLS, PMTU, and PMTU discovery that are all unrelated to liveliness checks, but deal with the other proposed use for the Heartbeat extension. Skip to proposed justification if you are familiar.

TLS (encryption on TCP) and DTLS (encryption on UDP)

Regular TLS adds encryption on top of TCP. TCP is a transport layer protocol that provides a reliable transport stream, meaning the application receives a reconstructed data stream with all packets presented to the application in the original order once everything is there, even if some had to wait some extra time for packets to be resent. TCP also provides congestion control (if packets are being dropped because of congestion, TCP will adjust the rate packets are sent). All HTTP, HTTPS, SFTP traffic is sent over TCP.

Datagram TLS (DTLS) is a newer protocol that adds encryption on top of UDP (and similar datagram protocols like DCCP where an application has full control on how to send packets). These are transport layer protocols that do not provide reliable streams, but send packets directly between client/server applications as controlled by an application. With TCP if a packet is lost it automatically gets resent and delays sending further packets until the missing packets get through. UDP gives packet level control to the application, which is often desirable for real-time communication like two-way video chat. If packets A, B, C, D were sent but packet C was lost, it doesn't make sense to either wait for C to be resent before showing packet D to the user -- causing a lengthy pause.

PMTU

For DTLS, it is desirable to know the path maximum transmission unit. An MTU for a single link between routers is maximum packet size that can be sent. Different routers and types of links often support different MTUs. The Path MTU (the smallest MTU on the path your packets take through the network) will generally not be known beforehand as its a property of the path through the network. If you send datagrams that are larger than the PMTU, they would have to fragment at the smallest MTU point which is undesirable for several reasons (inefficient, fragmented packets may be dropped by firewalls/NAT, its confusing to the application layer, and ipv6 by design will never fragment packets). So in the context of DTLS, the RFC forces the data from your record layer to fit in a single DTLS packet (that is smaller than the PMTU). (With TLS these PMTU issues are handled at the TCP level; not the application layer so you TLS can be agnostic to PMTU).

PMTU discovery

There are protocols to discover PMTU — specifically packetization layer Path MTU discovery (RFC 4821). In this context, you probe the network by sending packets of various size (configured to not fragment) and keep track of the upper bound and lower bound of the PMTU depending whether your packets made it through the network or not. This is described in RFC4821. If a probe packet makes it through, you raise the lower bound, if it gets lost you lower the upper bound until the upper/lower bound are close and you have your estimated PMTU, which is used to set upper size on your DTLS packets.


Claimed Justification of HB Having Payload Header, padding, having up to 2-byte size field

The heartbeats RFC RFC6520 says you can use Heartbeats for path MTU discovery for DTLS:

5.1. Path MTU Discovery

DTLS performs path MTU discovery as described in Section 4.1.1.1 of [RFC6347]. A detailed description of how to perform path MTU discovery is given in [RFC4821]. The necessary probe packets are the HeartbeatRequest messages.

DTLS applications do need to estimate PMTU. However, this is not done by DTLS, its done by the application using DTLS. Looking at the quoted section of the DTLS RFC Section 4.1.1.1 of RFC6347 it states "In general, DTLS's philosophy is to leave PMTU discovery to the application." It continues to give three caveats for why DTLS has to worry about PMTU (DTLS applications must subtract off the DTLS header to get effective PMTU size for data, DTLS may have to communicate ICMP "Datagram Too Big" back to the application layer, and DTLS handshakes should be smaller than PMTU. Earlier in the DTLS RFC it declares the DTLS record MUST fit in a single datagram smaller than the PMTU, so PMTU discovery/estimation must be done by the application using DTLS.

In PMTU discovery it makes sense to have a small field describing the length of the payload, have a large amount of arbitrary padding, and having something that echos back, yup got your request with this size MTU even though I'm only sending you back the sequence number (and for efficiency can drop the padding on the response). Granted it doesn't make sense if you describe the size of the payload to allow the payload to be bigger than about ~4-32 bytes, so payload size could be fixed or described by a one byte field, even if arbitrarily long padding could be concatenated.

Analysis of Claim

OpenSSL HB implementation and description in the HB RFC, doesn't describe or perform this PMTU discovery protocol. MTU is not present in the code. OpenSSL does provide only one mechanism to generate a HB request in TLS and DTLS, but it is of fixed size (18 byte payload, not configurable).

  1. There's no functionality to send a sequence of HBs to probe for PMTU discovery in the OpenSSL code, or detailed description of how HBs are used in a probing process,
  2. There's no indication that HBs are configured to not fragment (so they could even be used in this probing manner),
  3. If you wanted to use HB to do PMTU discovery, the application writer would have to write all the code themselves for client and server.
  4. The payload field even in the context of finding PMTU only needs to be one byte (even if its not fixed)
  5. There's no reason for them not to just do the entire probing process to use a HB packet versus any arbitrary type of packet in their client/server applications; e.g., using an ordinary UDP packet.
  6. PMTU discovery only makes sense in context of DTLS, so why are these completely unnecessary features present in TLS heartbeats -- applications that do not need to be PMTU aware?

At best this was a seriously flawed design (in TLS) incorporating a YAGNI features, that was then coded up badly to fully trust a user provided header field without any sanity testing. At worst, the PMTU sections were just a complicated cover story to allow insertion of vulnerable code that provides some semblance of justification.


Searching through the IETF TLS mailing list

If you search the IETF TLS mailing list, you can find interesting nuggets. Why is the payload/padding length uint16, and why is there padding if its to be discarded? PMTU discovery. The same asker (Juho Vähä-Herttua) states he would strongly prefer packet verification: read payload length, padding length, and verify that it matches record length (minus header). Also Simon Josefsson:

I have one mild concern with permitting arbitrary payload. What is the rationale for this? It opens up for a side channel in TLS. It could also be abused to send non-standardized data. Further, is there any reason to allow arbitrary sized payload? In my opinion, the payload_length, payload and padding fields seems unnecessary to me.

Michael Tüxen's response is largely inadequate (maybe some feature wants to be added on top to say calculate RTT) and summarizes with "The point here is that for interoperability, it does not matter what the payload is, it is only important that it is reflected."

Also of note reason for random padding "[we] randomize ... the data in the heartbeat message to attempt to head of any issues occurring from weak or flawed ciphers. " followed by question "Are there any papers or cipher documentation discussing how using randomized data in a packet would solve possible future cipher flaws?", followed by an paper on deterministic authenticated encryption. The response is great:

Indeed, but this is not a generic encryption mode like CBC or CTR. It is specifically designed to encrypt random keys, and thus depends on its randomness. Typical encryption modes are specifically designed to prevent someone distinguishing a given plaintext encryption from a random one.

Now, if one would like to use a subliminal channel in TLS, the heartbeat extension now provides an unbounded channel.

It's interesting that that comment was never addressed. (Other than someone else commenting " Well that's a whole different issue....").

dr jimbob
  • 38,768
  • 8
  • 92
  • 161
  • Yes, it's quite a mess. I wonder if you can think of a plausible justification for requiring the response to include padding? That was a change resulting from draft v4 (http://tools.ietf.org/rfcdiff?url2=draft-ietf-tls-dtls-heartbeat-04), which version I understand to have been the result of feedback from the IESG. A comparison to the case of the "Extended Random" TLS draft (https://projectbullrun.org/dual-ec/ext-rand.html) might be informative. – Peter Dettman Apr 14 '14 at 04:47
  • @PeterDettman - I added another long section with nuggets seen in the TLS IETF mailing list. It's interesting that many several people noted this potentially opens up side channel attacks. Adding randomized padding seems to be poorly justified based on an analysis of DAE and vague hope to prevent flaws of bad ciphers (meanwhile let's open this side channel). – dr jimbob Apr 14 '14 at 05:49
  • Indeed, I have made myself quite familiar with that list discussion over the last few days. A minor edit: "Why is the padding length uint16" should refer to payload_length? – Peter Dettman Apr 14 '14 at 07:01
  • @PeterDettman - At that point the standard said just dump the payload and padding -- didn't specify how the payload / padding was differentiated (presumably a header not deigned to mention at that point). Juho first said let's specify the header, (2) and was concerned why these fields are allowed to be so large (concerned about both types) and that if you allow 2^14 - 5= 16379 that requires a uint16 (two byte field). – dr jimbob Apr 14 '14 at 07:15
  • Ah, it was in the context of an earlier version, understood. – Peter Dettman Apr 14 '14 at 09:59
  • I'm still not seeing any reason given to remove "the padding field of the HeartbeatResponse message MUST have a length of zero" b/w draft version 3 and 4, nor indeed does the PMTU use-case require a mandatory minimum of 16 padding bytes across all use cases. Section 5 of draft 4 is amusing: "Each endpoint sends HeartbeatRequest messages at a rate and with the padding required for the particular use case". I guess they forgot to add: "unless your use case doesn't require any padding, in which case you have to add 16 bytes, which the receiver MUST ignore". – Peter Dettman Apr 14 '14 at 10:19
  • This is a very detailed description of the process, thank you for writing it. I would like to point out that what @drjimbob said about the payload / padding not being differentiated in the earlier versions of the draft is not quite correct. The notation field in TLS means that the data is prefixed by its own length, which means that there would be two length fields instead of the one that ended up in the final version. The suggestion was to reduce length fields to one to avoid size mismatches, and read from back to front to avoid reading past the buffer. – juhovh Sep 21 '19 at 13:46
  • @juhovh - Well as your user name appears to indicate you were the person asking the question I quoted in the list, you'd know. (Versus my cursory searches of an email list after the heartbleed discovery was announced). – dr jimbob Sep 21 '19 at 14:29
14

If you look into RFC6520 (heartbeat extension) there is a padding after the payload. So the length is required to know where the payload ends and the padding starts. Apart from that I find the design overengineered: the both reasons for this extension seem to be to make PMTU possible (by using messages of different size) and by having heartbeat to know if the other end is still alive or to reset some timeout counters in the states of middleboxes. There was no need to get a user-specified payload reflected in these scenarios.

And from a security perspective these kind of overengineered protocols, where you only test the stuff you really need, just beg for future trouble.

Steffen Ullrich
  • 184,332
  • 29
  • 363
  • 424
  • According to RFC6520 there is no way for the server to distinguish payload from padding, so if the server is required to send back only the payload it is _required_ to trust the input, which i think is a bad idea. Usually, paddings like PKCS1.5 or PKCS7 are such that the recipient can unambiguously tell the padding from the payload. BTW, what length are they padding _to_? – wallenborn Apr 14 '14 at 08:26
  • 1
    RFC6520, section 4 describes the structure. The field payload_length specifies only the length of the payload, so padding are just the remaining data until the end of the packet. The total length of the packet is defined by the TLS framing (e.g. on layer above). – Steffen Ullrich Apr 14 '14 at 09:55
  • Ah, i see, thanks. From a protocol design standpoint it might then have been a better idea to have a structure like: (type, length, fixed length nonce, payload, padding) where padding is a recognizable pattern. Then nobody has to actually make use of the length field, and PMTU diescovery is still possible. But i agree, the whole thing is just overengineered. – wallenborn Apr 14 '14 at 11:51