Hi Heinrich, On 2026-10-07T18:50:42, Heinrich Schuchardt <[email protected]> wrote: > lib/rsa: require the modulus size to be multiple of 32 > > In multiple places our RSA code assumes that the key size is a multiple > of 32 bit.
> > E.g in rsa_gen_key_prop() and rsa_mod_exp_sw() we allocate uint32_t[] > buffers for the modulus based on this assumption. Please can you say what goes wrong today with such a key? As I understand it, the 'key.len /= 32' in rsa_mod_exp_sw() truncates, so the modulus and rr buffers end up a word short, and the memcpy() into buf[sig_len / sizeof(uint32_t)] can overflow when sig_len is not a multiple of 4. The patch also adds a second rule, that the top bit of the modulus must be set, which the message doesn't mention. Also 'E.g' should be 'E.g.' and 'bit' should be 'bits'. > > E.g in rsa_gen_key_prop() and rsa_mod_exp_sw() we allocate uint32_t[] > buffers for the modulus based on this assumption. > > Signed-off-by: Heinrich Schuchardt <[email protected]> > > lib/rsa/rsa-keyprop.c | 11 +++++++++++ > lib/rsa/rsa-mod-exp.c | 10 ++++++++++ > 2 files changed, 21 insertions(+) > diff --git a/lib/rsa/rsa-mod-exp.c b/lib/rsa/rsa-mod-exp.c > @@ -283,6 +283,16 @@ int rsa_mod_exp_sw(const uint8_t *sig, uint32_t sig_len, > + /* > + * Our code cannot handle key sizes that are not a multiple of 32. > + * NIST FIPS 186-5, appendix A 1.3 requires the topmost bit to be set. > + * Reject all other key sizes. > + */ > + if (key.len % (sizeof(uint32_t) * 8)) { This comment is copied from rsa_gen_key_prop(), but nothing here checks the top bit. In this path num_bits comes from the 'rsa,num-bits' devicetree property. Please can you drop the FIPS sentence here, or add the check? BTW, since num_bits is now always a multiple of 32, the 'sig_len != num_bits / 8' check in rsa_verify_key() also ensures sig_len is a multiple of 4, which covers the buf[] sizing further down. It might be worth saying so in the commit message. > diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c > @@ -679,6 +679,17 @@ int rsa_gen_key_prop(const void *key, uint32_t keylen, > struct key_prop **prop) > + if (i >= rsa_key.n_sz || > + (rsa_key.n_sz - i) % sizeof(uint32_t) || > + !(rsa_key.n[i] & 0x80)) { > + ret = -EINVAL; > + goto out; > + } The logic looks right and the error path frees everything correctly. If it isn't to hard to set up, could you add a test with an unaligned modulus (e.g. a 2047-bit key) so this doesn't regress? Regards, Simon
