Repository: randombit/botan
Pull request #2225 (2019-12-10 16:47:14)

New TLS session encryption format
Changes:
- Adds magic number/versioning to make future extensions possible
- Adds key identifier to avoid needless decryption attempts, makes supporting ticket key rotation easier in the future
- Avoids key collision; in current format if the seed is duplicated the same key + nonce are generated. This does not leak the master ticket key but is still bad. Now nonce is random, and key is generated via a distinct 128-bit long input. Chances of a duplicated key/nonce are now about 2^-112.
- Include the whole header incl nonce as associated data
- Use SHA-512-256 instead of SHA-256

This breaks all saved encrypted sessions as well as saved session tickets. But the cost then is just a full renegotiation. The session ticket format is not guaranteed to be stable even across minor releases.

codecov-io: # [Codecov](https://codecov.io/gh/randombit/botan/pull/2225?src=pr&el=h1) Report
> Merging [#2225](https://codecov.io/gh/randombit/botan/pull/2225?src=pr&el=desc) into [master](https://codecov.io/gh/randombit/botan/commit/3ecca9aa149f35048ece9a0d8f5c16f8a5a17c2b?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `96.07%`.

[![Impacted file tree graph](https://codecov.io/gh/randombit/botan/pull/2225/graphs/tree.svg?width=650&token=VR6iYmSPH8&height=150&src=pr)](https://codecov.io/gh/randombit/botan/pull/2225?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #2225      +/-   ##
==========================================
+ Coverage   92.48%   92.48%   +<.01%     
==========================================
  Files         555      555              
  Lines       60439    60470      +31     
  Branches     6400     6402       +2     
==========================================
+ Hits        55895    55926      +31     
  Misses       4544     4544
```


| [Impacted Files](https://codecov.io/gh/randombit/botan/pull/2225?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/lib/tls/tls\_session.cpp](https://codecov.io/gh/randombit/botan/pull/2225/diff?src=pr&el=tree#diff-c3JjL2xpYi90bHMvdGxzX3Nlc3Npb24uY3Bw) | `92.56% <96.07%> (+0.25%)` | :arrow_up: |
| [src/lib/pubkey/elgamal/elgamal.cpp](https://codecov.io/gh/randombit/botan/pull/2225/diff?src=pr&el=tree#diff-c3JjL2xpYi9wdWJrZXkvZWxnYW1hbC9lbGdhbWFsLmNwcA==) | `95.94% <0%> (-1.36%)` | :arrow_down: |
| [src/lib/pubkey/mce/mceliece\_key.cpp](https://codecov.io/gh/randombit/botan/pull/2225/diff?src=pr&el=tree#diff-c3JjL2xpYi9wdWJrZXkvbWNlL21jZWxpZWNlX2tleS5jcHA=) | `85.39% <0%> (-1.13%)` | :arrow_down: |
| [src/bogo\_shim/bogo\_shim.cpp](https://codecov.io/gh/randombit/botan/pull/2225/diff?src=pr&el=tree#diff-c3JjL2JvZ29fc2hpbS9ib2dvX3NoaW0uY3Bw) | `89.12% <0%> (+0.17%)` | :arrow_up: |
| [src/lib/asn1/der\_enc.cpp](https://codecov.io/gh/randombit/botan/pull/2225/diff?src=pr&el=tree#diff-c3JjL2xpYi9hc24xL2Rlcl9lbmMuY3Bw) | `82.6% <0%> (+2.48%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/randombit/botan/pull/2225?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/randombit/botan/pull/2225?src=pr&el=footer). Last update [3ecca9a...7ac66f2](https://codecov.io/gh/randombit/botan/pull/2225?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/doc/api_ref/tls.rst b/doc/api_ref/tls.rst
index 1980e094b..8569a89c1 100644
--- a/doc/api_ref/tls.rst
+++ b/doc/api_ref/tls.rst
@@ -669,8 +669,8 @@ information about that session:
 .. note::
 
    The serialization format of Session is not considered stable and is allowed
-   to change even within a single major release cycle. In the event of such a
-   change, old sessions will no longer be able to be resumed.
+   to change even across minor releases. In the event of such a change, old
+   sessions will no longer be able to be resumed.
 
 .. cpp:class:: TLS::Session
 
@@ -1889,27 +1889,27 @@ A unified format is used for encrypting TLS sessions either for durable storage
 (on client or server) or when creating TLS session tickets. This format is *not
 stable* even across the same major version.
 
-The current session encryption scheme was introduced in 1.11.13.
+The current session encryption scheme was introduced in 2.13.0, replacing the
+format previously used since 1.11.13.
 
 Session encryption accepts a key of any length, though for best security a key
 of 256 bits should be used. This master key is used to key an instance of HMAC
-using the SHA-256 hash.
+using the SHA-512/256 hash.
 
-A random 96 bit nonce is created and included in the header. Then the nonce is
-hashed using the keyed HMAC to produce a 256-bit GCM key. This key and nonce
-pair are used to encrypt the session.
+First a "key name" or identifier is created, by HMAC'ing the fixed string "BOTAN
+TLS SESSION KEY NAME" and truncating to 4 bytes. This is the initial prefix of
+the encrypted session, and will remain fixed as long as the same ticket key is
+used. This allows quickly rejecting sessions which are encrypted using an
+unknown or incorrect key.
 
-The current design is unfortunate in several ways:
+Then a key used for AES-256 in GCM mode is created by first choosing a 128 bit
+random seed, and HMAC'ing it to produce a 256-bit value. This means for any one
+master key as many as 2\ :sup:`128` GCM keys can be created. This is done
+because NIST recommends that when using random nonces no one GCM key be used to
+encrypt more than 2\ :sup:`32` messages (to avoid the possiblity of nonce
+reuse).
 
- * If the random 96-bit nonce ever repeats, then the same GCM key *and nonce*
-   are used to encrypt 2 different sessions, breaking the security of sessions
-   encrypted using that duplicated key/nonce. The GCM nonce should have been
-   independently created and included in the header, making a key collision
-   harmless.
-
- * There is no simple way to detect old or rotated keys, except by attempting
-   decryption. Instead a key identifier should be included in the header.
-
- * There is no indicator of the format used, which makes handling format
-   upgrades without breaking compatability awkward.
+A random 96-bit nonce is created and included in the header.
 
+AES in GCM is used to encrypt and authenticate the serialized session. The
+key name, key seed, and AEAD nonce are all included as additional data.
diff --git a/src/lib/tls/info.txt b/src/lib/tls/info.txt
index d81cbb997..690bd4150 100644
--- a/src/lib/tls/info.txt
+++ b/src/lib/tls/info.txt
@@ -1,5 +1,5 @@
 <defines>
-TLS -> 20150319
+TLS -> 20191210
 </defines>
 
 <header:public>
diff --git a/src/lib/tls/tls_session.cpp b/src/lib/tls/tls_session.cpp
index 0f603454b..7d1bd7200 100644
--- a/src/lib/tls/tls_session.cpp
+++ b/src/lib/tls/tls_session.cpp
@@ -1,11 +1,12 @@
 /*
 * TLS Session State
-* (C) 2011-2012,2015 Jack Lloyd
+* (C) 2011-2012,2015,2019 Jack Lloyd
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
 
 #include <botan/tls_session.h>
+#include <botan/loadstor.h>
 #include <botan/der_enc.h>
 #include <botan/ber_dec.h>
 #include <botan/asn1_str.h>
@@ -176,48 +177,114 @@ std::chrono::seconds Session::session_age() const
       std::chrono::system_clock::now() - m_start_time);
    }
 
+namespace {
+
+// The output length of the HMAC must be a valid keylength for the AEAD
+const char* TLS_SESSION_CRYPT_HMAC = "HMAC(SHA-512-256)";
+// SIV would be better, but we can't assume it is available
+const char* TLS_SESSION_CRYPT_AEAD = "AES-256/GCM";
+const char* TLS_SESSION_CRYPT_KEY_NAME = "BOTAN TLS SESSION KEY NAME";
+const uint64_t TLS_SESSION_CRYPT_MAGIC = 0x068B5A9D396C0000;
+const size_t TLS_SESSION_CRYPT_MAGIC_LEN = 8;
+const size_t TLS_SESSION_CRYPT_KEY_NAME_LEN = 4;
+const size_t TLS_SESSION_CRYPT_AEAD_NONCE_LEN = 12;
+const size_t TLS_SESSION_CRYPT_AEAD_KEY_SEED_LEN = 16;
+const size_t TLS_SESSION_CRYPT_AEAD_TAG_SIZE = 16;
+
+const size_t TLS_SESSION_CRYPT_HDR_LEN =
+   TLS_SESSION_CRYPT_MAGIC_LEN +
+   TLS_SESSION_CRYPT_KEY_NAME_LEN +
+   TLS_SESSION_CRYPT_AEAD_NONCE_LEN +
+   TLS_SESSION_CRYPT_AEAD_KEY_SEED_LEN;
+
+const size_t TLS_SESSION_CRYPT_OVERHEAD =
+   TLS_SESSION_CRYPT_HDR_LEN + TLS_SESSION_CRYPT_AEAD_TAG_SIZE;
+
+}
+
 std::vector<uint8_t>
 Session::encrypt(const SymmetricKey& key, RandomNumberGenerator& rng) const
    {
-   std::unique_ptr<AEAD_Mode> aead = AEAD_Mode::create_or_throw("AES-256/GCM", ENCRYPTION);
-   const size_t nonce_len = aead->default_nonce_length();
-
-   const secure_vector<uint8_t> nonce = rng.random_vec(nonce_len);
-   const secure_vector<uint8_t> bits = this->DER_encode();
-
-   // Support any length key for input
-   std::unique_ptr<MessageAuthenticationCode> hmac(MessageAuthenticationCode::create("HMAC(SHA-256)"));
+   auto hmac = MessageAuthenticationCode::create_or_throw(TLS_SESSION_CRYPT_HMAC);
    hmac->set_key(key);
-   hmac->update(nonce);
-   aead->set_key(hmac->final());
 
-   secure_vector<uint8_t> buf = nonce;
+   // First derive the "key name"
+   std::vector<uint8_t> key_name(hmac->output_length());
+   hmac->update(TLS_SESSION_CRYPT_KEY_NAME);
+   hmac->final(key_name.data());
+   key_name.resize(TLS_SESSION_CRYPT_KEY_NAME_LEN);
+
+   std::vector<uint8_t> aead_nonce;
+   std::vector<uint8_t> key_seed;
+
+   rng.random_vec(aead_nonce, TLS_SESSION_CRYPT_AEAD_NONCE_LEN);
+   rng.random_vec(key_seed, TLS_SESSION_CRYPT_AEAD_KEY_SEED_LEN);
+
+   hmac->update(key_seed);
+   const secure_vector<uint8_t> aead_key = hmac->final();
+
+   secure_vector<uint8_t> bits = this->DER_encode();
+
+   // create the header
+   std::vector<uint8_t> buf;
+   buf.reserve(TLS_SESSION_CRYPT_OVERHEAD + bits.size());
+   buf.resize(TLS_SESSION_CRYPT_MAGIC_LEN);
+   store_be(TLS_SESSION_CRYPT_MAGIC, &buf[0]);
+   buf += key_name;
+   buf += key_seed;
+   buf += aead_nonce;
+
+   std::unique_ptr<AEAD_Mode> aead = AEAD_Mode::create_or_throw(TLS_SESSION_CRYPT_AEAD, ENCRYPTION);
+   BOTAN_ASSERT_NOMSG(aead->valid_nonce_length(TLS_SESSION_CRYPT_AEAD_NONCE_LEN));
+   BOTAN_ASSERT_NOMSG(aead->tag_size() == TLS_SESSION_CRYPT_AEAD_TAG_SIZE);
+   aead->set_key(aead_key);
+   aead->set_associated_data_vec(buf);
+   aead->start(aead_nonce);
+   aead->finish(bits, 0);
+
+   // append the ciphertext
    buf += bits;
-   aead->start(buf.data(), nonce_len);
-   aead->finish(buf, nonce_len);
-   return unlock(buf);
+   return buf;
    }
 
 Session Session::decrypt(const uint8_t in[], size_t in_len, const SymmetricKey& key)
    {
    try
       {
-      std::unique_ptr<AEAD_Mode> aead = AEAD_Mode::create_or_throw("AES-256/GCM", DECRYPTION);
-      const size_t nonce_len = aead->default_nonce_length();
-
-      if(in_len < nonce_len + aead->tag_size())
+      const size_t min_session_size = 48 + 4; // serious under-estimate
+      if(in_len < TLS_SESSION_CRYPT_OVERHEAD + min_session_size)
          throw Decoding_Error("Encrypted session too short to be valid");
 
-      // Support any length key for input
-      std::unique_ptr<MessageAuthenticationCode> hmac(MessageAuthenticationCode::create("HMAC(SHA-256)"));
+      const uint8_t* magic = &in[0];
+      const uint8_t* key_name = magic + TLS_SESSION_CRYPT_MAGIC_LEN;
+      const uint8_t* key_seed = key_name + TLS_SESSION_CRYPT_KEY_NAME_LEN;
+      const uint8_t* aead_nonce = key_seed + TLS_SESSION_CRYPT_AEAD_KEY_SEED_LEN;
+      const uint8_t* ctext = aead_nonce + TLS_SESSION_CRYPT_AEAD_NONCE_LEN;
+      const size_t ctext_len = in_len - TLS_SESSION_CRYPT_HDR_LEN; // includes the tag
+
+      if(load_be<uint64_t>(magic, 0) != TLS_SESSION_CRYPT_MAGIC)
+         throw Decoding_Error("Missing expected magic numbers");
+
+      auto hmac = MessageAuthenticationCode::create_or_throw(TLS_SESSION_CRYPT_HMAC);
       hmac->set_key(key);
-      hmac->update(in, nonce_len); // nonce bytes
-      aead->set_key(hmac->final());
 
-      aead->start(in, nonce_len);
-      secure_vector<uint8_t> buf(in + nonce_len, in + in_len);
-      aead->finish(buf, 0);
+      // First derive and check the "key name"
+      std::vector<uint8_t> cmp_key_name(hmac->output_length());
+      hmac->update(TLS_SESSION_CRYPT_KEY_NAME);
+      hmac->final(cmp_key_name.data());
 
+      if(same_mem(cmp_key_name.data(), key_name, TLS_SESSION_CRYPT_KEY_NAME_LEN) == false)
+         throw Decoding_Error("Wrong key name for encrypted session");
+
+      hmac->update(key_seed, TLS_SESSION_CRYPT_AEAD_KEY_SEED_LEN);
+      const secure_vector<uint8_t> aead_key = hmac->final();
+
+      auto aead = AEAD_Mode::create_or_throw(TLS_SESSION_CRYPT_AEAD, DECRYPTION);
+      aead->set_key(aead_key);
+      aead->set_associated_data(in, TLS_SESSION_CRYPT_HDR_LEN);
+      aead->start(aead_nonce, TLS_SESSION_CRYPT_AEAD_NONCE_LEN);
+      secure_vector<uint8_t> buf(ctext, ctext + ctext_len);
+      aead->finish(buf, 0);
       return Session(buf.data(), buf.size());
       }
    catch(std::exception& e)
diff --git a/src/scripts/ci/setup_travis.sh b/src/scripts/ci/setup_travis.sh
index 0583f65b6..fc11067ee 100755
--- a/src/scripts/ci/setup_travis.sh
+++ b/src/scripts/ci/setup_travis.sh
@@ -11,12 +11,11 @@ set -ev
 
 if [ "$TRAVIS_OS_NAME" = "linux" ]; then
 
-    if [ "$TRAVIS_ARCH" = "aarch64" ]; then
+    if [ "$TRAVIS_ARCH" = "aarch64" ] || [ "$TRAVIS_ARCH" = "ppc64le" ] || [ "$TRAVIS_ARCH" = "s390x" ]; then
         sudo apt-get -qq update
         sudo apt-get install liblzma-dev libbz2-dev ccache
-    fi
 
-    if [ "$TARGET" = "valgrind" ]; then
+    elif [ "$TARGET" = "valgrind" ]; then
         sudo apt-get -qq update
         sudo apt-get install valgrind
 


Test patch:
diff --git a/src/tests/data/mac/hmac.vec b/src/tests/data/mac/hmac.vec
index 4f07c0894..d01972f69 100644
--- a/src/tests/data/mac/hmac.vec
+++ b/src/tests/data/mac/hmac.vec
@@ -171,6 +171,10 @@ Key = 4A656665
 In = 7768617420646F2079612077616E7420666F72206E6F7468696E673F
 Out = 6DF7B24630D5CCB2EE335407081A87188C221489768FA2020513B2D593359456
 
+Key = ABCDEF
+In = 424F54414E20544C532053455353494F4E204B4559204E414D45
+Out = F2322CAEB257314E587F78A59F41D237D2EDFE3356C3D9643C520A1286BA7A4B
+
 # HMAC SHA-3 tests from http://www.wolfgang-ehrhardt.de/hmac-sha3-testvectors.html
 
 [HMAC(SHA-3(224))]
diff --git a/src/tests/test_tls.cpp b/src/tests/test_tls.cpp
index 0798fc120..34991d08b 100644
--- a/src/tests/test_tls.cpp
+++ b/src/tests/test_tls.cpp
@@ -9,6 +9,7 @@
 #include <memory>
 
 #if defined(BOTAN_HAS_TLS)
+  #include "test_rng.h"
   #include <botan/tls_alert.h>
   #include <botan/tls_policy.h>
   #include <botan/tls_session.h>
@@ -52,18 +53,35 @@ class TLS_Session_Tests final : public Test
                                      "SRP username",
                                      0x0000);
 
+         const std::string pem = session.PEM_encode();
+         Botan::TLS::Session session_from_pem(pem);
+         result.test_eq("Roundtrip from pem", session.DER_encode(), session_from_pem.DER_encode());
+
          const Botan::SymmetricKey key("ABCDEF");
-         std::vector<uint8_t> ctext1 = session.encrypt(key, Test::rng());
-         std::vector<uint8_t> ctext2 = session.encrypt(key, Test::rng());
+         const std::vector<uint8_t> ctext1 = session.encrypt(key, Test::rng());
+         const std::vector<uint8_t> ctext2 = session.encrypt(key, Test::rng());
 
          result.test_ne("TLS session encryption is non-determinsitic",
                         ctext1.data(), ctext1.size(),
                         ctext2.data(), ctext2.size());
 
-         Botan::TLS::Session dsession = Botan::TLS::Session::decrypt(ctext1.data(), ctext1.size(), key);
+         const std::vector<uint8_t> expected_hdr = Botan::hex_decode("068B5A9D396C0000F2322CAE");
+
+         result.test_eq("tls", "TLS session encryption same header",
+                        ctext1.data(), 12, expected_hdr.data(), 12);
+         result.test_eq("tls", "TLS session encryption same header",
+                        ctext2.data(), 12, expected_hdr.data(), 12);
 
+         Botan::TLS::Session dsession = Botan::TLS::Session::decrypt(ctext1.data(), ctext1.size(), key);
          result.test_eq("Decrypted session access works", dsession.srp_identifier(), "SRP username");
 
+         Fixed_Output_RNG frng1("00112233445566778899AABBCCDDEEFF802802802802802802802802");
+         const std::vector<uint8_t> ctextf1 = session.encrypt(key, frng1);
+         Fixed_Output_RNG frng2("00112233445566778899AABBCCDDEEFF802802802802802802802802");
+         const std::vector<uint8_t> ctextf2 = session.encrypt(key, frng2);
+
+         result.test_eq("Only randomness comes from RNG", ctextf1, ctextf2);
+
          return {result};
          }
    };