2023-01-09 18:39:53

by Ondrej Mosnáček

[permalink] [raw]
Subject: [BUG] It is possible to circumvent dh_check_params_length() in the generic implementation

Hello,

While debugging an SELinux test that happens to use keyctl(2) with
KEYCTL_DH_COMPUTE, I noticed that mpi_read_raw_data() (used in
crypto/dh.c to import big numbers from byte strings) accepts any
number of leading zero bytes and discards them. This has a bad
implication for dh_set_params(), as it passes the original prime byte
string size to dh_check_params_length(), which means that it is
possible to get it to accept a prime of any size, simply by
left-padding it with zero bytes to the minimum accepted size.

I ran into this because the test was accidentally passing the prime
byte string with an extra zero byte at the beginning (an artifact from
the output of `openssl dhparam -text -2 2048`) and while this ran fine
when the generic "dh" implementation was used, it was failing on
machines where the intel_qat driver was used, which doesn't have this
validation bug.

For me the fix is simply to drop the extra zero byte from the test's
input, but I wanted to report this, so that someone with more vested
interest in the crypto subsystem can fix the prime length validation
issue as well.

Cheers,

Ondrej