5 files changed, 42 insertions, 61 deletions

diff --git a/src/lib/krb5/asn.1/asn1_decode.c b/src/lib/krb5/asn.1/asn1_decode.c
index 53e2045495..1ded579b81 100644
--- a/src/lib/krb5/asn.1/asn1_decode.c
+++ b/src/lib/krb5/asn.1/asn1_decode.c
@@ -197,6 +197,32 @@ asn1_decode_generalstring(asn1buf *buf, unsigned int *retlen, char **val)
     cleanup();
 }
 
+asn1_error_code
+asn1_decode_bitstring(asn1buf *buf, unsigned int *retlen, char **val)
+{
+    setup();
+    asn1_octet unused;
+
+    tag(ASN1_BITSTRING);
+
+    /* Get the number of unused bits in the last byte (0-7). */
+    retval = asn1buf_remove_octet(buf, &unused);
+    if (retval)
+        return retval;
+    if (unused > 7)
+        return ASN1_BAD_FORMAT;
+
+    retval = asn1buf_remove_charstring(buf, length - 1, val);
+    if (retval)
+        return retval;
+
+    /* Mask out unused bits (unnecessary for correct DER, but be safe). */
+    if (length > 1)
+        (*val)[length - 2] &= (0xff << unused);
+
+    *retlen = length - 1;
+    return 0;
+}
 
 asn1_error_code
 asn1_decode_null(asn1buf *buf)
diff --git a/src/lib/krb5/asn.1/asn1_decode.h b/src/lib/krb5/asn.1/asn1_decode.h
index 0e14491de6..7573b4fb71 100644
--- a/src/lib/krb5/asn.1/asn1_decode.h
+++ b/src/lib/krb5/asn.1/asn1_decode.h
@@ -45,6 +45,7 @@
  *  asn1_decode_octetstring
  *  asn1_decode_charstring
  *  asn1_decode_generalstring
+ *  asn1_decode_bitstring
  *  asn1_decode_null
  *  asn1_decode_printablestring
  *  asn1_decode_ia5string
@@ -72,6 +73,8 @@ asn1_error_code asn1_decode_octetstring(asn1buf *buf, unsigned int *retlen,
                                         asn1_octet **val);
 asn1_error_code asn1_decode_generalstring(asn1buf *buf, unsigned int *retlen,
                                           char **val);
+asn1_error_code asn1_decode_bitstring(asn1buf *buf, unsigned int *retlen,
+                                      char **val);
 asn1_error_code asn1_decode_charstring(asn1buf *buf, unsigned int *retlen,
                                        char **val);
 /*
diff --git a/src/lib/krb5/asn.1/asn1_k_decode.c b/src/lib/krb5/asn.1/asn1_k_decode.c
index b2471004aa..e0c827c4f1 100644
--- a/src/lib/krb5/asn.1/asn1_k_decode.c
+++ b/src/lib/krb5/asn.1/asn1_k_decode.c
@@ -1515,11 +1515,12 @@ asn1_decode_kdc_dh_key_info(asn1buf *buf, krb5_kdc_dh_key_info *val)
     setup();
     val->subjectPublicKey.data = NULL;
     { begin_structure();
-        retval = asn1buf_remove_charstring(&subbuf, taglen,
-                                           &val->subjectPublicKey.data);
-        if (retval) clean_return(retval);
-        val->subjectPublicKey.length = taglen;
-        next_tag();
+        /* Special handling for [0] IMPLICIT BIT STRING */
+        error_if_bad_tag(0);
+        if (asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED)
+            clean_return(ASN1_BAD_ID);
+        get_lenfield_body(val->subjectPublicKey.length,
+                          val->subjectPublicKey.data, asn1_decode_bitstring);
         get_field(val->nonce, 1, asn1_decode_int32);
         opt_field(val->dhKeyExpiration, 2, asn1_decode_kerberos_time, 0);
         end_structure();
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_nss.c b/src/plugins/preauth/pkinit/pkinit_crypto_nss.c
index 3aa44bc4c8..8785ffb34a 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_nss.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_nss.c
@@ -950,26 +950,21 @@ secitem_to_dh_pubval(SECItem *item, unsigned char **out, unsigned int *len)
     return i;
 }
 
-/* Decode a bitstring that contains an unsigned integer, and return just the
- * bits that make up that integer. */
+/* Decode a DER unsigned integer, and return just the bits that make up that
+ * integer. */
 static int
 secitem_from_dh_pubval(PLArenaPool *pool,
                        unsigned char *dh_pubkey, unsigned int dh_pubkey_len,
                        SECItem *bits_out)
 {
-    SECItem tmp, uinteger;
+    SECItem tmp;
 
     tmp.data = dh_pubkey;
     tmp.len = dh_pubkey_len;
-    memset(&uinteger, 0, sizeof(uinteger));
-    if (SEC_ASN1DecodeItem(pool, &uinteger,
-                           SEC_ASN1_GET(SEC_BitStringTemplate),
-                           &tmp) != SECSuccess)
-        return ENOMEM;
     memset(bits_out, 0, sizeof(*bits_out));
     if (SEC_ASN1DecodeItem(pool, bits_out,
                            SEC_ASN1_GET(SEC_IntegerTemplate),
-                           &uinteger) != SECSuccess)
+                           &tmp) != SECSuccess)
         return ENOMEM;
     return 0;
 }
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index 2fb506821f..b8ad380c97 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -147,9 +147,6 @@ static krb5_error_code pkinit_decode_data_fs
  unsigned char *data, unsigned int data_len,
  unsigned char **decoded_data, unsigned int *decoded_data_len);
 
-static krb5_error_code der_decode_data
-(unsigned char *, long, unsigned char **, long *);
-
 static krb5_error_code
 create_krb5_invalidCertificates(krb5_context context,
                                 pkinit_plg_crypto_context plg_cryptoctx,
@@ -2647,25 +2644,15 @@ client_process_dh(krb5_context context,
     BIGNUM *server_pub_key = NULL;
     ASN1_INTEGER *pub_key = NULL;
     const unsigned char *p = NULL;
-    unsigned char *data = NULL;
-    long data_len;
-
-    /* decode subjectPublicKey (retrieve INTEGER from OCTET_STRING) */
-
-    if (der_decode_data(subjectPublicKey_data, (long)subjectPublicKey_length,
-                        &data, &data_len) != 0) {
-        pkiDebug("failed to decode subjectPublicKey\n");
-        retval = -1;
-        goto cleanup;
-    }
 
     *client_key_len = DH_size(cryptoctx->dh);
     if ((*client_key = malloc(*client_key_len)) == NULL) {
         retval = ENOMEM;
         goto cleanup;
     }
-    p = data;
-    if ((pub_key = d2i_ASN1_INTEGER(NULL, &p, data_len)) == NULL)
+    p = subjectPublicKey_data;
+    pub_key = d2i_ASN1_INTEGER(NULL, &p, (long)subjectPublicKey_length);
+    if (pub_key == NULL)
         goto cleanup;
     if ((server_pub_key = ASN1_INTEGER_to_BN(pub_key, NULL)) == NULL)
         goto cleanup;
@@ -2682,8 +2669,6 @@ client_process_dh(krb5_context context,
         BN_free(server_pub_key);
     if (pub_key != NULL)
         ASN1_INTEGER_free(pub_key);
-    if (data != NULL)
-        free (data);
 
     return retval;
 
@@ -2692,8 +2677,6 @@ cleanup:
     *client_key = NULL;
     if (pub_key != NULL)
         ASN1_INTEGER_free(pub_key);
-    if (data != NULL)
-        free (data);
 
     return retval;
 }
@@ -5999,33 +5982,6 @@ pkcs7_dataDecode(krb5_context context,
     return(out);
 }
 
-static krb5_error_code
-der_decode_data(unsigned char *data, long data_len,
-                unsigned char **out, long *out_len)
-{
-    krb5_error_code retval = KRB5KDC_ERR_PREAUTH_FAILED;
-    ASN1_OCTET_STRING *s = NULL;
-    const unsigned char *p = data;
-
-    if ((s = d2i_ASN1_BIT_STRING(NULL, &p, data_len)) == NULL)
-        goto cleanup;
-    *out_len = s->length;
-    if ((*out = malloc((size_t) *out_len + 1)) == NULL) {
-        retval = ENOMEM;
-        goto cleanup;
-    }
-    memcpy(*out, s->data, (size_t) s->length);
-    (*out)[s->length] = '\0';
-
-    retval = 0;
-cleanup:
-    if (s != NULL)
-        ASN1_OCTET_STRING_free(s);
-
-    return retval;
-}
-
-
 #ifdef DEBUG_DH
 static void
 print_dh(DH * dh, char *msg)