Commit 567c3904 authored by Tianjia Zhang's avatar Tianjia Zhang Committed by Shuah Khan

selftests/sgx: Fix Q1 and Q2 calculation in sigstruct.c

Q1 and Q2 are numbers with *maximum* length of 384 bytes. If the
calculated length of Q1 and Q2 is less than 384 bytes, things will
go wrong.

E.g. if Q2 is 383 bytes, then

1. The bytes of q2 are copied to sigstruct->q2 in calc_q1q2().
2. The entire sigstruct->q2 is reversed, which results it being
   256 * Q2, given that the last byte of sigstruct->q2 is added
   to before the bytes given by calc_q1q2().

Either change in key or measurement can trigger the bug. E.g. an
unmeasured heap could cause a devastating change in Q1 or Q2.

Reverse exactly the bytes of Q1 and Q2 in calc_q1q2() before returning
to the caller.

Fixes: 2adcba79 ("selftests/x86: Add a selftest for SGX")
Link: https://lore.kernel.org/linux-sgx/20210301051836.30738-1-tianjia.zhang@linux.alibaba.com/Signed-off-by: default avatarTianjia Zhang <tianjia.zhang@linux.alibaba.com>
Signed-off-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: default avatarShuah Khan <skhan@linuxfoundation.org>
parent 2734d6c1
...@@ -55,10 +55,27 @@ static bool alloc_q1q2_ctx(const uint8_t *s, const uint8_t *m, ...@@ -55,10 +55,27 @@ static bool alloc_q1q2_ctx(const uint8_t *s, const uint8_t *m,
return true; return true;
} }
static void reverse_bytes(void *data, int length)
{
int i = 0;
int j = length - 1;
uint8_t temp;
uint8_t *ptr = data;
while (i < j) {
temp = ptr[i];
ptr[i] = ptr[j];
ptr[j] = temp;
i++;
j--;
}
}
static bool calc_q1q2(const uint8_t *s, const uint8_t *m, uint8_t *q1, static bool calc_q1q2(const uint8_t *s, const uint8_t *m, uint8_t *q1,
uint8_t *q2) uint8_t *q2)
{ {
struct q1q2_ctx ctx; struct q1q2_ctx ctx;
int len;
if (!alloc_q1q2_ctx(s, m, &ctx)) { if (!alloc_q1q2_ctx(s, m, &ctx)) {
fprintf(stderr, "Not enough memory for Q1Q2 calculation\n"); fprintf(stderr, "Not enough memory for Q1Q2 calculation\n");
...@@ -89,8 +106,10 @@ static bool calc_q1q2(const uint8_t *s, const uint8_t *m, uint8_t *q1, ...@@ -89,8 +106,10 @@ static bool calc_q1q2(const uint8_t *s, const uint8_t *m, uint8_t *q1,
goto out; goto out;
} }
BN_bn2bin(ctx.q1, q1); len = BN_bn2bin(ctx.q1, q1);
BN_bn2bin(ctx.q2, q2); reverse_bytes(q1, len);
len = BN_bn2bin(ctx.q2, q2);
reverse_bytes(q2, len);
free_q1q2_ctx(&ctx); free_q1q2_ctx(&ctx);
return true; return true;
...@@ -152,22 +171,6 @@ static RSA *gen_sign_key(void) ...@@ -152,22 +171,6 @@ static RSA *gen_sign_key(void)
return key; return key;
} }
static void reverse_bytes(void *data, int length)
{
int i = 0;
int j = length - 1;
uint8_t temp;
uint8_t *ptr = data;
while (i < j) {
temp = ptr[i];
ptr[i] = ptr[j];
ptr[j] = temp;
i++;
j--;
}
}
enum mrtags { enum mrtags {
MRECREATE = 0x0045544145524345, MRECREATE = 0x0045544145524345,
MREADD = 0x0000000044444145, MREADD = 0x0000000044444145,
...@@ -367,8 +370,6 @@ bool encl_measure(struct encl *encl) ...@@ -367,8 +370,6 @@ bool encl_measure(struct encl *encl)
/* BE -> LE */ /* BE -> LE */
reverse_bytes(sigstruct->signature, SGX_MODULUS_SIZE); reverse_bytes(sigstruct->signature, SGX_MODULUS_SIZE);
reverse_bytes(sigstruct->modulus, SGX_MODULUS_SIZE); reverse_bytes(sigstruct->modulus, SGX_MODULUS_SIZE);
reverse_bytes(sigstruct->q1, SGX_MODULUS_SIZE);
reverse_bytes(sigstruct->q2, SGX_MODULUS_SIZE);
EVP_MD_CTX_destroy(ctx); EVP_MD_CTX_destroy(ctx);
RSA_free(key); RSA_free(key);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment