kris/pqcrypto
1
1
Fork 0
mirror of https://github.com/henrydcase/pqc.git synced 2024-11-22 15:39:07 +00:00
Code Issues 1 Releases Wiki Activity

Fixed some buffer handling bugs that should never happen in practice (but may occur if reusing the internal functions with different parameters).

Browse Source
This commit is contained in:
Thomas Pornin 2019-07-31 16:17:23 -04:00
parent 83c974e45b
commit bad9956869
4 changed files with 116 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
crypto_sign/falcon-1024/clean/keygen.c
View File

@ -4069,7 +4069,7 @@ PQCLEAN_FALCON1024_CLEAN_keygen(shake256_context *rng,
* and Res(g,phi) are not prime to each other. * and Res(g,phi) are not prime to each other.
*/ */
size_t n, u; size_t n, u;
uint16_t *tmp2; uint16_t *h2, *tmp2;
n = MKN(logn); n = MKN(logn);
@ -4176,12 +4176,13 @@ PQCLEAN_FALCON1024_CLEAN_keygen(shake256_context *rng,
* fails, we must restart. * fails, we must restart.
*/ */
if (h == NULL) { if (h == NULL) {
h = (uint16_t *)tmp; h2 = (uint16_t *)tmp;
tmp2 = h + n; tmp2 = h2 + n;
} else { } else {
h2 = h;
tmp2 = (uint16_t *)tmp; tmp2 = (uint16_t *)tmp;
} }
if (!PQCLEAN_FALCON1024_CLEAN_compute_public(h, f, g, logn, (uint8_t *)tmp2)) { if (!PQCLEAN_FALCON1024_CLEAN_compute_public(h2, f, g, logn, (uint8_t *)tmp2)) {
continue; continue;
} }

94
crypto_sign/falcon-1024/clean/sign.c
View File

@ -461,15 +461,16 @@ ffSampling_fft(samplerZ samp, void *samp_ctx,
} }
/* /*
* Compute a signature: the signature contains two vectors, s1 and s2; * Compute a signature: the signature contains two vectors, s1 and s2.
* the caller must still check that they comply with the signature * The s1 vector is not returned. The squared norm of (s1,s2) is
* bound, and try again if that is not the case. The s1 vector is not * computed, and if it is short enough, then s2 is returned into the
* returned; instead, its squared norm (saturated) is returned. This * s2[] buffer, and 1 is returned; otherwise, s2[] is untouched and 0 is
* function uses an expanded key. * returned; the caller should then try again. This function uses an
* expanded key.
* *
* tmp[] must have room for at least six polynomials. * tmp[] must have room for at least six polynomials.
*/ */
static uint32_t static int
do_sign_tree(samplerZ samp, void *samp_ctx, int16_t *s2, do_sign_tree(samplerZ samp, void *samp_ctx, int16_t *s2,
const fpr *expanded_key, const fpr *expanded_key,
const uint16_t *hm, const uint16_t *hm,
@ -479,6 +480,7 @@ do_sign_tree(samplerZ samp, void *samp_ctx, int16_t *s2,
const fpr *b00, *b01, *b10, *b11, *tree; const fpr *b00, *b01, *b10, *b11, *tree;
fpr ni; fpr ni;
uint32_t sqn, ng; uint32_t sqn, ng;
int16_t *s2tmp;
n = MKN(logn); n = MKN(logn);
t0 = tmp; t0 = tmp;
@ -550,23 +552,37 @@ do_sign_tree(samplerZ samp, void *samp_ctx, int16_t *s2,
ng |= sqn; ng |= sqn;
} }
sqn |= -(ng >> 31); sqn |= -(ng >> 31);
/*
* With "normal" degrees (e.g. 512 or 1024), it is very
* improbable that the computed vector is not short enough;
* however, it may happen in practice for the very reduced
* versions (e.g. degree 16 or below). In that case, the caller
* will loop, and we must not write anything into s2[] because
* s2[] may overlap with the hashed message hm[] and we need
* hm[] for the next iteration.
*/
s2tmp = (int16_t *)tmp;
for (u = 0; u < n; u ++) { for (u = 0; u < n; u ++) {
s2[u] = (int16_t) - fpr_rint(t1[u]); s2tmp[u] = (int16_t) - fpr_rint(t1[u]);
} }
return sqn; if (PQCLEAN_FALCON1024_CLEAN_is_short_half(sqn, s2tmp, logn)) {
memcpy(s2, s2tmp, n * sizeof * s2);
return 1;
}
return 0;
} }
/* /*
* Compute a signature: the signature contains two vectors, s1 and s2; * Compute a signature: the signature contains two vectors, s1 and s2.
* the caller must still check that they comply with the signature * The s1 vector is not returned. The squared norm of (s1,s2) is
* bound, and try again if that is not the case. The s1 vector is not * computed, and if it is short enough, then s2 is returned into the
* returned; instead, its squared norm (saturated) is returned. This * s2[] buffer, and 1 is returned; otherwise, s2[] is untouched and 0 is
* function uses a raw key and recomputes the B0 matrix and LDL tree * returned; the caller should then try again.
* dynamically.
* *
* tmp[] must have room for at least nine polynomials. * tmp[] must have room for at least nine polynomials.
*/ */
static uint32_t static int
do_sign_dyn(samplerZ samp, void *samp_ctx, int16_t *s2, do_sign_dyn(samplerZ samp, void *samp_ctx, int16_t *s2,
const int8_t *f, const int8_t *g, const int8_t *f, const int8_t *g,
const int8_t *F, const int8_t *G, const int8_t *F, const int8_t *G,
@ -576,6 +592,7 @@ do_sign_dyn(samplerZ samp, void *samp_ctx, int16_t *s2,
fpr *b00, *b01, *b10, *b11, *g00, *g01, *g11; fpr *b00, *b01, *b10, *b11, *g00, *g01, *g11;
fpr ni; fpr ni;
uint32_t sqn, ng; uint32_t sqn, ng;
int16_t *s2tmp;
n = MKN(logn); n = MKN(logn);
@ -738,10 +755,25 @@ do_sign_dyn(samplerZ samp, void *samp_ctx, int16_t *s2,
ng |= sqn; ng |= sqn;
} }
sqn |= -(ng >> 31); sqn |= -(ng >> 31);
/*
* With "normal" degrees (e.g. 512 or 1024), it is very
* improbable that the computed vector is not short enough;
* however, it may happen in practice for the very reduced
* versions (e.g. degree 16 or below). In that case, the caller
* will loop, and we must not write anything into s2[] because
* s2[] may overlap with the hashed message hm[] and we need
* hm[] for the next iteration.
*/
s2tmp = (int16_t *)tmp;
for (u = 0; u < n; u ++) { for (u = 0; u < n; u ++) {
s2[u] = (int16_t) - fpr_rint(t1[u]); s2tmp[u] = (int16_t) - fpr_rint(t1[u]);
} }
return sqn; if (PQCLEAN_FALCON1024_CLEAN_is_short_half(sqn, s2tmp, logn)) {
memcpy(s2, s2tmp, n * sizeof * s2);
return 1;
}
return 0;
} }
/* /*
@ -984,7 +1016,6 @@ PQCLEAN_FALCON1024_CLEAN_sign_tree(int16_t *sig, shake256_context *rng,
sampler_context spc; sampler_context spc;
samplerZ samp; samplerZ samp;
void *samp_ctx; void *samp_ctx;
uint32_t sqn;
/* /*
* Normal sampling. We use a fast PRNG seeded from our * Normal sampling. We use a fast PRNG seeded from our
@ -1000,17 +1031,8 @@ PQCLEAN_FALCON1024_CLEAN_sign_tree(int16_t *sig, shake256_context *rng,
/* /*
* Do the actual signature. * Do the actual signature.
*/ */
sqn = do_sign_tree(samp, samp_ctx, sig, if (do_sign_tree(samp, samp_ctx, sig,
expanded_key, hm, logn, ftmp); expanded_key, hm, logn, ftmp)) {
/*
* Check that the norm is correct. With our chosen
* acceptance bound, this should almost always be true.
* With a tighter bound, it may happen sometimes that we
* end up with an invalidly large signature, in which
* case we just loop.
*/
if (PQCLEAN_FALCON1024_CLEAN_is_short_half(sqn, sig, logn)) {
break; break;
} }
} }
@ -1039,7 +1061,6 @@ PQCLEAN_FALCON1024_CLEAN_sign_dyn(int16_t *sig, shake256_context *rng,
sampler_context spc; sampler_context spc;
samplerZ samp; samplerZ samp;
void *samp_ctx; void *samp_ctx;
uint32_t sqn;
/* /*
* Normal sampling. We use a fast PRNG seeded from our * Normal sampling. We use a fast PRNG seeded from our
@ -1055,17 +1076,8 @@ PQCLEAN_FALCON1024_CLEAN_sign_dyn(int16_t *sig, shake256_context *rng,
/* /*
* Do the actual signature. * Do the actual signature.
*/ */
sqn = do_sign_dyn(samp, samp_ctx, sig, if (do_sign_dyn(samp, samp_ctx, sig,
f, g, F, G, hm, logn, ftmp); f, g, F, G, hm, logn, ftmp)) {
/*
* Check that the norm is correct. With our chosen
* acceptance bound, this should almost always be true.
* With a tighter bound, it may happen sometimes that we
* end up with an invalidly large signature, in which
* case we just loop.
*/
if (PQCLEAN_FALCON1024_CLEAN_is_short_half(sqn, sig, logn)) {
break; break;
} }
} }

9
crypto_sign/falcon-512/clean/keygen.c
View File

@ -4069,7 +4069,7 @@ PQCLEAN_FALCON512_CLEAN_keygen(shake256_context *rng,
* and Res(g,phi) are not prime to each other. * and Res(g,phi) are not prime to each other.
*/ */
size_t n, u; size_t n, u;
uint16_t *tmp2; uint16_t *h2, *tmp2;
n = MKN(logn); n = MKN(logn);
@ -4176,12 +4176,13 @@ PQCLEAN_FALCON512_CLEAN_keygen(shake256_context *rng,
* fails, we must restart. * fails, we must restart.
*/ */
if (h == NULL) { if (h == NULL) {
h = (uint16_t *)tmp; h2 = (uint16_t *)tmp;
tmp2 = h + n; tmp2 = h2 + n;
} else { } else {
h2 = h;
tmp2 = (uint16_t *)tmp; tmp2 = (uint16_t *)tmp;
} }
if (!PQCLEAN_FALCON512_CLEAN_compute_public(h, f, g, logn, (uint8_t *)tmp2)) { if (!PQCLEAN_FALCON512_CLEAN_compute_public(h2, f, g, logn, (uint8_t *)tmp2)) {
continue; continue;
} }

94
crypto_sign/falcon-512/clean/sign.c
View File

@ -461,15 +461,16 @@ ffSampling_fft(samplerZ samp, void *samp_ctx,
} }
/* /*
* Compute a signature: the signature contains two vectors, s1 and s2; * Compute a signature: the signature contains two vectors, s1 and s2.
* the caller must still check that they comply with the signature * The s1 vector is not returned. The squared norm of (s1,s2) is
* bound, and try again if that is not the case. The s1 vector is not * computed, and if it is short enough, then s2 is returned into the
* returned; instead, its squared norm (saturated) is returned. This * s2[] buffer, and 1 is returned; otherwise, s2[] is untouched and 0 is
* function uses an expanded key. * returned; the caller should then try again. This function uses an
* expanded key.
* *
* tmp[] must have room for at least six polynomials. * tmp[] must have room for at least six polynomials.
*/ */
static uint32_t static int
do_sign_tree(samplerZ samp, void *samp_ctx, int16_t *s2, do_sign_tree(samplerZ samp, void *samp_ctx, int16_t *s2,
const fpr *expanded_key, const fpr *expanded_key,
const uint16_t *hm, const uint16_t *hm,
@ -479,6 +480,7 @@ do_sign_tree(samplerZ samp, void *samp_ctx, int16_t *s2,
const fpr *b00, *b01, *b10, *b11, *tree; const fpr *b00, *b01, *b10, *b11, *tree;
fpr ni; fpr ni;
uint32_t sqn, ng; uint32_t sqn, ng;
int16_t *s2tmp;
n = MKN(logn); n = MKN(logn);
t0 = tmp; t0 = tmp;
@ -550,23 +552,37 @@ do_sign_tree(samplerZ samp, void *samp_ctx, int16_t *s2,
ng |= sqn; ng |= sqn;
} }
sqn |= -(ng >> 31); sqn |= -(ng >> 31);
/*
* With "normal" degrees (e.g. 512 or 1024), it is very
* improbable that the computed vector is not short enough;
* however, it may happen in practice for the very reduced
* versions (e.g. degree 16 or below). In that case, the caller
* will loop, and we must not write anything into s2[] because
* s2[] may overlap with the hashed message hm[] and we need
* hm[] for the next iteration.
*/
s2tmp = (int16_t *)tmp;
for (u = 0; u < n; u ++) { for (u = 0; u < n; u ++) {
s2[u] = (int16_t) - fpr_rint(t1[u]); s2tmp[u] = (int16_t) - fpr_rint(t1[u]);
} }
return sqn; if (PQCLEAN_FALCON512_CLEAN_is_short_half(sqn, s2tmp, logn)) {
memcpy(s2, s2tmp, n * sizeof * s2);
return 1;
}
return 0;
} }
/* /*
* Compute a signature: the signature contains two vectors, s1 and s2; * Compute a signature: the signature contains two vectors, s1 and s2.
* the caller must still check that they comply with the signature * The s1 vector is not returned. The squared norm of (s1,s2) is
* bound, and try again if that is not the case. The s1 vector is not * computed, and if it is short enough, then s2 is returned into the
* returned; instead, its squared norm (saturated) is returned. This * s2[] buffer, and 1 is returned; otherwise, s2[] is untouched and 0 is
* function uses a raw key and recomputes the B0 matrix and LDL tree * returned; the caller should then try again.
* dynamically.
* *
* tmp[] must have room for at least nine polynomials. * tmp[] must have room for at least nine polynomials.
*/ */
static uint32_t static int
do_sign_dyn(samplerZ samp, void *samp_ctx, int16_t *s2, do_sign_dyn(samplerZ samp, void *samp_ctx, int16_t *s2,
const int8_t *f, const int8_t *g, const int8_t *f, const int8_t *g,
const int8_t *F, const int8_t *G, const int8_t *F, const int8_t *G,
@ -576,6 +592,7 @@ do_sign_dyn(samplerZ samp, void *samp_ctx, int16_t *s2,
fpr *b00, *b01, *b10, *b11, *g00, *g01, *g11; fpr *b00, *b01, *b10, *b11, *g00, *g01, *g11;
fpr ni; fpr ni;
uint32_t sqn, ng; uint32_t sqn, ng;
int16_t *s2tmp;
n = MKN(logn); n = MKN(logn);
@ -738,10 +755,25 @@ do_sign_dyn(samplerZ samp, void *samp_ctx, int16_t *s2,
ng |= sqn; ng |= sqn;
} }
sqn |= -(ng >> 31); sqn |= -(ng >> 31);
/*
* With "normal" degrees (e.g. 512 or 1024), it is very
* improbable that the computed vector is not short enough;
* however, it may happen in practice for the very reduced
* versions (e.g. degree 16 or below). In that case, the caller
* will loop, and we must not write anything into s2[] because
* s2[] may overlap with the hashed message hm[] and we need
* hm[] for the next iteration.
*/
s2tmp = (int16_t *)tmp;
for (u = 0; u < n; u ++) { for (u = 0; u < n; u ++) {
s2[u] = (int16_t) - fpr_rint(t1[u]); s2tmp[u] = (int16_t) - fpr_rint(t1[u]);
} }
return sqn; if (PQCLEAN_FALCON512_CLEAN_is_short_half(sqn, s2tmp, logn)) {
memcpy(s2, s2tmp, n * sizeof * s2);
return 1;
}
return 0;
} }
/* /*
@ -984,7 +1016,6 @@ PQCLEAN_FALCON512_CLEAN_sign_tree(int16_t *sig, shake256_context *rng,
sampler_context spc; sampler_context spc;
samplerZ samp; samplerZ samp;
void *samp_ctx; void *samp_ctx;
uint32_t sqn;
/* /*
* Normal sampling. We use a fast PRNG seeded from our * Normal sampling. We use a fast PRNG seeded from our
@ -1000,17 +1031,8 @@ PQCLEAN_FALCON512_CLEAN_sign_tree(int16_t *sig, shake256_context *rng,
/* /*
* Do the actual signature. * Do the actual signature.
*/ */
sqn = do_sign_tree(samp, samp_ctx, sig, if (do_sign_tree(samp, samp_ctx, sig,
expanded_key, hm, logn, ftmp); expanded_key, hm, logn, ftmp)) {
/*
* Check that the norm is correct. With our chosen
* acceptance bound, this should almost always be true.
* With a tighter bound, it may happen sometimes that we
* end up with an invalidly large signature, in which
* case we just loop.
*/
if (PQCLEAN_FALCON512_CLEAN_is_short_half(sqn, sig, logn)) {
break; break;
} }
} }
@ -1039,7 +1061,6 @@ PQCLEAN_FALCON512_CLEAN_sign_dyn(int16_t *sig, shake256_context *rng,
sampler_context spc; sampler_context spc;
samplerZ samp; samplerZ samp;
void *samp_ctx; void *samp_ctx;
uint32_t sqn;
/* /*
* Normal sampling. We use a fast PRNG seeded from our * Normal sampling. We use a fast PRNG seeded from our
@ -1055,17 +1076,8 @@ PQCLEAN_FALCON512_CLEAN_sign_dyn(int16_t *sig, shake256_context *rng,
/* /*
* Do the actual signature. * Do the actual signature.
*/ */
sqn = do_sign_dyn(samp, samp_ctx, sig, if (do_sign_dyn(samp, samp_ctx, sig,
f, g, F, G, hm, logn, ftmp); f, g, F, G, hm, logn, ftmp)) {
/*
* Check that the norm is correct. With our chosen
* acceptance bound, this should almost always be true.
* With a tighter bound, it may happen sometimes that we
* end up with an invalidly large signature, in which
* case we just loop.
*/
if (PQCLEAN_FALCON512_CLEAN_is_short_half(sqn, sig, logn)) {
break; break;
} }
} }