That rule is common to the whole module and not a likely mistake to
make. Also, the test was not really precise as G, I, T were oversized.
Better remove it than give a false sense of security.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
With this data, the loop only settles to its final state u == 0 and v ==
GCD(A, N) at the last iteration. However it already has v == GCD(A, N)
at the previous iteration. Concretely, this means that if in
mbedtls_mpi_core_gcd_modinv_odd() we change the main loop as follows
- for (size_t i = 0; i < (A_limbs + N_limbs) * biL; i++) {
+ for (size_t i = 0; i < (A_limbs + N_limbs) * biL - 2; i++) {
then this test case would fail. Ideally we'd like a test case that would
fail with -1 above but I've not been able to find one and I have no idea
whether that's possible.
Experimentally I've systematically tried small values (8 bit) and
noticed the case A = 2^n and N significantly larger then A is promising,
so I explored that further. Clearly we want A and N's bitlength to be a
multiple of biL because the bound in the paper is with bitlenths while
we use limbs * biL.
Anyway, I ended up with the following Python script.
import secrets
import math
bil = 64
def bitlimbs(x):
return (x.bit_length() + bil - 1) // bil * bil
def sict_gcd(p, a):
assert p >= a >= 0
assert p & 1 != 0 or a & 1 != 0
u, v = a, p
for i in range(2 * p.bit_length()):
s, z = u & 1, v & 1
t1 = (s ^ z) * v + (2 * s * z - 1) * u
t2 = (s * v + (2 - 2 * s - z) * u) >> 1
if t2 >= t1:
u, v = t1, t2
else:
u, v = t2, t1
if u == 0: # v == 1 ideally, but can't get it
return bitlimbs(a) + bitlimbs(p) - (i + 1)
return 0
a = 2 ** (bil - 1)
m = 1000
while m != 0:
n = secrets.randbits(2 * bil) | 1
d = sict_gcd(n, a)
if d < m:
m = d
print(d)
g = math.gcd(a, n)
i = pow(a, -1, n)
print(f'mpi_core_gcd_modinv_odd:"{a:x}":"{n:x}":"{g:x}":"{i:x}"')
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This function has specific code to handle carries and it's not clear how
to exercises that code through the modinv function, so well, that's what
unit tests are for.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This is consistent with the general rules documented at the top of the
file:
- when computing GCD(A, N), there is no modular arithmetic, so the
output can alias any of the inputs;
- when computing a modular inverse, N is the modulus, so it can't be
aliased by any of the outputs (we'll use it for modular operations
over the entire course of the function's execution).
But since this function has two modes of operations with different
aliasing rules (G can alias N only if I == NULL), I think it should
really be stated explicitly.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This is a direct translation of sict_mi2() from
https://github.com/mpg/cryptohack/blob/main/ct-pres.py
which was presented in the book club's special session.
This commit only includes two test cases which is very little. Most of
the test cases will be generated by Python modules that belong to the
framework. However we can't have the framework generate those before we
have the corresponding test function in the consuming branches. So,
extended tests are coming as a 2nd step, after the test function has
been merged.
(The test cases in .misc should stay, as they can be convenient when
working on the test function.)
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
In some cases, we were calling `mbedtls_test_ssl_endpoint_free()` on an
uninitialized `mbedtls_test_ssl_endpoint` object if the test case failed
early, e.g. due to `psa_crypto_init()` failing. This was largely harmless,
but could have caused weird test results in case of failure, and was flagged
by Coverity.
Use a more systematic style for initializing the stack object as soon as
it's declared.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
In some cases, we were calling `mbedtls_test_ssl_endpoint_free()` on an
uninitialized `mbedtls_test_ssl_endpoint` object if the test case failed
early, e.g. due to `psa_crypto_init()` failing. This was largely harmless,
but could have caused weird test results in case of failure, and was flagged
by Coverity.
Use a more systematic style for initializing the stack object as soon as
it's declared.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Now that Base64 validates the number of trailing equals, adjust the PEM test
case that has a Base64 payload with a wrong number of trailing equals, where
`mbedtls_pem_read_buffer()` now returns a different error code. I'm not sure
what the exact intent of the test was, so add a variant with trailing equals
as well.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Correct base64 input (excluding ignored characters such as spaces) consists
of exactly 4*k, 4*k-1 or 4*k-2 digits, followed by 0, 1 or 2 equal signs
respectively.
Previously, any number of trailing equal signs up to 2 was accepted, but if
there fewer than 4*k digits-or-equals, the last partial block was counted in
`*olen` in buffer-too-small mode, but was not output despite returning 0.
Now `mbedtls_base64_decode()` insists on correct padding. This is
backward-compatible since the only plausible useful inputs that used to be
accepted were inputs with 4*k-1 or 4*k-2 digits and no trailing equal signs,
and those led to invalid (truncated) output. Furthermore the function now
always reports the exact output size in buffer-too-small mode.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
