On 7/17/24 12:17, Lukas Wunner wrote:
Hi Stefan, On Tue, Mar 16, 2021 at 05:07:32PM -0400, Stefan Berger wrote:+/* + * Get the r and s components of a signature from the X509 certificate. + */ +static int ecdsa_get_signature_rs(u64 *dest, size_t hdrlen, unsigned char tag, + const void *value, size_t vlen, unsigned int ndigits) +{ + size_t keylen = ndigits * sizeof(u64); + ssize_t diff = vlen - keylen; + const char *d = value; + u8 rs[ECC_MAX_BYTES]; + + if (!value || !vlen) + return -EINVAL; + + /* diff = 0: 'value' has exacly the right size + * diff > 0: 'value' has too many bytes; one leading zero is allowed that + * makes the value a positive integer; error on more + * diff < 0: 'value' is missing leading zeros, which we add + */ + if (diff > 0) { + /* skip over leading zeros that make 'value' a positive int */ + if (*d == 0) { + vlen -= 1; + diff--; + d++; + } + if (diff) + return -EINVAL; + } + if (-diff >= keylen) + return -EINVAL;I'm in the process of creating a crypto_template for decoding an x962 signature as requested by Herbert: https://lore.kernel.org/all/ZoHXyGwRzVvYkcTP@xxxxxxxxxxxxxxxxxxx/ I intend to move the above code to the template and to do so I'm trying to understand what it's doing. There's an oddity in the above-quoted function. The check ... + if (-diff >= keylen) + return -EINVAL; ... seems superfluous. diff is assigned the following value at the top of the function: + ssize_t diff = vlen - keylen; This means that: -diff == keylen - vlen. Now, if vlen is zero, -diff would equal keylen and then the "-diff >= keylen" check would be true. However at the top of the function, there's already a !vlen check. No need to check the same thing again!
You're right, this check is not necessary. Stefan
