| Message ID | 3323567.LZWGnKmheA@positron.chronox.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add SP800-108 KDF implementation to crypto API | expand |
On Mon, 15 Nov 2021, Stephan Müller wrote: > Remove the specific code that adds a zero padding that was intended > to be invoked when the DH operation result was smaller than the > modulus. However, this cannot occur any more these days because the > function mpi_write_to_sgl is used in the code path that calculates the > shared secret in dh_compute_value. This MPI service function guarantees > that leading zeros are introduced as needed to ensure the resulting data > is exactly as long as the modulus. This implies that the specific code > to add zero padding is dead code which can be safely removed. > > Signed-off-by: Stephan Mueller <smueller@chronox.de> > --- > security/keys/dh.c | 25 ++++--------------------- > 1 file changed, 4 insertions(+), 21 deletions(-) Hi Stephan - Thanks for the cleanup! Acked-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > > diff --git a/security/keys/dh.c b/security/keys/dh.c > index 1abfa70ed6e1..56e12dae4534 100644 > --- a/security/keys/dh.c > +++ b/security/keys/dh.c > @@ -141,7 +141,7 @@ static void kdf_dealloc(struct kdf_sdesc *sdesc) > * 'dlen' must be a multiple of the digest size. > */ > static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen, > - u8 *dst, unsigned int dlen, unsigned int zlen) > + u8 *dst, unsigned int dlen) > { > struct shash_desc *desc = &sdesc->shash; > unsigned int h = crypto_shash_digestsize(desc->tfm); > @@ -158,22 +158,6 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen, > if (err) > goto err; > > - if (zlen && h) { > - u8 tmpbuffer[32]; > - size_t chunk = min_t(size_t, zlen, sizeof(tmpbuffer)); > - memset(tmpbuffer, 0, chunk); > - > - do { > - err = crypto_shash_update(desc, tmpbuffer, > - chunk); > - if (err) > - goto err; > - > - zlen -= chunk; > - chunk = min_t(size_t, zlen, sizeof(tmpbuffer)); > - } while (zlen); > - } > - > if (src && slen) { > err = crypto_shash_update(desc, src, slen); > if (err) > @@ -198,7 +182,7 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen, > > static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc, > char __user *buffer, size_t buflen, > - uint8_t *kbuf, size_t kbuflen, size_t lzero) > + uint8_t *kbuf, size_t kbuflen) > { > uint8_t *outbuf = NULL; > int ret; > @@ -211,7 +195,7 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc, > goto err; > } > > - ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len, lzero); > + ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len); > if (ret) > goto err; > > @@ -384,8 +368,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params, > } > > ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, outbuf, > - req->dst_len + kdfcopy->otherinfolen, > - outlen - req->dst_len); > + req->dst_len + kdfcopy->otherinfolen); > } else if (copy_to_user(buffer, outbuf, req->dst_len) == 0) { > ret = req->dst_len; > } else { > -- > 2.33.1 > > > > > -- Mat Martineau Intel
Am Mittwoch, 17. November 2021, 22:28:46 CET schrieb Mat Martineau: Hi Mat, > On Mon, 15 Nov 2021, Stephan Müller wrote: > > Remove the specific code that adds a zero padding that was intended > > to be invoked when the DH operation result was smaller than the > > modulus. However, this cannot occur any more these days because the > > function mpi_write_to_sgl is used in the code path that calculates the > > shared secret in dh_compute_value. This MPI service function guarantees > > that leading zeros are introduced as needed to ensure the resulting data > > is exactly as long as the modulus. This implies that the specific code > > to add zero padding is dead code which can be safely removed. > > > > Signed-off-by: Stephan Mueller <smueller@chronox.de> > > --- > > security/keys/dh.c | 25 ++++--------------------- > > 1 file changed, 4 insertions(+), 21 deletions(-) > > Hi Stephan - > > Thanks for the cleanup! Thank you for the review. > > Acked-by: Mat Martineau <mathew.j.martineau@linux.intel.com> I have added your signature to the patch. Ciao Stephan
diff --git a/security/keys/dh.c b/security/keys/dh.c index 1abfa70ed6e1..56e12dae4534 100644 --- a/security/keys/dh.c +++ b/security/keys/dh.c @@ -141,7 +141,7 @@ static void kdf_dealloc(struct kdf_sdesc *sdesc) * 'dlen' must be a multiple of the digest size. */ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen, - u8 *dst, unsigned int dlen, unsigned int zlen) + u8 *dst, unsigned int dlen) { struct shash_desc *desc = &sdesc->shash; unsigned int h = crypto_shash_digestsize(desc->tfm); @@ -158,22 +158,6 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen, if (err) goto err; - if (zlen && h) { - u8 tmpbuffer[32]; - size_t chunk = min_t(size_t, zlen, sizeof(tmpbuffer)); - memset(tmpbuffer, 0, chunk); - - do { - err = crypto_shash_update(desc, tmpbuffer, - chunk); - if (err) - goto err; - - zlen -= chunk; - chunk = min_t(size_t, zlen, sizeof(tmpbuffer)); - } while (zlen); - } - if (src && slen) { err = crypto_shash_update(desc, src, slen); if (err) @@ -198,7 +182,7 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen, static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc, char __user *buffer, size_t buflen, - uint8_t *kbuf, size_t kbuflen, size_t lzero) + uint8_t *kbuf, size_t kbuflen) { uint8_t *outbuf = NULL; int ret; @@ -211,7 +195,7 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc, goto err; } - ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len, lzero); + ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len); if (ret) goto err; @@ -384,8 +368,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params, } ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, outbuf, - req->dst_len + kdfcopy->otherinfolen, - outlen - req->dst_len); + req->dst_len + kdfcopy->otherinfolen); } else if (copy_to_user(buffer, outbuf, req->dst_len) == 0) { ret = req->dst_len; } else {
Remove the specific code that adds a zero padding that was intended to be invoked when the DH operation result was smaller than the modulus. However, this cannot occur any more these days because the function mpi_write_to_sgl is used in the code path that calculates the shared secret in dh_compute_value. This MPI service function guarantees that leading zeros are introduced as needed to ensure the resulting data is exactly as long as the modulus. This implies that the specific code to add zero padding is dead code which can be safely removed. Signed-off-by: Stephan Mueller <smueller@chronox.de> --- security/keys/dh.c | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-)