Skip to content

Commit 567c390

Browse files
uudiinshuahkh
authored andcommitted
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: 2adcba7 ("selftests/x86: Add a selftest for SGX") Link: https://lore.kernel.org/linux-sgx/[email protected]/ Signed-off-by: Tianjia Zhang <[email protected]> Signed-off-by: Jarkko Sakkinen <[email protected]> Signed-off-by: Shuah Khan <[email protected]>
1 parent 2734d6c commit 567c390

File tree

1 file changed

+21
-20
lines changed

1 file changed

+21
-20
lines changed

tools/testing/selftests/sgx/sigstruct.c

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,27 @@ static bool alloc_q1q2_ctx(const uint8_t *s, const uint8_t *m,
5555
return true;
5656
}
5757

58+
static void reverse_bytes(void *data, int length)
59+
{
60+
int i = 0;
61+
int j = length - 1;
62+
uint8_t temp;
63+
uint8_t *ptr = data;
64+
65+
while (i < j) {
66+
temp = ptr[i];
67+
ptr[i] = ptr[j];
68+
ptr[j] = temp;
69+
i++;
70+
j--;
71+
}
72+
}
73+
5874
static bool calc_q1q2(const uint8_t *s, const uint8_t *m, uint8_t *q1,
5975
uint8_t *q2)
6076
{
6177
struct q1q2_ctx ctx;
78+
int len;
6279

6380
if (!alloc_q1q2_ctx(s, m, &ctx)) {
6481
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,
89106
goto out;
90107
}
91108

92-
BN_bn2bin(ctx.q1, q1);
93-
BN_bn2bin(ctx.q2, q2);
109+
len = BN_bn2bin(ctx.q1, q1);
110+
reverse_bytes(q1, len);
111+
len = BN_bn2bin(ctx.q2, q2);
112+
reverse_bytes(q2, len);
94113

95114
free_q1q2_ctx(&ctx);
96115
return true;
@@ -152,22 +171,6 @@ static RSA *gen_sign_key(void)
152171
return key;
153172
}
154173

155-
static void reverse_bytes(void *data, int length)
156-
{
157-
int i = 0;
158-
int j = length - 1;
159-
uint8_t temp;
160-
uint8_t *ptr = data;
161-
162-
while (i < j) {
163-
temp = ptr[i];
164-
ptr[i] = ptr[j];
165-
ptr[j] = temp;
166-
i++;
167-
j--;
168-
}
169-
}
170-
171174
enum mrtags {
172175
MRECREATE = 0x0045544145524345,
173176
MREADD = 0x0000000044444145,
@@ -367,8 +370,6 @@ bool encl_measure(struct encl *encl)
367370
/* BE -> LE */
368371
reverse_bytes(sigstruct->signature, SGX_MODULUS_SIZE);
369372
reverse_bytes(sigstruct->modulus, SGX_MODULUS_SIZE);
370-
reverse_bytes(sigstruct->q1, SGX_MODULUS_SIZE);
371-
reverse_bytes(sigstruct->q2, SGX_MODULUS_SIZE);
372373

373374
EVP_MD_CTX_destroy(ctx);
374375
RSA_free(key);

0 commit comments

Comments
 (0)