On 7/17/24 12:17, Lukas Wunner wrote:

Hi Stefan,

On Tue, Mar 16, 2021 at 05:07:32PM -0400, Stefan Berger wrote:

quoted

+/*
+ * 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@gondor.apana.org.au/ (local)

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