Why Application Security do matter. (or why you should care more)

tl;dr. Just a reminder that #Heartbleed is not a one-time event and that companies have much more bugs (partly written by themselves). They need to be aware that their developers don’t have the expertise to write safe code, and that they should invest in people who have the right skill-set – but also that at the end of the day we need better products and not software security solutions that just increase the attack surface.

In my previous article, I covered the vulnerable function to Heartbleed and its fix but also how confusing reading the code is.

After witnessing the panic and all the media attention (i.e. Colbert) this bug had been generating this week, it is finally obvious to the non-security community that software/application security is a key factor of the security of The Internet but also of our Critical Infrastructures. This isn’t the first critical bug we have seen, this one just happened to have much more media exposure than the previous ones – and it won’t be the last one either.

Software needs proper Quality Assurance and Risk Assessment. If a widely deployed open-source software like OpenSSL is showing such a poor code quality & security control due to its lack of resource, you can easily imagine how bad (i.e. worse!) it is with the rest of software used by critical infrastructures. Financial (Retail Banking, Investment Banking, Sovereign Wealth Funds, Hedge Funds, etc.), Industrial (Cars, Oil&Gas, etc.), Telecom (ISP, etc.) sectors, are all developing their internal software to address specific needs but in most of cases the resources allocated to Application Security are nonexistence or low and badly managed. And I’m not even mentioning the Enterprise softwares they are using who don’t benefit of a coverage like OpenSSL due to their restrictive licences.

What would be considered “correct” amount of resources ? Well, Risk management creates a value when its resources expended to mitigate risk should be less than the consequence of inaction. But since the consequence of inaction in the case of software control are infinite, a company can’t just wait for a regulation to be mandatory to take action.

But even if the resources are present, it often goes to inefficient security products that are just temporary mitigation and increasing the attack surface of the company instead of investing in building more robust products. But I understand how difficult it can be for a non-technical decision makers to identify what candidates/contractors/companies are qualified. Moreover, it is an industry where it is actually hard and rare to find skilled engineers (which is partly why Obama was recently promoting programming). This is mainly due to the fact that InfoSec is an industry of autodidacts experts. In addition to that, since InfoSec is becoming a “hype” industry, we are also witnessing growing number of “experts” with limited programming skills and who will summarize their exposure to Information Technology Security by mentioning the certifications they are holding (ed. superficial knowledge).

Books like Threat Modeling, exist for more than 10 years – it is only now that we start to see new ISO standards such as ISO 30111 Vulnerability handling processes (Kudos to @k8em0!) or ISO/IEC TS 17961:2013 (has an access fee). Which is one step further for the Enterprise World.

Having a CISO and following ISO 2700X are a good things, but it doesn’t necessary mean a company understands its problems and how to efficiently address them. But things are evolving and there are obviously a lot of progress made on “security awareness” compared to 10 years ago, but there are still a lot of things still need to change.

#Eyebleed. A technical analysis of the fix (not the bug!) for the Heartbleed issue

eyebleed
Heartbleed (CVE-2014-0160) is a bug found in OpenSSL due to a misimplementation of the Heartbeat functionality, and found by Neel Mehta (Google Security Team)

As a side note, this only affects OpenSSL which is NOT used by Microsoft ! See official statement here

A lot of people (Ted Angust, Existentialize, IOActive, etc.) already provided an analysis of the CVE-2014-0160 bug, widely known under the name “Heartbleed”, but nobody has analyzed the fix yet.

I had a look at both the code and the RFC, and it raised few questions to me.

1. The fix (and the original code)

We can see below a copy of the fixed-version of the dtls1_process_heartbeat() function.

  1. int
  2. dtls1_process_heartbeat(SSL *s)
  3.   {
  4.   unsigned char *p = &s->s3->rrec.data[0], *pl;
  5.   unsigned short hbtype;
  6.   unsigned int payload;
  7.   unsigned int padding = 16; /* Use minimum padding */
  8.  
  9.   if (s->msg_callback)
  10.     s->msg_callback(0, s->version, TLS1_RT_HEARTBEAT,
  11.       &s->s3->rrec.data[0], s->s3->rrec.length,
  12.       s, s->msg_callback_arg);
  13.  
  14.   /* Read type and payload length first */
  15.   if (1 + 2 + 16 > s->s3->rrec.length)
  16.     return 0; /* silently discard */
  17.   hbtype = *p++;
  18.   n2s(p, payload);
  19.   if (1 + 2 + payload + 16 > s->s3->rrec.length)
  20.     return 0; /* silently discard per RFC 6520 sec. 4 */
  21.   pl = p;
  22.  
  23.   if (hbtype == TLS1_HB_REQUEST)
  24.     {
  25.     unsigned char *buffer, *bp;
  26.     unsigned int write_length = 1 /* heartbeat type */ +
  27.               2 /* heartbeat length */ +
  28.               payload + padding;
  29.     int r;
  30.  
  31.     if (write_length > SSL3_RT_MAX_PLAIN_LENGTH)
  32.       return 0;
  33.  
  34.     /* Allocate memory for the response, size is 1 byte
  35.      * message type, plus 2 bytes payload length, plus
  36.      * payload, plus padding
  37.      */
  38.     buffer = OPENSSL_malloc(write_length);
  39.     bp = buffer;
  40.  
  41.     /* Enter response type, length and copy payload */
  42.     *bp++ = TLS1_HB_RESPONSE;
  43.     s2n(payload, bp);
  44.     memcpy(bp, pl, payload); < ==== The initial bug was here, because it was copying "too-much" data from the source buffer.
  45.     bp += payload;
  46.     /* Random padding */
  47.     RAND_pseudo_bytes(bp, padding);
  48.  
  49.     r = dtls1_write_bytes(s, TLS1_RT_HEARTBEAT, buffer, write_length);
  50.  
  51.     if (r >= 0 && s->msg_callback)
  52.       s->msg_callback(1, s->version, TLS1_RT_HEARTBEAT,
  53.         buffer, write_length,
  54.         s, s->msg_callback_arg);
  55.  
  56.     OPENSSL_free(buffer);
  57.  
  58.     if (r < 0)
  59.       return r;
  60.     }

1.1 Misleading comments & undefined constants

I found the function so hard to read that it looked like it was obfuscated JavaScript code, especially with all those undefined constants everywhere and misleading comments. For instance, what is described as “heartbeat length” is in fact the “payload length of the heartbeat message” but what intrigued me the most was the static constant for “padding length” (mentioned as “padding” grrr.)

1.2 Padding Length

After a look at the RFC 6520 “Transport Layer Security (TLS) and Datagram Transport Layer Security (DTLS) Heartbeat Extension”, I didn’t see any point mentioning that padding length was supposed to be a have static size (16) as done in the code. On the contrary, it is quite confusing to understand who is supposed to take care of it.

1.2.1 RFC 6520. Heartbeat messages

RFC 6520: Therefore, the padding_length is TLSPlaintext.length – payload_length – 3 for TLS and
DTLSPlaintext.length – payload_length – 3 for DTLS. The padding_length MUST be at least 16.
The sender of a HeartbeatMessage MUST use a random padding of at
least 16 bytes. The padding of a received HeartbeatMessage message
MUST be ignored.

The Heartbeat protocol messages consist of their type and an arbitrary payload and padding.

Read the full RFC 6520.

1.2.2 My understanding

My understanding is that there are two types of messages:

  • HeartbeatResponse
  • HeartbeatRequest

And that *both* have a padding, including the padding of a received HeartbeatMessage message MUST be ignored. But by that, I understand that the RFC it means the content and not the field itself.
This is my I concluded that OpenSSL should not be the one deciding of the size of the padding buffer, which therefore would have prevented to allocate a buffer and to use memcpy().

Below is my pseudo-code interpretation of the RFC 6520 and what the code should be:

  1. typedef struct heartbeat_msg_st
  2. {
  3.     /*
  4.         RFC 6520: The total length of a HeartbeatMessage MUST NOT exceed 2^14 or
  5.         max_fragment_length when negotiated as defined in [RFC6066].
  6.     */
  7.     char type;
  8.     unsigned short payload_length;
  9.     unsigned char payload[1];  /* Any size array. RFC 6520 only mentions "arbitrary content" which is (probably?) assumed to be non-empty. */
  10.     unsigned char padding[16]; /*  The padding_length MUST be at least 16. */
  11. } HEARTBEAT_MSG;
  12.  
  13. int
  14. dtls1_process_heartbeat(SSL *s)
  15. {
  16.     HEARTBEAT_MSG *hb_msg = &s->s3->rrec.data[0];
  17.     char hb_type;
  18.     unsigned short payload_length;
  19.     unsigned int padding_length;
  20.     unsigned int hb_msg_length = s->s3->rrec.length;
  21.  
  22.     if (s->msg_callback)
  23.         s->msg_callback(0, s->version, TLS1_RT_HEARTBEAT,
  24.             hb_msg, hb_msg_length,
  25.             s, s->msg_callback_arg);
  26.  
  27.     if (hb_msg_length < sizeof(HEARTBEAT_MSG)) return 0; // silently discard.
  28.     if (hb_msg_length < (hb_msg->payload_length + sizeof(payload_length) + sizeof(hb_type))) return 0; // silently discard.
  29.     padding_length = hb_msg_length - hb_msg->payload_length - sizeof(payload_length) - sizeof(hb_type);
  30.     if (padding_length < 16) return 0; // silently discard.
  31.  
  32.     hb_type = hb_msg->type;
  33.     payload_length = hb_msg->payload_length;
  34.  
  35.     if (hb_type == TLS1_HB_REQUEST)
  36.     {
  37.         unsigned char *padding = (unsigned char *)&hb_msg;
  38.         padding = &padding[hb_msg_length - padding_length];
  39.         if (hb_msg_length > SSL3_RT_MAX_PLAIN_LENGTH) return 0;
  40.  
  41.         hb_msg->type = TLS1_HB_RESPONSE;
  42.        
  43.         /* Random padding */
  44.         RAND_pseudo_bytes(padding, padding_length);
  45.  
  46.         r = dtls1_write_bytes(s, TLS1_RT_HEARTBEAT, hb_msg, hb_msg_length);
  47.  
  48.         if (r >= 0 && s->msg_callback)
  49.             s->msg_callback(1, s->version, TLS1_RT_HEARTBEAT,
  50.             hb_msg, hb_msg_length,
  51.             s, s->msg_callback_arg);
  52.  
  53.         if (r < 0) return r;
  54.     }

2. Any thoughts ?

Reading a RFC can be hard, including widely deployed code such as OpenSSL. I'd be curious to hear (@msuiche) your interpretation of the RFC.

Faces of Yemen

Access to the full album here, and the second album (B&W) here.

I recently had the chance to travel across Yemen, which is a not well known but beautiful country with a unique ecosystem.

Even through the country is listed as high security level threat, the areas I’ve been visited were relatively safe and I didn’t experience any troubles during my trip. From what I can say, Yemenis are part of the most welcoming, and friendly people I had the chance to meet.

As you can see, people are full of life and their faces tell lots of stories. That’s why I called the Album “Faces of Yemen“.

Sana’a


Great Mosque of Sana’a


Happy kids from Kawkaban


Sunset on Sana’a


Confidences over Sana’a

Socotra Island

Socotra Island, is an Island between Yemen and Somalia. Mainly known for her dragonblood trees which are a unique specie that can only be found on the Island. There are several legends about its origins but my favorite is this one:
In the Middle Ages, an European King was looking for a potion to cure his ill unique daughter. He offered her hand to anyone who would bring her the medicine. One of the candidates, mentioned he knew about an Island with a special remedy that could save his daughter. The King supported the Chevalier who went to Socotra to find the medicine. Once the Chevalier arrived on the Island, he had to fight with a Giant bird/dragon that he managed to severely injure during his battle. Then the bird/dragon tried to escape and flew all over the Island, and for each drop of its blood, a Dragonblood tree grew out of it.

Socotra Island as several protected area established by the Yemeni Environment Protection Authority (EPA), and counts several NGO such as the Triangle or the Yemeni Biological Society founded and presided by Dr. Abdul Karim Nasher, a well known and respect Yemeni scientist, who I had the chance to meet and who is involved with multiple projects such as the protection of sea turtles across the country. I recommend you to watch his talk from TEDxSanaa: Yemen is richer with its plants to know more about the unique ecosystem of Yemen.
I look at Zoology/Biology as a kind of reverse engineering study applied to life and living organisms, which is the reason why I find it so fascinating.


Dragonblood tree


Kid on the beach


Happy girl


Fisherman


Bottletree next to a lagoon


Altitude


Goatkeeper