Add additional sanity checks to RSA verification code.

Also, make algorithm unsigned int in most places.

BUG=chrome-os-partner:701
TEST=existing RSA verification tests still pass

Review URL: http://codereview.chromium.org/3136017

firmware/lib/cryptolib/include/rsa.h

@@ -27,7 +27,7 @@ typedef struct RSAPublicKey {
   uint32_t n0inv;  /* -1 / n[0] mod 2^32 */
   uint32_t* n;  /* modulus as little endian array */
   uint32_t* rr; /* R^2 as little endian array */
-  int algorithm; /* Algorithm to use when verifying binaries with the key */
+  unsigned int algorithm; /* Algorithm to use when verifying with the key */
 } RSAPublicKey;
 
 /* Verify a RSA PKCS1.5 signature [sig] of [sig_type] and length [sig_len]
@@ -57,7 +57,7 @@ int RSAVerifyBinary_f(const uint8_t* key_blob,
                       const uint8_t* buf,
                       uint64_t len,
                       const uint8_t* sig,
-                      int algorithm);
+                      unsigned int algorithm);
 
 /* Version of RSAVerifyBinary_f() where instead of the raw binary blob
  * of data, its digest is passed as the argument. */
@@ -65,14 +65,17 @@ int RSAVerifyBinaryWithDigest_f(const uint8_t* key_blob,
                                 const RSAPublicKey* key,
                                 const uint8_t* digest,
                                 const uint8_t* sig,
-                                int algorithm);
+                                unsigned int algorithm);
 
 
 /* ----Some additional utility functions for RSA.---- */
 
-/* Returns the size of a pre-processed RSA public key in bytes with algorithm
- * [algorithm]. */
-int RSAProcessedKeySize(int algorithm);
+/* Returns the size of a pre-processed RSA public key in
+ * [out_size] with the algorithm [algorithm].
+ *
+ * Returns 1 on success, 0 on failure.
+ */
+int RSAProcessedKeySize(unsigned int algorithm, int* out_size);
 
 /* Allocate a new RSAPublicKey structure and initialize its pointer fields to
  * NULL */

firmware/lib/cryptolib/rsa.c

@@ -134,6 +134,9 @@ int RSAVerify(const RSAPublicKey *key,
   const uint8_t* padding;
   int success = 1;
 
+  if (!key || !sig || !hash)
+    return 0;
+
   if (sig_len != (key->len * sizeof(uint32_t))) {
     VBDEBUG(("Signature is of incorrect length!\n"));
     return 0;

firmware/lib/cryptolib/rsa_utility.c

@@ -9,14 +9,18 @@
 #include "stateful_util.h"
 #include "utility.h"
 
-int RSAProcessedKeySize(int algorithm) {
-  int key_len = siglen_map[algorithm];  /* Key length in
-                                         * bytes. */
-  /* Total size needed by a RSAPublicKey structure is =
-   *  2 * key_len bytes for the  n and rr arrays
-   *  + sizeof len + sizeof n0inv.
-   */
-  return (2 * key_len + sizeof(uint32_t) + sizeof(uint32_t));
+int RSAProcessedKeySize(unsigned int algorithm, int* out_size) {
+  int key_len; /* Key length in bytes. */
+  if (algorithm < kNumAlgorithms) {
+    key_len =  siglen_map[algorithm];
+    /* Total size needed by a RSAPublicKey structure is =
+     *  2 * key_len bytes for the  n and rr arrays
+     *  + sizeof len + sizeof n0inv.
+     */
+    *out_size = (2 * key_len + sizeof(uint32_t) + sizeof(uint32_t));
+    return 1;
+  }
+  return 0;
 }
 
 RSAPublicKey* RSAPublicKeyNew(void) {
@@ -74,7 +78,7 @@ int RSAVerifyBinary_f(const uint8_t* key_blob,
                       const uint8_t* buf,
                       uint64_t len,
                       const uint8_t* sig,
-                      int algorithm) {
+                      unsigned int algorithm) {
   RSAPublicKey* verification_key = NULL;
   uint8_t* digest = NULL;
   int key_size;
@@ -83,7 +87,8 @@ int RSAVerifyBinary_f(const uint8_t* key_blob,
 
   if (algorithm >= kNumAlgorithms)
     return 0;  /* Invalid algorithm. */
-  key_size = RSAProcessedKeySize(algorithm);
+  if (!RSAProcessedKeySize(algorithm, &key_size))
+    return 0;
   sig_size = siglen_map[algorithm];
 
   if (key_blob && !key)
@@ -93,6 +98,10 @@ int RSAVerifyBinary_f(const uint8_t* key_blob,
   else
     return 0; /* Both can't be NULL or non-NULL. */
 
+  /* Ensure we have a valid key. */
+  if (!verification_key)
+    return 0;
+
   digest = DigestBuf(buf, len, algorithm);
   success = RSAVerify(verification_key, sig, (uint32_t)sig_size,
                       (uint8_t)algorithm, digest);
@@ -109,7 +118,7 @@ int RSAVerifyBinaryWithDigest_f(const uint8_t* key_blob,
                                 const RSAPublicKey* key,
                                 const uint8_t* digest,
                                 const uint8_t* sig,
-                                int algorithm) {
+                                unsigned int algorithm) {
   RSAPublicKey* verification_key = NULL;
   int key_size;
   int sig_size;
@@ -117,7 +126,8 @@ int RSAVerifyBinaryWithDigest_f(const uint8_t* key_blob,
 
   if (algorithm >= kNumAlgorithms)
     return 0;  /* Invalid algorithm. */
-  key_size = RSAProcessedKeySize(algorithm);
+  if (!RSAProcessedKeySize(algorithm, &key_size))
+    return 0;
   sig_size = siglen_map[algorithm];
 
   if (key_blob && !key)
@@ -127,6 +137,10 @@ int RSAVerifyBinaryWithDigest_f(const uint8_t* key_blob,
   else
     return 0; /* Both can't be NULL or non-NULL. */
 
+  /* Ensure we have a valid key. */
+  if (!verification_key)
+    return 0;
+
   success = RSAVerify(verification_key, sig, (uint32_t)sig_size,
                       (uint8_t)algorithm, digest);
 

firmware/lib/vboot_common.c

@@ -114,12 +114,14 @@ int PublicKeyCopy(VbPublicKey* dest, const VbPublicKey* src) {
 
 RSAPublicKey* PublicKeyToRSA(const VbPublicKey* key) {
   RSAPublicKey *rsa;
+  int key_size;
 
   if (kNumAlgorithms <= key->algorithm) {
     VBDEBUG(("Invalid algorithm.\n"));
     return NULL;
   }
-  if (RSAProcessedKeySize((int)key->algorithm) != (int)key->key_size) {
+  if (!RSAProcessedKeySize((int)key->algorithm, &key_size) ||
+      key_size != (int)key->key_size) {
     VBDEBUG(("Wrong key size for algorithm\n"));
     return NULL;
   }

firmware/version.c

@@ -1 +1 @@
-char* VbootVersion = "VBOOv=4fa4f8d2";
+char* VbootVersion = "VBOOv=018098b3";

host/include/file_keys.h

@@ -38,6 +38,6 @@ uint8_t* DigestFile(char* input_file, int sig_algorithm);
  * Returns the signature. Caller owns the buffer and must Free() it.
  */
 uint8_t* SignatureFile(const char* input_file, const char* key_file,
-                       int algorithm);
+                       unsigned int algorithm);
 
 #endif  /* VBOOT_REFERENCE_FILE_KEYS_H_ */

host/include/signature_digest.h

@@ -6,12 +6,12 @@
 #ifndef VBOOT_REFERENCE_SIGNATURE_DIGEST_H_
 #define VBOOT_REFERENCE_SIGNATURE_DIGEST_H_
 
-#include <inttypes.h>
+#include <stdint.h>
 
 /* Returns a buffer with DigestInfo (which depends on [algorithm])
  * prepended to [digest].
  */
-uint8_t* PrependDigestInfo(int algorithm, uint8_t* digest);
+uint8_t* PrependDigestInfo(unsigned int algorithm, uint8_t* digest);
 
 /* Function that outputs the message digest of the contents of a buffer in a
  * format that can be used as input to OpenSSL for an RSA signature.
@@ -22,7 +22,8 @@ uint8_t* PrependDigestInfo(int algorithm, uint8_t* digest);
  * choice of the hash algorithm (see padding.c). Caller owns the returned
  * pointer and must Free() it.
  */
-uint8_t* SignatureDigest(const uint8_t* buf, uint64_t len, int algorithm);
+uint8_t* SignatureDigest(const uint8_t* buf, uint64_t len,
+                         unsigned int algorithm);
 
 /* Calculates the signature on a buffer [buf] of length [len] using
  * the private RSA key file from [key_file] and signature algorithm
@@ -31,5 +32,5 @@ uint8_t* SignatureDigest(const uint8_t* buf, uint64_t len, int algorithm);
  * Returns the signature. Caller owns the buffer and must Free() it.
  */
 uint8_t* SignatureBuf(const uint8_t* buf, uint64_t len, const char* key_file,
-                      int algorithm);
+                      unsigned int algorithm);
 #endif  /* VBOOT_REFERENCE_SIGNATURE_DIGEST_H_ */

host/lib/file_keys.c

@@ -82,7 +82,7 @@ uint8_t* DigestFile(char* input_file, int sig_algorithm) {
 }
 
 uint8_t* SignatureFile(const char* input_file, const char* key_file,
-                       int algorithm) {
+                       unsigned int algorithm) {
   char* sign_utility = "./sign_data.sh";
   char* cmd;  /* Command line to invoke. */
   int cmd_len;
@@ -99,7 +99,7 @@ uint8_t* SignatureFile(const char* input_file, const char* key_file,
              strlen(input_file) +
              1);  /* For the trailing '\0'. */
   cmd = (char*) Malloc(cmd_len);
-  snprintf(cmd, cmd_len, "%s %d %s %s", sign_utility, algorithm, key_file,
+  snprintf(cmd, cmd_len, "%s %u %s %s", sign_utility, algorithm, key_file,
            input_file);
   cmd_out = popen(cmd, "r");
   Free(cmd);

host/lib/host_key.c

@@ -148,8 +148,6 @@ VbPrivateKey* PrivateKeyRead(const char* filename) {
 }
 
 
-
-
 /* Allocate a new public key with space for a [key_size] byte key. */
 VbPublicKey* PublicKeyAlloc(uint64_t key_size, uint64_t algorithm,
                             uint64_t version) {
@@ -164,12 +162,12 @@ VbPublicKey* PublicKeyAlloc(uint64_t key_size, uint64_t algorithm,
   return key;
 }
 
-
 VbPublicKey* PublicKeyReadKeyb(const char* filename, uint64_t algorithm,
                                uint64_t version) {
   VbPublicKey* key;
   uint8_t* key_data;
   uint64_t key_size;
+  int expected_key_size;
 
   if (algorithm >= kNumAlgorithms) {
     VBDEBUG(("PublicKeyReadKeyb() called with invalid algorithm!\n"));
@@ -185,7 +183,8 @@ VbPublicKey* PublicKeyReadKeyb(const char* filename, uint64_t algorithm,
   if (!key_data)
     return NULL;
 
-  if (RSAProcessedKeySize(algorithm) != key_size) {
+  if (!RSAProcessedKeySize(algorithm, &expected_key_size) ||
+      expected_key_size != key_size) {
     VBDEBUG(("PublicKeyReadKeyb() wrong key size for algorithm\n"));
     Free(key_data);
     return NULL;
@@ -206,6 +205,7 @@ VbPublicKey* PublicKeyReadKeyb(const char* filename, uint64_t algorithm,
 VbPublicKey* PublicKeyRead(const char* filename) {
   VbPublicKey* key;
   uint64_t file_size;
+  int key_size;
 
   key = (VbPublicKey*)ReadFile(filename, &file_size);
   if (!key)
@@ -225,7 +225,8 @@ VbPublicKey* PublicKeyRead(const char* filename) {
       VBDEBUG(("PublicKeyRead() invalid version\n"));
       break;  /* Currently, TPM only supports 16-bit version */
     }
-    if (RSAProcessedKeySize(key->algorithm) != key->key_size) {
+    if (!RSAProcessedKeySize(key->algorithm, &key_size) ||
+        key_size != key->key_size) {
       VBDEBUG(("PublicKeyRead() wrong key size for algorithm\n"));
       break;
     }
@@ -240,7 +241,6 @@ VbPublicKey* PublicKeyRead(const char* filename) {
   return NULL;
 }
 
-
 int PublicKeyWrite(const char* filename, const VbPublicKey* key) {
   VbPublicKey* kcopy;
   int rv;

host/lib/signature_digest.c

@@ -16,7 +16,7 @@
 #include "cryptolib.h"
 #include "utility.h"
 
-uint8_t* PrependDigestInfo(int algorithm, uint8_t* digest) {
+uint8_t* PrependDigestInfo(unsigned int algorithm, uint8_t* digest) {
   const int digest_size = hash_size_map[algorithm];
   const int digestinfo_size = digestinfo_size_map[algorithm];
   const uint8_t* digestinfo = hash_digestinfo_map[algorithm];
@@ -26,7 +26,8 @@ uint8_t* PrependDigestInfo(int algorithm, uint8_t* digest) {
   return p;
 }
 
-uint8_t* SignatureDigest(const uint8_t* buf, uint64_t len, int algorithm) {
+uint8_t* SignatureDigest(const uint8_t* buf, uint64_t len,
+                         unsigned int algorithm) {
   uint8_t* info_digest  = NULL;
   uint8_t* digest = NULL;
 
@@ -40,7 +41,7 @@ uint8_t* SignatureDigest(const uint8_t* buf, uint64_t len, int algorithm) {
 }
 
 uint8_t* SignatureBuf(const uint8_t* buf, uint64_t len, const char* key_file,
-                      int algorithm) {
+                      unsigned int algorithm) {
   FILE* key_fp = NULL;
   RSA* key = NULL;
   uint8_t* signature = NULL;