Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
From:   =?UTF-8?B?T25kcmVqIE1vc27DocSNZWs=?= <omosnacek@gmail.com>
Date:   Mon, 9 Jan 2023 19:35:19 +0100
Message-ID: <CAAUqJDsuFT0XBBkOH_OkhXpEJSaZCLP56-MvBMpde=OS-Dt8VQ@mail.gmail.com>
Subject: [BUG] It is possible to circumvent dh_check_params_length() in the
 generic implementation
To:     linux-crypto@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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