Make dh_key_info encoder and decoder symmetric

The dh_key_info encoder expects subjectPublicKey to contain the
contents of a bit string, but the decoder outputs the DER encoding of
the bit string including tag.  The PKINIT client code expects this, so
everything works, but the encoder and decoder should be symmetric.
Change the decoder to process the bit string (adding a bit string
decoding primitive) and modify the PKINIT client code to expect only
the bit string contents.

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@25626 dc483132-0cff-0310-8789-dd5450dbe970

diff --git a/src/lib/krb5/asn.1/asn1_decode.c b/src/lib/krb5/asn.1/asn1_decode.c
index 53e20454950134e17d2b2294727529c78f89e59a..1ded579b810dc1f555aa9b13237edbbbb78be96f 100644 (file)
--- 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 0e14491de6e5fe0d934ffe9b8df64cf145f6feab..7573b4fb7135ace8f85d634825cd147c1b11d7ef 100644 (file)
--- 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 b2471004aa42ba36831e5d4be277d624598176d7..e0c827c4f1829dc9ec4a6d8a3caee1e58c17c6b3 100644 (file)
--- 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 3aa44bc4c888db7780b29b531c1a068154b207e2..8785ffb34abd8f928072b30dcc66cb7496169ea6 100644 (file)
--- 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 2fb506821f26d3921c3770b521d1a685a676adb6..b8ad380c973d87b82cdcaf62d8b840c61188f0c7 100644 (file)
--- 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)