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
diff --git a/firmware/lib/cryptolib/include/rsa.h b/firmware/lib/cryptolib/include/rsa.h
index b6a83a1..f5b83ef 100644
--- a/firmware/lib/cryptolib/include/rsa.h
+++ b/firmware/lib/cryptolib/include/rsa.h
@@ -27,7 +27,7 @@
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 @@
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 @@
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 */
diff --git a/firmware/lib/cryptolib/rsa.c b/firmware/lib/cryptolib/rsa.c
index adc0dc0..bad01d8 100644
--- a/firmware/lib/cryptolib/rsa.c
+++ b/firmware/lib/cryptolib/rsa.c
@@ -134,6 +134,9 @@
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;
diff --git a/firmware/lib/cryptolib/rsa_utility.c b/firmware/lib/cryptolib/rsa_utility.c
index c3cf50e..3b82c06 100644
--- a/firmware/lib/cryptolib/rsa_utility.c
+++ b/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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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);
diff --git a/firmware/lib/vboot_common.c b/firmware/lib/vboot_common.c
index 61510d1..be76b05 100644
--- a/firmware/lib/vboot_common.c
+++ b/firmware/lib/vboot_common.c
@@ -114,12 +114,14 @@
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;
}
diff --git a/firmware/version.c b/firmware/version.c
index c50bc7d..9075336 100644
--- a/firmware/version.c
+++ b/firmware/version.c
@@ -1 +1 @@
-char* VbootVersion = "VBOOv=4fa4f8d2";
+char* VbootVersion = "VBOOv=018098b3";
diff --git a/host/include/file_keys.h b/host/include/file_keys.h
index 285a3e5..39fdc5a 100644
--- a/host/include/file_keys.h
+++ b/host/include/file_keys.h
@@ -38,6 +38,6 @@
* 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_ */
diff --git a/host/include/signature_digest.h b/host/include/signature_digest.h
index 55662b9..40c2703 100644
--- a/host/include/signature_digest.h
+++ b/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 @@
* 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 @@
* 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_ */
diff --git a/host/lib/file_keys.c b/host/lib/file_keys.c
index 181879d..0c57dff 100644
--- a/host/lib/file_keys.c
+++ b/host/lib/file_keys.c
@@ -82,7 +82,7 @@
}
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 @@
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);
diff --git a/host/lib/host_key.c b/host/lib/host_key.c
index da91e66..bcc89fc 100644
--- a/host/lib/host_key.c
+++ b/host/lib/host_key.c
@@ -116,7 +116,7 @@
uint64_t filelen = 0;
uint8_t *buffer;
const unsigned char *start;
-
+
buffer = ReadFile(filename, &filelen);
if (!buffer) {
error("unable to read from file %s\n", filename);
@@ -148,8 +148,6 @@
}
-
-
/* 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 @@
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 @@
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* 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 @@
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 @@
return NULL;
}
-
int PublicKeyWrite(const char* filename, const VbPublicKey* key) {
VbPublicKey* kcopy;
int rv;
diff --git a/host/lib/signature_digest.c b/host/lib/signature_digest.c
index eaa6069..6bf5c82 100644
--- a/host/lib/signature_digest.c
+++ b/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 @@
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* 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;