Stop using VLAs

While the lengths were never actually variable, in the eyes of the C
language, they could have been. The compiler doesn't acknowledge that
the variables are actually constants but using #define instead is enough
to stop it worrying about it.

With this change, -Wvla is added to the build flags to make sure there
aren't any truly variable length arrays and lets the library fit more
easily into projects that take VLAs more seriously.

Test: ninja -C out/
Change-Id: Ib41174211e1f55fe9d8f4b1e0e834e999b8a6b7d
Reviewed-on: https://pigweed-review.googlesource.com/c/open-dice/+/65680
Commit-Queue: Andrew Scull <ascull@google.com>
Pigweed-Auto-Submit: Andrew Scull <ascull@google.com>
Reviewed-by: Darren Krahn <dkrahn@google.com>

6 files changed, 42 insertions, 42 deletions

diff --git a/src/boringssl_cert_op.c b/src/boringssl_cert_op.c
index 8ff0503..08315de 100644
--- a/src/boringssl_cert_op.c
+++ b/src/boringssl_cert_op.c
@@ -31,6 +31,8 @@
 #include "openssl/x509.h"
 #include "openssl/x509v3.h"
 
+#define DICE_MAX_EXTENSION_SIZE 2048
+
 typedef struct DiceExtensionAsn1 {
   ASN1_OCTET_STRING* code_hash;
   ASN1_OCTET_STRING* code_descriptor;
@@ -440,7 +442,6 @@ out:
 
 static DiceResult AddDiceExtension(const DiceInputValues* input_values,
                                    X509* x509) {
-  const size_t kMaxExtensionSize = 2048;
   const char* kDiceExtensionOid = "1.3.6.1.4.1.11129.2.1.24";
 
   // Initialize variables that are cleaned up on 'goto out'.
@@ -448,7 +449,7 @@ static DiceResult AddDiceExtension(const DiceInputValues* input_values,
   ASN1_OCTET_STRING* octets = NULL;
   X509_EXTENSION* extension = NULL;
 
-  uint8_t extension_buffer[kMaxExtensionSize];
+  uint8_t extension_buffer[DICE_MAX_EXTENSION_SIZE];
   size_t extension_size = 0;
   DiceResult result =
       GetDiceExtensionData(input_values, sizeof(extension_buffer),
diff --git a/src/cbor_cert_op.c b/src/cbor_cert_op.c
index 915d443..8006294 100644
--- a/src/cbor_cert_op.c
+++ b/src/cbor_cert_op.c
@@ -33,11 +33,11 @@
 #endif
 
 // Max size of COSE_Sign1 including payload.
-static const size_t kMaxCertificateSize = 2048;
+#define DICE_MAX_CERTIFICATE_SIZE 2048
 // Max size of COSE_Key encoding.
-static const size_t kMaxPublicKeySize = 64;
+#define DICE_MAX_PUBLIC_KEY_SIZE 64
 // Max size of the COSE_Sign1 protected attributes.
-static const size_t kMaxProtectedAttributesSize = 16;
+#define DICE_MAX_PROTECTED_ATTRIBUTES_SIZE 16
 
 DiceResult DiceCoseEncodePublicKey(
     void* context_not_used, const uint8_t public_key[DICE_PUBLIC_KEY_SIZE],
@@ -161,7 +161,7 @@ DiceResult DiceCoseSignAndEncodeSign1(
 
   // The encoded protected attributes are used in the TBS and the final
   // COSE_Sign1 structure.
-  uint8_t protected_attributes[kMaxProtectedAttributesSize];
+  uint8_t protected_attributes[DICE_MAX_PROTECTED_ATTRIBUTES_SIZE];
   size_t protected_attributes_size = 0;
   result = EncodeProtectedAttributes(sizeof(protected_attributes),
                                      protected_attributes,
@@ -313,10 +313,6 @@ DiceResult DiceGenerateCertificate(
   uint8_t subject_private_key[DICE_PRIVATE_KEY_SIZE];
   uint8_t authority_private_key[DICE_PRIVATE_KEY_SIZE];
 
-  // These are 'variably modified' types so need to be declared upfront.
-  uint8_t encoded_public_key[kMaxPublicKeySize];
-  uint8_t payload[kMaxCertificateSize];
-
   // Derive keys and IDs from the private key seeds.
   uint8_t subject_public_key[DICE_PUBLIC_KEY_SIZE];
   result = DiceKeypairFromSeed(context, subject_private_key_seed,
@@ -355,6 +351,7 @@ DiceResult DiceGenerateCertificate(
   authority_id_hex[sizeof(authority_id_hex) - 1] = '\0';
 
   // The public key encoded as a COSE_Key structure is embedded in the CWT.
+  uint8_t encoded_public_key[DICE_MAX_PUBLIC_KEY_SIZE];
   size_t encoded_public_key_size = 0;
   result = DiceCoseEncodePublicKey(
       context, subject_public_key, sizeof(encoded_public_key),
@@ -364,6 +361,7 @@ DiceResult DiceGenerateCertificate(
   }
 
   // The CWT is the payload in both the TBS and the final COSE_Sign1 structure.
+  uint8_t payload[DICE_MAX_CERTIFICATE_SIZE];
   size_t payload_size = 0;
   result = EncodeCwt(context, input_values, authority_id_hex, subject_id_hex,
                      encoded_public_key, encoded_public_key_size,
diff --git a/src/dice.c b/src/dice.c
index df1dee3..e9b0cd9 100644
--- a/src/dice.c
+++ b/src/dice.c
@@ -18,6 +18,11 @@
 
 #include "dice/ops.h"
 
+#define DICE_CODE_SIZE DICE_HASH_SIZE
+#define DICE_CONFIG_SIZE DICE_INLINE_CONFIG_SIZE
+#define DICE_AUTHORITY_SIZE DICE_HASH_SIZE
+#define DICE_MODE_SIZE 1
+
 static const uint8_t kAsymSalt[] = {
     0x63, 0xB6, 0xA0, 0x4D, 0x2C, 0x07, 0x7F, 0xC1, 0x0F, 0x63, 0x9F,
     0x21, 0xDA, 0x79, 0x38, 0x44, 0x35, 0x6C, 0xC2, 0xB0, 0xB4, 0x41,
@@ -77,32 +82,27 @@ DiceResult DiceMainFlow(void* context,
   // ---------------------------------------------------------------------------
   // | Code Input | Config Input | Authority Input | Mode Input | Hidden Input |
   // ---------------------------------------------------------------------------
-  const size_t kCodeSize = DICE_HASH_SIZE;
-  const size_t kConfigSize = DICE_INLINE_CONFIG_SIZE;
-  const size_t kAuthoritySize = DICE_HASH_SIZE;
-  const size_t kModeSize = 1;
-  const size_t kHiddenSize = DICE_HIDDEN_SIZE;
   const size_t kCodeOffset = 0;
-  const size_t kConfigOffset = kCodeOffset + kCodeSize;
-  const size_t kAuthorityOffset = kConfigOffset + kConfigSize;
-  const size_t kModeOffset = kAuthorityOffset + kAuthoritySize;
-  const size_t kHiddenOffset = kModeOffset + kModeSize;
+  const size_t kConfigOffset = kCodeOffset + DICE_CODE_SIZE;
+  const size_t kAuthorityOffset = kConfigOffset + DICE_CONFIG_SIZE;
+  const size_t kModeOffset = kAuthorityOffset + DICE_AUTHORITY_SIZE;
+  const size_t kHiddenOffset = kModeOffset + DICE_MODE_SIZE;
 
   DiceResult result = kDiceResultOk;
 
   // Declare buffers that get cleaned up on 'goto out'.
-  uint8_t input_buffer[kCodeSize + kConfigSize + kAuthoritySize + kModeSize +
-                       kHiddenSize];
+  uint8_t input_buffer[DICE_CODE_SIZE + DICE_CONFIG_SIZE + DICE_AUTHORITY_SIZE +
+                       DICE_MODE_SIZE + DICE_HIDDEN_SIZE];
   uint8_t attest_input_hash[DICE_HASH_SIZE];
   uint8_t seal_input_hash[DICE_HASH_SIZE];
   uint8_t current_cdi_private_key_seed[DICE_PRIVATE_KEY_SEED_SIZE];
   uint8_t next_cdi_private_key_seed[DICE_PRIVATE_KEY_SEED_SIZE];
 
   // Assemble the input buffer.
-  memcpy(&input_buffer[kCodeOffset], input_values->code_hash, kCodeSize);
+  memcpy(&input_buffer[kCodeOffset], input_values->code_hash, DICE_CODE_SIZE);
   if (input_values->config_type == kDiceConfigTypeInline) {
     memcpy(&input_buffer[kConfigOffset], input_values->config_value,
-           kConfigSize);
+           DICE_CONFIG_SIZE);
   } else if (!input_values->config_descriptor) {
     result = kDiceResultInvalidInput;
     goto out;
@@ -115,9 +115,9 @@ DiceResult DiceMainFlow(void* context,
     }
   }
   memcpy(&input_buffer[kAuthorityOffset], input_values->authority_hash,
-         kAuthoritySize);
+         DICE_AUTHORITY_SIZE);
   input_buffer[kModeOffset] = input_values->mode;
-  memcpy(&input_buffer[kHiddenOffset], input_values->hidden, kHiddenSize);
+  memcpy(&input_buffer[kHiddenOffset], input_values->hidden, DICE_HIDDEN_SIZE);
 
   // Hash the appropriate input values for both attestation and sealing. For
   // attestation all the inputs are used, and for sealing only the authority,
@@ -128,7 +128,8 @@ DiceResult DiceMainFlow(void* context,
     goto out;
   }
   result = DiceHash(context, &input_buffer[kAuthorityOffset],
-                    kAuthoritySize + kModeSize + kHiddenSize, seal_input_hash);
+                    DICE_AUTHORITY_SIZE + DICE_MODE_SIZE + DICE_HIDDEN_SIZE,
+                    seal_input_hash);
   if (result != kDiceResultOk) {
     goto out;
   }
diff --git a/src/mbedtls_ops.c b/src/mbedtls_ops.c
index 7190d0e..18eb4d4 100644
--- a/src/mbedtls_ops.c
+++ b/src/mbedtls_ops.c
@@ -35,9 +35,9 @@
 #include "mbedtls/x509.h"
 #include "mbedtls/x509_crt.h"
 
-static const size_t kMaxCertificateSize = 2048;
-static const size_t kMaxExtensionSize = 2048;
-static const size_t kMaxKeyIdSize = 40;
+#define DICE_MAX_CERTIFICATE_SIZE 2048
+#define DICE_MAX_EXTENSION_SIZE 2048
+#define DICE_MAX_KEY_ID_SIZE 40
 
 static DiceResult SetupKeyPair(
     const uint8_t private_key_seed[DICE_PRIVATE_KEY_SEED_SIZE],
@@ -321,12 +321,6 @@ DiceResult DiceGenerateCertificate(
   mbedtls_mpi serial_number;
   mbedtls_mpi_init(&serial_number);
 
-  // These are 'variably modified' types so need to be declared upfront.
-  uint8_t authority_key_id[kMaxKeyIdSize];
-  uint8_t subject_key_id[kMaxKeyIdSize];
-  uint8_t dice_extension[kMaxExtensionSize];
-  uint8_t tmp_buffer[kMaxCertificateSize];
-
   // Derive key pairs and IDs.
   result = SetupKeyPair(authority_private_key_seed, &authority_key_context);
   if (result != kDiceResultOk) {
@@ -342,6 +336,7 @@ DiceResult DiceGenerateCertificate(
   char authority_name[54];
   GetNameFromId(authority_id, authority_name);
 
+  uint8_t authority_key_id[DICE_MAX_KEY_ID_SIZE];
   size_t authority_key_id_size = 0;
   result = GetAuthorityKeyIdFromId(authority_id, sizeof(authority_key_id),
                                    authority_key_id, &authority_key_id_size);
@@ -362,6 +357,7 @@ DiceResult DiceGenerateCertificate(
   char subject_name[54];
   GetNameFromId(subject_id, subject_name);
 
+  uint8_t subject_key_id[DICE_MAX_KEY_ID_SIZE];
   size_t subject_key_id_size = 0;
   result = GetSubjectKeyIdFromId(subject_id, sizeof(subject_key_id),
                                  subject_key_id, &subject_key_id_size);
@@ -369,6 +365,7 @@ DiceResult DiceGenerateCertificate(
     goto out;
   }
 
+  uint8_t dice_extension[DICE_MAX_EXTENSION_SIZE];
   size_t dice_extension_size = 0;
   result = GetDiceExtensionData(input_values, sizeof(dice_extension),
                                 dice_extension, &dice_extension_size);
@@ -443,6 +440,7 @@ DiceResult DiceGenerateCertificate(
   // This implementation is deterministic and assumes entropy is not available.
   // If this code is run where entropy is available, however, f_rng and p_rng
   // should be set appropriately.
+  uint8_t tmp_buffer[DICE_MAX_CERTIFICATE_SIZE];
   int length_or_error =
       mbedtls_x509write_crt_der(&cert_context, tmp_buffer, sizeof(tmp_buffer),
                                 /*f_rng=*/NULL, /*p_rng=*/NULL);
diff --git a/src/template_cbor_cert_op.c b/src/template_cbor_cert_op.c
index c2f6f42..641e83b 100644
--- a/src/template_cbor_cert_op.c
+++ b/src/template_cbor_cert_op.c
@@ -49,6 +49,9 @@
 #error "Only Ed25519 is supported; 64 bytes needed to store the signature."
 #endif
 
+// 20 bytes of header, 366 bytes of payload.
+#define DICE_TBS_SIZE 386
+
 // A well-formed certificate, but with zeros in all fields to be filled.
 static const uint8_t kTemplate[441] = {
     // Constant encoding.
@@ -123,8 +126,6 @@ static const uint8_t kTemplate[441] = {
 static const uint8_t kTbsHeader[20] = {0x84, 0x6a, 0x53, 0x69, 0x67, 0x6e, 0x61,
                                        0x74, 0x75, 0x72, 0x65, 0x31, 0x43, 0xa1,
                                        0x01, 0x27, 0x40, 0x59, 0x01, 0x6e};
-// 20 bytes of header, 366 bytes of payload.
-static const size_t kTbsSize = 386;
 
 static const struct {
   size_t offset;
@@ -180,9 +181,6 @@ DiceResult DiceGenerateCertificate(
   uint8_t subject_private_key[DICE_PRIVATE_KEY_SIZE];
   uint8_t authority_private_key[DICE_PRIVATE_KEY_SIZE];
 
-  // These are 'variably modified' types so need to be declared upfront.
-  uint8_t tbs[kTbsSize];
-
   // Derive keys and IDs from the private key seeds.
   uint8_t subject_public_key[DICE_PUBLIC_KEY_SIZE];
   result = DiceKeypairFromSeed(context, subject_private_key_seed,
@@ -231,17 +229,20 @@ DiceResult DiceGenerateCertificate(
   certificate[kFieldTable[kFieldIndexMode].offset] = input_values->mode;
 
   // Fill the TBS structure using the payload from the certificate.
+  uint8_t tbs[DICE_TBS_SIZE];
   memcpy(tbs, kTbsHeader, sizeof(kTbsHeader));
   memcpy(&tbs[sizeof(kTbsHeader)],
          &certificate[kFieldTable[kFieldIndexPayload].offset],
          kFieldTable[kFieldIndexPayload].length);
 
   uint8_t signature[DICE_SIGNATURE_SIZE];
-  result = DiceSign(context, tbs, kTbsSize, authority_private_key, signature);
+  result =
+      DiceSign(context, tbs, sizeof(tbs), authority_private_key, signature);
   if (result != kDiceResultOk) {
     goto out;
   }
-  result = DiceVerify(context, tbs, kTbsSize, signature, authority_public_key);
+  result =
+      DiceVerify(context, tbs, sizeof(tbs), signature, authority_public_key);
   if (result != kDiceResultOk) {
     goto out;
   }
diff --git a/toolchains/BUILD.gn b/toolchains/BUILD.gn
index 8287c00..76de412 100644
--- a/toolchains/BUILD.gn
+++ b/toolchains/BUILD.gn
@@ -28,6 +28,7 @@ config("common_config") {
 
   # No language extensions, to promote portability.
   cflags_c = [
+    "-Wvla",
     "-std=c99",
     "-pedantic",
   ]