fix: handle record padding

Co-authored-by: René Meusel <rene.meusel@nexenio.com>

src/bogo_shim/bogo_shim.cpp

@@ -141,6 +141,7 @@ std::string map_to_bogo_error(const std::string& e)
  { "Invalid authentication tag: ChaCha20Poly1305 tag check failed", ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:" },
  { "Invalid authentication tag: GCM tag check failed", ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:" },
  { "Message authentication failure", ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:" },
+ { "No content type found in encrypted record", ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:" },
  { "No shared DTLS version", ":UNSUPPORTED_PROTOCOL:" },
  { "No shared TLS version", ":UNSUPPORTED_PROTOCOL:" },
  { "OS2ECP: Unknown format type 251", ":BAD_ECPOINT:" },

src/lib/tls/tls13/tls_channel_impl_13.cpp

@@ -323,10 +323,7 @@ void Channel_Impl_13::process_alert(const secure_vector<uint8_t>& record)
  m_record_layer.clear_read_buffer();
  }
 
- if(is_user_canceled_alert(alert))
- {
- // ignore
- }
+ // user canceled alerts are ignored
 
  if(is_error_alert(alert))
  { shutdown(); }

src/lib/tls/tls13/tls_record_layer_13.cpp

@@ -284,8 +284,17 @@ Record_Layer::ReadResult<Record> Record_Layer::next_record(Cipher_State* cipher_
 
  record.seq_no = cipher_state->decrypt_record_fragment(plaintext_header.serialized, record.fragment);
 
+ // Remove record padding (RFC 8446 5.4).
+ const auto end_of_content = std::find_if(record.fragment.crbegin(), record.fragment.crend(), [](auto byte)
+ {
+ return byte != 0x00;
+ });
+
+ if(end_of_content == record.fragment.crend())
+ { throw TLS_Exception(Alert::DECODE_ERROR, "No content type found in encrypted record"); }
+
  // hydrate the actual content type from TLSInnerPlaintext
- record.type = read_record_type(record.fragment.back());
+ record.type = read_record_type(*end_of_content);
 
  if(record.type == Record_Type::CHANGE_CIPHER_SPEC)
  {
@@ -294,7 +303,9 @@ Record_Layer::ReadResult<Record> Record_Layer::next_record(Cipher_State* cipher_
  // abort the handshake with an "unexpected_message" alert.
  throw TLS_Exception(Alert::UNEXPECTED_MESSAGE, "protected change cipher spec received");
  }
- record.fragment.pop_back();
+
+ // erase content type and padding
+ record.fragment.erase((end_of_content+1).base(), record.fragment.cend());
  }
 
  m_initial_record = false;

src/tests/test_tls_record_layer_13.cpp

@@ -488,6 +488,38 @@ read_encrypted_records()
  "93 98 28 fd 4a e3 79 4e 44 f9 4d f5 a6 31 ed e4 2c 17 19 bf da"
  "bf 02 53 fe 51 75 be 89 8e 75 0e dc 53 37 0d 2b");
 
+ // the record above padded with 42 zeros
+ const auto encrypted_record_with_padding = Botan::hex_decode(
+ "17 03 03 02 cc d1 ff 33 4a 56 f5 bf f6 59 4a 07 cc 87 b5 80 23 3f 50 0f 45"
+ "e4 89 e7 f3 3a f3 5e df 78 69 fc f4 0a a4 0a a2 b8 ea 73 f8 48 a7 ca 07 61"
+ "2e f9 f9 45 cb 96 0b 40 68 90 51 23 ea 78 b1 11 b4 29 ba 91 91 cd 05 d2 a3"
+ "89 28 0f 52 61 34 aa dc 7f c7 8c 4b 72 9d f8 28 b5 ec f7 b1 3b d9 ae fb 0e"
+ "57 f2 71 58 5b 8e a9 bb 35 5c 7c 79 02 07 16 cf b9 b1 18 3e f3 ab 20 e3 7d"
+ "57 a6 b9 d7 47 76 09 ae e6 e1 22 a4 cf 51 42 73 25 25 0c 7d 0e 50 92 89 44"
+ "4c 9b 3a 64 8f 1d 71 03 5d 2e d6 5b 0e 3c dd 0c ba e8 bf 2d 0b 22 78 12 cb"
+ "b3 60 98 72 55 cc 74 41 10 c4 53 ba a4 fc d6 10 92 8d 80 98 10 e4 b7 ed 1a"
+ "8f d9 91 f0 6a a6 24 82 04 79 7e 36 a6 a7 3b 70 a2 55 9c 09 ea d6 86 94 5b"
+ "a2 46 ab 66 e5 ed d8 04 4b 4c 6d e3 fc f2 a8 94 41 ac 66 27 2f d8 fb 33 0e"
+ "f8 19 05 79 b3 68 45 96 c9 60 bd 59 6e ea 52 0a 56 a8 d6 50 f5 63 aa d2 74"
+ "09 96 0d ca 63 d3 e6 88 61 1e a5 e2 2f 44 15 cf 95 38 d5 1a 20 0c 27 03 42"
+ "72 96 8a 26 4e d6 54 0c 84 83 8d 89 f7 2c 24 46 1a ad 6d 26 f5 9e ca ba 9a"
+ "cb bb 31 7b 66 d9 02 f4 f2 92 a3 6a c1 b6 39 c6 37 ce 34 31 17 b6 59 62 22"
+ "45 31 7b 49 ee da 0c 62 58 f1 00 d7 d9 61 ff b1 38 64 7e 92 ea 33 0f ae ea"
+ "6d fa 31 c7 a8 4d c3 bd 7e 1b 7a 6c 71 78 af 36 87 90 18 e3 f2 52 10 7f 24"
+ "3d 24 3d c7 33 9d 56 84 c8 b0 37 8b f3 02 44 da 8c 87 c8 43 f5 e5 6e b4 c5"
+ "e8 28 0a 2b 48 05 2c f9 3b 16 49 9a 66 db 7c ca 71 e4 59 94 26 f7 d4 61 e6"
+ "6f 99 88 2b d8 9f c5 08 00 be cc a6 2d 6c 74 11 6d bd 29 72 fd a1 fa 80 f8"
+ "5d f8 81 ed be 5a 37 66 89 36 b3 35 58 3b 59 91 86 dc 5c 69 18 a3 96 fa 48"
+ "a1 81 d6 b6 fa 4f 9d 62 d5 13 af bb 99 2f 2b 99 2f 67 f8 af e6 7f 76 91 3f"
+ "a3 88 cb 56 30 c8 ca 01 e0 c6 5d 11 c6 6a 1e 2a c4 c8 59 77 b7 c7 a6 99 9b"
+ "bf 10 dc 35 ae 69 f5 51 56 14 63 6c 0b 9b 68 c1 9e d2 e3 1c 0b 3b 66 76 30"
+ "38 eb ba 42 f3 b3 8e dc 03 99 f3 a9 f2 3f aa 63 97 8c 31 7f c9 fa 66 a7 3f"
+ "60 f0 50 4d e9 3b 5b 84 5e 27 55 92 c1 23 35 ee 34 0b bc 4f dd d5 02 78 40"
+ "16 e4 b3 be 7e f0 4d da 49 f4 b4 40 a3 0c b5 d2 af 93 98 28 fd 4a e3 79 4e"
+ "44 f9 4d f5 a6 31 ed e4 2c 17 19 bf da 04 d8 68 77 bb e0 dc ce f9 01 ed 32"
+ "59 50 7a 0c d0 62 3f 90 1b 5c 89 d4 b4 f2 d1 56 f6 da 4f 3e c5 fd 2d e5 e2"
+ "fa 44 23 0a e0 c9 dd dd bb a8 be db d9 d7 f6 b8 3d 56 4c a5 47");
+
  auto parse_records = [](const std::vector<uint8_t>& data)
  {
  auto rl = record_layer_client(true);
@@ -621,6 +653,26 @@ read_encrypted_records()
  const auto enc_exts = client.next_record(cs.get());
  result.confirm("read a record", std::holds_alternative<TLS::Record>(enc_exts));
  result.confirm("is handshake record", std::get<TLS::Record>(enc_exts).type == TLS::HANDSHAKE);
+ }),
+
+ CHECK("read a padded record", [&](Test::Result& result)
+ {
+ auto client = record_layer_client(true);
+ client.copy_data(encrypted_record_with_padding);
+
+ auto cs = rfc8448_rtt1_handshake_traffic();
+ const auto record = client.next_record(cs.get());
+ result.confirm("read a record with padding", std::holds_alternative<TLS::Record>(record));
+ }),
+
+ CHECK("read an empty encrypted record", [&](Test::Result& result)
+ {
+ auto client = record_layer_client(true);
+ client.copy_data(Botan::hex_decode("1703030011CE43CA0D2F28336715E770071B2D5EE0FE"));
+
+ auto cs = rfc8448_rtt1_handshake_traffic();
+ const auto record = client.next_record(cs.get());
+ result.confirm("read an empty record", std::holds_alternative<TLS::Record>(record));
  })
  };
  }