Fix signedness handling for EC signatures

If R or S have the top bit set, we need to prepend a zero byte to prevent
them from being interpreted as negative.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/gnutls.h b/gnutls.h
index 267f83f991f1b604cf1c9346d4657a3838c8e88b..7e40707bc3423a92bb9b08b1d27fa0860b55dbfe 100644 (file)
--- a/gnutls.h
+++ b/gnutls.h
@@ -34,6 +34,15 @@ void release_tpm2_ctx(struct openconnect_info *info);
 int install_tpm2_key(struct openconnect_info *vpninfo, gnutls_privkey_t *pkey, gnutls_datum_t *pkey_sig,
                     unsigned int parent, int emptyauth, gnutls_datum_t *privdata, gnutls_datum_t *pubdata);
 
+/* GnuTLS 3.6.0+ provides this. We have our own for older GnuTLS. There is
+ * also _gnutls_encode_ber_rs_raw() in some older versions, but there were
+ * zero-padding bugs in that, and some of the... less diligently maintained
+ * distributions (like Ubuntu even in 18.04) don't have the fix yet, two
+ * years later. */
+#if GNUTLS_VERSION_NUMBER < 0x030600
+#define gnutls_encode_rs_value oc_gnutls_encode_rs_value
+int oc_gnutls_encode_rs_value(gnutls_datum_t *sig_value, const gnutls_datum_t *r, const gnutls_datum_t *s);
+#endif
 
 char *get_gnutls_cipher(gnutls_session_t session);
 

diff --git a/gnutls_tpm2.c b/gnutls_tpm2.c
index d2f2749f5280dbb6ac796450698ab1a626908466..34b9f0cbac292ee8f5e0095b058fa53243eca11b 100644 (file)
--- a/gnutls_tpm2.c
+++ b/gnutls_tpm2.c
@@ -168,4 +168,60 @@ int load_tpm2_key(struct openconnect_info *vpninfo, gnutls_datum_t *fdata,
        return ret;
 }
 
+#if GNUTLS_VERSION_NUMBER < 0x030600
+static void append_bignum(struct oc_text_buf *sig_der, const gnutls_datum_t *d)
+{
+       unsigned char derlen[2];
+
+       buf_append_bytes(sig_der, "\x02", 1); // INTEGER
+       derlen[0] = d->size;
+       /* If it might be interpreted as negative, prepend a zero */
+       if (d->data[0] >= 0x80) {
+               derlen[0]++;
+               derlen[1] = 0;
+               buf_append_bytes(sig_der, derlen, 2);
+       } else {
+               buf_append_bytes(sig_der, derlen, 1);
+       }
+       buf_append_bytes(sig_der, d->data, d->size);
+}
+
+int oc_gnutls_encode_rs_value(gnutls_datum_t *sig, const gnutls_datum_t *sig_r,
+                             const gnutls_datum_t *sig_s)
+{
+       struct oc_text_buf *sig_der = NULL;
+       /*
+        * Create the DER-encoded SEQUENCE containing R and S:
+        *
+        *      DSASignatureValue ::= SEQUENCE {
+        *        r                   INTEGER,
+        *        s                   INTEGER
+        *      }
+        */
+
+       sig_der = buf_alloc();
+       buf_append_bytes(sig_der, "\x30\x80", 2); // SEQUENCE, indeterminate length
+
+       append_bignum(sig_der, sig_r);
+       append_bignum(sig_der, sig_s);
+
+       /* If the length actually fits in one byte (which it should), do
+        * it that way.  Else, leave it indeterminate and add two
+        * end-of-contents octets to mark the end of the SEQUENCE. */
+       if (!buf_error(sig_der) && sig_der->pos <= 0x80)
+               sig_der->data[1] = sig_der->pos - 2;
+       else {
+               buf_append_bytes(sig_der, "\0\0", 2);
+               if (buf_error(sig_der))
+                       goto out;
+       }
+
+       sig->data = (void *)sig_der->data;
+       sig->size = sig_der->pos;
+       sig_der->data = NULL;
+ out:
+       return buf_free(sig_der);
+}
+#endif /* GnuTLS < 3.6.0 */
+
 #endif /* HAVE_TSS2 */

diff --git a/gnutls_tpm2_esys.c b/gnutls_tpm2_esys.c
index 4d98a5e6372b57b9144e829ee03744eabe9b04b2..438fea8659049692f5744ed6044069b772a1a160 100644 (file)
--- a/gnutls_tpm2_esys.c
+++ b/gnutls_tpm2_esys.c
@@ -405,8 +405,7 @@ static int tpm2_ec_sign_hash_fn(gnutls_privkey_t key, gnutls_sign_algorithm_t al
                                         .hierarchy = TPM2_RH_NULL,
                                         .digest.size = 0 };
        TPMT_SIG_SCHEME inScheme = { .scheme = TPM2_ALG_ECDSA };
-       struct oc_text_buf *sig_der = NULL;
-       unsigned char derlen;
+       gnutls_datum_t sig_r, sig_s;
 
        vpn_progress(vpninfo, PRG_DEBUG,
                     _("TPM2 EC sign function called for %d bytes.\n"),
@@ -450,44 +449,13 @@ static int tpm2_ec_sign_hash_fn(gnutls_privkey_t key, gnutls_sign_algorithm_t al
                goto out;
        }
 
-       /*
-        * Create the DER-encoded SEQUENCE containing R and S:
-        *
-        *      DSASignatureValue ::= SEQUENCE {
-        *        r                   INTEGER,
-        *        s                   INTEGER
-        *      }
-        */
-       sig_der = buf_alloc();
-       buf_append_bytes(sig_der, "\x30\x80", 2); // SEQUENCE, indeterminate length
-       buf_append_bytes(sig_der, "\x02", 1); // INTEGER
-       derlen = tsig->signature.ecdsa.signatureR.size;
-       buf_append_bytes(sig_der, &derlen, 1);
-       buf_append_bytes(sig_der, tsig->signature.ecdsa.signatureR.buffer, tsig->signature.ecdsa.signatureR.size);
-
-       buf_append_bytes(sig_der, "\x02", 1); // INTEGER
-       derlen = tsig->signature.ecdsa.signatureS.size;
-       buf_append_bytes(sig_der, &derlen, 1);
-       buf_append_bytes(sig_der, tsig->signature.ecdsa.signatureS.buffer, tsig->signature.ecdsa.signatureS.size);
-
-       /* If the length actually fits in one byte (which it should), do
-        * it that way.  Else, leave it indeterminate and add two
-        * end-of-contents octets to mark the end of the SEQUENCE. */
-       if (!buf_error(sig_der) && sig_der->pos <= 0x80)
-               sig_der->data[1] = sig_der->pos - 2;
-       else {
-               buf_append_bytes(sig_der, "\0\0", 2);
-               if (buf_error(sig_der))
-                       goto out;
-       }
-
-       sig->data = (void *)sig_der->data;
-       sig->size = sig_der->pos;
-       sig_der->data = NULL;
+       sig_r.data = tsig->signature.ecdsa.signatureR.buffer;
+       sig_r.size = tsig->signature.ecdsa.signatureR.size;
+       sig_s.data = tsig->signature.ecdsa.signatureS.buffer;
+       sig_s.size = tsig->signature.ecdsa.signatureS.size;
 
-       ret = 0;
+       ret = gnutls_encode_rs_value(sig, &sig_r, &sig_s);
  out:
-       buf_free(sig_der);
        free(tsig);
 
        if (key_handle != ESYS_TR_NONE)