Better support legacy DES customers (#1671)

### Issues:
Addresses CryptoAlg-2421

### Description of changes: 
No one should start using these DES functions, or continue using DES in
general. However, for legacy customers that can't change this PR adds a
few small utility functions and aligns AWS-LC with the behavior they
expect from OpenSSL.

### Call-outs:
I updated DES_set_key to perform the same checks as OpenSSL and updated
internal usages to use DES_set_key_unchecked.

### Testing:
Added new tests and ensured existing tests still pass. 

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.

crypto/CMakeLists.txt

@@ -721,6 +721,7 @@ if(BUILD_TESTING)
     digest_extra/digest_test.cc
     dilithium/p_dilithium_test.cc
     dsa/dsa_test.cc
+    des/des_test.cc
     endian_test.cc
     err/err_test.cc
     evp_extra/evp_extra_test.cc

crypto/cipher_extra/e_des.c

@@ -74,7 +74,7 @@ static int des_init_key(EVP_CIPHER_CTX *ctx, const uint8_t *key,
   DES_cblock *deskey = (DES_cblock *)key;
   EVP_DES_KEY *dat = (EVP_DES_KEY *)ctx->cipher_data;
 
-  DES_set_key(deskey, &dat->ks.ks);
+  DES_set_key_unchecked(deskey, &dat->ks.ks);
   return 1;
 }
 
@@ -147,9 +147,9 @@ static int des_ede3_init_key(EVP_CIPHER_CTX *ctx, const uint8_t *key,
   DES_cblock *deskey = (DES_cblock *)key;
   DES_EDE_KEY *dat = (DES_EDE_KEY *)ctx->cipher_data;
 
-  DES_set_key(&deskey[0], &dat->ks.ks[0]);
-  DES_set_key(&deskey[1], &dat->ks.ks[1]);
-  DES_set_key(&deskey[2], &dat->ks.ks[2]);
+  DES_set_key_unchecked(&deskey[0], &dat->ks.ks[0]);
+  DES_set_key_unchecked(&deskey[1], &dat->ks.ks[1]);
+  DES_set_key_unchecked(&deskey[2], &dat->ks.ks[2]);
 
   return 1;
 }
@@ -185,9 +185,9 @@ static int des_ede_init_key(EVP_CIPHER_CTX *ctx, const uint8_t *key,
   DES_cblock *deskey = (DES_cblock *)key;
   DES_EDE_KEY *dat = (DES_EDE_KEY *)ctx->cipher_data;
 
-  DES_set_key(&deskey[0], &dat->ks.ks[0]);
-  DES_set_key(&deskey[1], &dat->ks.ks[1]);
-  DES_set_key(&deskey[0], &dat->ks.ks[2]);
+  DES_set_key_unchecked(&deskey[0], &dat->ks.ks[0]);
+  DES_set_key_unchecked(&deskey[1], &dat->ks.ks[1]);
+  DES_set_key_unchecked(&deskey[0], &dat->ks.ks[2]);
 
   return 1;
 }

crypto/des/des.c

@@ -293,7 +293,7 @@ static const uint32_t DES_SPtrans[8][64] = {
   ((t) = ((((a) << (16 - (n))) ^ (a)) & (m)), \
    (a) = (a) ^ (t) ^ ((t) >> (16 - (n))))
 
-void DES_set_key(const DES_cblock *key, DES_key_schedule *schedule) {
+void DES_set_key_unchecked(const DES_cblock *key, DES_key_schedule *schedule) {
   static const int shifts2[16] = {0, 0, 1, 1, 1, 1, 1, 1,
                                   0, 1, 1, 1, 1, 1, 1, 0};
   uint32_t c, d, t, s, t2;
@@ -349,9 +349,38 @@ void DES_set_key(const DES_cblock *key, DES_key_schedule *schedule) {
   }
 }
 
+// SP 800-67r2 section 2, the last bit of each byte in DES_cblock.bytes is used
+// for parity. The parity bits should be set to the complement of the modulo 2
+// sum of the previous seven bits
+static int DES_check_key_parity(const DES_cblock *key) {
+  uint8_t result = UINT8_MAX;
+
+  for (size_t i = 0; i < DES_KEY_SZ; i++) {
+    uint8_t b = key->bytes[i];
+    b ^= b >> 4;
+    b ^= b >> 2;
+    b ^= b >> 1;
+    result &= constant_time_eq_8(b & 1, 1);
+  }
+  return result & 1;
+}
+
+int DES_set_key(const DES_cblock *key, DES_key_schedule *schedule)
+{
+  int result = 0;
+
+  if (!DES_check_key_parity(key)) {
+    result = -1;
+  }
+  if (DES_is_weak_key(key)) {
+    result = -2;
+  }
+  DES_set_key_unchecked(key, schedule);
+  return result;
+}
+
 int DES_key_sched(const DES_cblock *key, DES_key_schedule *schedule) {
-  DES_set_key(key, schedule);
-  return 1;
+  return DES_set_key(key, schedule);
 }
 
 static const uint8_t kOddParity[256] = {
@@ -383,6 +412,39 @@ void DES_set_odd_parity(DES_cblock *key) {
   }
 }
 
+// Weak keys have unintended behaviors which may hurt the security of their use
+// see SP 800-67r2 section 3.3.2
+static const DES_cblock weak_keys[] = {
+    // Weak keys: encryption is equal to decryption (encrypting twice produces the original plaintext)
+    {{0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01}},
+    {{0xFE, 0xFE, 0xFE, 0xFE, 0xFE, 0xFE, 0xFE, 0xFE}},
+    {{0x1F, 0x1F, 0x1F, 0x1F, 0x0E, 0x0E, 0x0E, 0x0E}},
+    {{0xE0, 0xE0, 0xE0, 0xE0, 0xF1, 0xF1, 0xF1, 0xF1}},
+    // Semi-weak keys: encryption with one of these keys is equal to encryption with a different key
+    {{0x01, 0xFE, 0x01, 0xFE, 0x01, 0xFE, 0x01, 0xFE}},
+    {{0xFE, 0x01, 0xFE, 0x01, 0xFE, 0x01, 0xFE, 0x01}},
+    {{0x1F, 0xE0, 0x1F, 0xE0, 0x0E, 0xF1, 0x0E, 0xF1}},
+    {{0xE0, 0x1F, 0xE0, 0x1F, 0xF1, 0x0E, 0xF1, 0x0E}},
+    {{0x01, 0xE0, 0x01, 0xE0, 0x01, 0xF1, 0x01, 0xF1}},
+    {{0xE0, 0x01, 0xE0, 0x01, 0xF1, 0x01, 0xF1, 0x01}},
+    {{0x1F, 0xFE, 0x1F, 0xFE, 0x0E, 0xFE, 0x0E, 0xFE}},
+    {{0xFE, 0x1F, 0xFE, 0x1F, 0xFE, 0x0E, 0xFE, 0x0E}},
+    {{0x01, 0x1F, 0x01, 0x1F, 0x01, 0x0E, 0x01, 0x0E}},
+    {{0x1F, 0x01, 0x1F, 0x01, 0x0E, 0x01, 0x0E, 0x01}},
+    {{0xE0, 0xFE, 0xE0, 0xFE, 0xF1, 0xFE, 0xF1, 0xFE}},
+    {{0xFE, 0xE0, 0xFE, 0xE0, 0xFE, 0xF1, 0xFE, 0xF1}}
+};
+
+int DES_is_weak_key(const DES_cblock *key)
+{
+  crypto_word_t result = 0;
+  for (size_t i = 0; i < OPENSSL_ARRAY_SIZE(weak_keys); i++) {
+    int match = CRYPTO_memcmp(&weak_keys[i], key, sizeof(DES_cblock));
+    result |= constant_time_is_zero_w(match);
+  }
+  return (int)(result & 1);
+}
+
 static void DES_encrypt1(uint32_t *data, const DES_key_schedule *ks, int enc) {
   uint32_t l, r, t, u;
 

crypto/des/des_test.cc

@@ -0,0 +1,50 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0 OR ISC
+
+#include <openssl/des.h>
+
+#include <gtest/gtest.h>
+
+TEST(DESTest, WeakKeys) {
+  // The all 2 key is not weak and has odd parity
+  DES_cblock validKey =  {{2, 2, 2, 2, 2, 2, 2, 2}};
+  EXPECT_FALSE(DES_is_weak_key(&validKey));
+  DES_key_schedule des;
+  EXPECT_EQ(0, DES_set_key(&validKey, &des));
+
+  // Weak key example from SP 800-67r2 section 3.3.2
+  static const DES_cblock weakKey = {{0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01}};
+  EXPECT_TRUE(DES_is_weak_key(&weakKey));
+  EXPECT_EQ(-2, DES_set_key(&weakKey, &des));
+}
+
+// If it wasn't for MSVC this could be __builtin_popcount, if this was C++20
+// it could be std::popcount
+static int countSetBits(uint8_t n) {
+  int count = 0;
+  while (n) {
+    count += n & 1;
+    n >>= 1;
+  }
+  return count;
+}
+
+TEST(DESTest, Parity) {
+  // The all 2 key is not weak and has odd parity for each byte
+  DES_cblock key = {{2, 2, 2, 2, 2, 2, 2, 2}};
+  DES_key_schedule des;
+  int result = DES_set_key(&key, &des);
+  EXPECT_EQ(result, 0);
+
+  for (uint8_t i = 0; i < 255; i++) {
+    key.bytes[0] = i;
+    result = DES_set_key(&key, &des);
+    int bitsSet = countSetBits(i);
+    int oddParity = bitsSet % 2 == 1;
+    if (oddParity) {
+      EXPECT_EQ(result, 0);
+    } else {
+      EXPECT_EQ(result, -1);
+    }
+  }
+}

include/openssl/des.h

@@ -91,11 +91,22 @@ typedef struct DES_ks {
 #define DES_CBC_MODE 0
 #define DES_PCBC_MODE 1
 
-// DES_set_key performs a key schedule and initialises |schedule| with |key|.
-OPENSSL_EXPORT void DES_set_key(const DES_cblock *key,
-                                DES_key_schedule *schedule);
-
-// DES_key_sched calls |DES_set_key| and returns 1.
+// DES_is_weak_key checks if |key| is a weak or semi-weak key, see SP 800-67r2
+// section 3.3.2
+OPENSSL_EXPORT int DES_is_weak_key(const DES_cblock *key);
+
+// DES_set_key checks that |key| is not weak and the parity before calling
+// |DES_set_key_unchecked|. The key schedule is always initialized, the checks
+// only affect the return value:
+// 0: key is not weak and has odd parity
+// -1: key is not odd
+// -2: key is a weak key, the parity might also be even
+OPENSSL_EXPORT int DES_set_key(const DES_cblock *key, DES_key_schedule *schedule);
+
+// DES_set_key_unchecked performs a key schedule and initialises |schedule| with |key|.
+OPENSSL_EXPORT void DES_set_key_unchecked(const DES_cblock *key, DES_key_schedule *schedule);
+
+// DES_key_sched calls |DES_set_key|.
 OPENSSL_EXPORT int DES_key_sched(const DES_cblock *key, DES_key_schedule *schedule);
 
 // DES_set_odd_parity sets the parity bits (the least-significant bits in each

util/fipstools/test_fips.c

@@ -276,9 +276,9 @@ int main(int argc, char **argv) {
 
   DES_key_schedule des1, des2, des3;
   DES_cblock des_iv;
-  DES_set_key(&kDESKey1, &des1);
-  DES_set_key(&kDESKey2, &des2);
-  DES_set_key(&kDESKey3, &des3);
+  DES_set_key_unchecked(&kDESKey1, &des1);
+  DES_set_key_unchecked(&kDESKey2, &des2);
+  DES_set_key_unchecked(&kDESKey3, &des3);
 
   /* 3DES Encryption */
   memcpy(&des_iv, &kDESIV, sizeof(des_iv));