krb5 commit: Simplify PKINIT cert iteration and selection

Greg Hudson ghudson at mit.edu
Thu Mar 23 13:34:00 EDT 2017


https://github.com/krb5/krb5/commit/01b1c0e26252a00f2215408b0e473b84aa0f6a87
commit 01b1c0e26252a00f2215408b0e473b84aa0f6a87
Author: Matt Rogers <mrogers at redhat.com>
Date:   Tue Mar 21 21:24:14 2017 -0400

    Simplify PKINIT cert iteration and selection
    
    Remove the pkinit_cert_handle structures and iteration functions used
    during certificate matching.  Instead, make pkinit_matching.c obtain a
    list of matching data objects from the crypto code, and then select a
    cert based on the index into that list.
    
    Also fix a typo in the name of crypto_retrieve_X509_key_usage().
    
    [ghudson at mit.edu: simplified code]

 src/plugins/preauth/pkinit/pkinit_crypto.h         |   75 ++---
 src/plugins/preauth/pkinit/pkinit_crypto_openssl.c |  373 ++++++++------------
 src/plugins/preauth/pkinit/pkinit_crypto_openssl.h |   19 -
 src/plugins/preauth/pkinit/pkinit_matching.c       |  139 +-------
 4 files changed, 189 insertions(+), 417 deletions(-)

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto.h b/src/plugins/preauth/pkinit/pkinit_crypto.h
index 49b96b8..a0176ac 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto.h
+++ b/src/plugins/preauth/pkinit/pkinit_crypto.h
@@ -96,7 +96,6 @@ typedef struct _pkinit_cert_iter_info *pkinit_cert_iter_handle;
 #define PKINIT_ITER_NO_MORE	0x11111111  /* XXX */
 
 typedef struct _pkinit_cert_matching_data {
-    pkinit_cert_handle ch;  /* cert handle for this certificate */
     char *subject_dn;	    /* rfc2253-style subject name string */
     char *issuer_dn;	    /* rfc2253-style issuer name string */
     unsigned int ku_bits;   /* key usage information */
@@ -458,68 +457,38 @@ krb5_error_code crypto_free_cert_info
 
 
 /*
- * Get number of certificates available after crypto_load_certs()
+ * Get a null-terminated list of certificate matching data objects for the
+ * certificates loaded in id_cryptoctx.
  */
-krb5_error_code crypto_cert_get_count
-	(krb5_context context,				/* IN */
-	pkinit_plg_crypto_context plg_cryptoctx,	/* IN */
-	pkinit_req_crypto_context req_cryptoctx,	/* IN */
-	pkinit_identity_crypto_context id_cryptoctx,	/* IN */
-	int *cert_count);				/* OUT */
-
-/*
- * Begin iteration over the certs loaded in crypto_load_certs()
- */
-krb5_error_code crypto_cert_iteration_begin
-	(krb5_context context,				/* IN */
-	pkinit_plg_crypto_context plg_cryptoctx,	/* IN */
-	pkinit_req_crypto_context req_cryptoctx,	/* IN */
-	pkinit_identity_crypto_context id_cryptoctx,	/* IN */
-	pkinit_cert_iter_handle *iter_handle);		/* OUT */
-
-/*
- * End iteration over the certs loaded in crypto_load_certs()
- */
-krb5_error_code crypto_cert_iteration_end
-	(krb5_context context,				/* IN */
-	pkinit_cert_iter_handle iter_handle);		/* IN */
-
-/*
- * Get next certificate handle
- */
-krb5_error_code crypto_cert_iteration_next
-	(krb5_context context,				/* IN */
-	pkinit_cert_iter_handle iter_handle,		/* IN */
-	pkinit_cert_handle *cert_handle);		/* OUT */
-
-/*
- * Release cert handle
- */
-krb5_error_code crypto_cert_release
-	(krb5_context context,				/* IN */
-	pkinit_cert_handle cert_handle);		/* IN */
+krb5_error_code
+crypto_cert_get_matching_data(krb5_context context,
+			      pkinit_plg_crypto_context plg_cryptoctx,
+			      pkinit_req_crypto_context req_cryptoctx,
+			      pkinit_identity_crypto_context id_cryptoctx,
+			      pkinit_cert_matching_data ***md_out);
 
 /*
- * Get certificate matching information
+ * Free a matching data object.
  */
-krb5_error_code crypto_cert_get_matching_data
-	(krb5_context context,				/* IN */
-	pkinit_cert_handle cert_handle,			/* IN */
-	pkinit_cert_matching_data **ret_data);		/* OUT */
+void
+crypto_cert_free_matching_data(krb5_context context,
+			       pkinit_cert_matching_data *md);
 
 /*
- * Free certificate information
+ * Free a list of matching data objects.
  */
-krb5_error_code crypto_cert_free_matching_data
-	(krb5_context context,				/* IN */
-	pkinit_cert_matching_data *data);		/* IN */
+void
+crypto_cert_free_matching_data_list(krb5_context context,
+				    pkinit_cert_matching_data **matchdata);
 
 /*
- * Make the given certificate "the chosen one"
+ * Choose one of the certificates loaded in idctx to use for PKINIT client
+ * operations.  cred_index must be an index into the array of matching objects
+ * returned by crypto_cert_get_matching_data().
  */
-krb5_error_code crypto_cert_select
-	(krb5_context context,				/* IN */
-	pkinit_cert_matching_data *data);		/* IN */
+krb5_error_code
+crypto_cert_select(krb5_context context, pkinit_identity_crypto_context idctx,
+		   size_t cred_index);
 
 /*
  * Select the default certificate as "the chosen one"
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index c79edae..028ec64 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -4944,135 +4944,15 @@ cleanup:
 }
 
 /*
- * Get number of certificates available after crypto_load_certs()
- */
-krb5_error_code
-crypto_cert_get_count(krb5_context context,
-                      pkinit_plg_crypto_context plg_cryptoctx,
-                      pkinit_req_crypto_context req_cryptoctx,
-                      pkinit_identity_crypto_context id_cryptoctx,
-                      int *cert_count)
-{
-    int count;
-
-    if (id_cryptoctx == NULL || id_cryptoctx->creds[0] == NULL)
-        return EINVAL;
-
-    for (count = 0;
-         count <= MAX_CREDS_ALLOWED && id_cryptoctx->creds[count] != NULL;
-         count++);
-    *cert_count = count;
-    return 0;
-}
-
-
-/*
- * Begin iteration over the certs loaded in crypto_load_certs()
- */
-krb5_error_code
-crypto_cert_iteration_begin(krb5_context context,
-                            pkinit_plg_crypto_context plg_cryptoctx,
-                            pkinit_req_crypto_context req_cryptoctx,
-                            pkinit_identity_crypto_context id_cryptoctx,
-                            pkinit_cert_iter_handle *ih_ret)
-{
-    struct _pkinit_cert_iter_data *id;
-
-    if (id_cryptoctx == NULL || ih_ret == NULL)
-        return EINVAL;
-    if (id_cryptoctx->creds[0] == NULL) /* No cred info available */
-        return ENOENT;
-
-    id = calloc(1, sizeof(*id));
-    if (id == NULL)
-        return ENOMEM;
-    id->magic = ITER_MAGIC;
-    id->plgctx = plg_cryptoctx,
-        id->reqctx = req_cryptoctx,
-        id->idctx = id_cryptoctx;
-    id->index = 0;
-    *ih_ret = (pkinit_cert_iter_handle) id;
-    return 0;
-}
-
-/*
- * End iteration over the certs loaded in crypto_load_certs()
- */
-krb5_error_code
-crypto_cert_iteration_end(krb5_context context,
-                          pkinit_cert_iter_handle ih)
-{
-    struct _pkinit_cert_iter_data *id = (struct _pkinit_cert_iter_data *)ih;
-
-    if (id == NULL || id->magic != ITER_MAGIC)
-        return EINVAL;
-    free(ih);
-    return 0;
-}
-
-/*
- * Get next certificate handle
- */
-krb5_error_code
-crypto_cert_iteration_next(krb5_context context,
-                           pkinit_cert_iter_handle ih,
-                           pkinit_cert_handle *ch_ret)
-{
-    struct _pkinit_cert_iter_data *id = (struct _pkinit_cert_iter_data *)ih;
-    struct _pkinit_cert_data *cd;
-    pkinit_identity_crypto_context id_cryptoctx;
-
-    if (id == NULL || id->magic != ITER_MAGIC)
-        return EINVAL;
-
-    if (ch_ret == NULL)
-        return EINVAL;
-
-    id_cryptoctx = id->idctx;
-    if (id_cryptoctx == NULL)
-        return EINVAL;
-
-    if (id_cryptoctx->creds[id->index] == NULL)
-        return PKINIT_ITER_NO_MORE;
-
-    cd = calloc(1, sizeof(*cd));
-    if (cd == NULL)
-        return ENOMEM;
-
-    cd->magic = CERT_MAGIC;
-    cd->plgctx = id->plgctx;
-    cd->reqctx = id->reqctx;
-    cd->idctx = id->idctx;
-    cd->index = id->index;
-    cd->cred = id_cryptoctx->creds[id->index++];
-    *ch_ret = (pkinit_cert_handle)cd;
-    return 0;
-}
-
-/*
- * Release cert handle
- */
-krb5_error_code
-crypto_cert_release(krb5_context context,
-                    pkinit_cert_handle ch)
-{
-    struct _pkinit_cert_data *cd = (struct _pkinit_cert_data *)ch;
-    if (cd == NULL || cd->magic != CERT_MAGIC)
-        return EINVAL;
-    free(cd);
-    return 0;
-}
-
-/*
  * Get certificate Key Usage and Extended Key Usage
  */
 static krb5_error_code
-crypto_retieve_X509_key_usage(krb5_context context,
-                              pkinit_plg_crypto_context plgcctx,
-                              pkinit_req_crypto_context reqcctx,
-                              X509 *x,
-                              unsigned int *ret_ku_bits,
-                              unsigned int *ret_eku_bits)
+crypto_retrieve_X509_key_usage(krb5_context context,
+                               pkinit_plg_crypto_context plgcctx,
+                               pkinit_req_crypto_context reqcctx,
+                               X509 *x,
+                               unsigned int *ret_ku_bits,
+                               unsigned int *ret_eku_bits)
 {
     krb5_error_code retval = 0;
     int i;
@@ -5171,55 +5051,99 @@ X509_NAME_oneline_ex(X509_NAME * a,
 }
 
 /*
- * Get certificate information
+ * Get number of certificates available after crypto_load_certs()
  */
-krb5_error_code
-crypto_cert_get_matching_data(krb5_context context,
-                              pkinit_cert_handle ch,
-                              pkinit_cert_matching_data **ret_md)
+static krb5_error_code
+crypto_cert_get_count(pkinit_identity_crypto_context id_cryptoctx,
+                      int *cert_count)
 {
-    krb5_error_code retval;
-    pkinit_cert_matching_data *md;
-    krb5_principal *pkinit_sans =NULL, *upn_sans = NULL;
-    struct _pkinit_cert_data *cd = (struct _pkinit_cert_data *)ch;
-    unsigned int i, j;
+    int count;
+
+    *cert_count = 0;
+    if (id_cryptoctx == NULL || id_cryptoctx->creds[0] == NULL)
+        return EINVAL;
+
+    for (count = 0;
+         count <= MAX_CREDS_ALLOWED && id_cryptoctx->creds[count] != NULL;
+         count++);
+    *cert_count = count;
+    return 0;
+}
+
+void
+crypto_cert_free_matching_data(krb5_context context,
+                               pkinit_cert_matching_data *md)
+{
+    int i;
+
+    if (md == NULL)
+        return;
+    free(md->subject_dn);
+    free(md->issuer_dn);
+    for (i = 0; md->sans != NULL && md->sans[i] != NULL; i++)
+        krb5_free_principal(context, md->sans[i]);
+    free(md->sans);
+    free(md);
+}
+
+/*
+ * Free certificate matching data.
+ */
+void
+crypto_cert_free_matching_data_list(krb5_context context,
+                                    pkinit_cert_matching_data **list)
+{
+    int i;
+
+    for (i = 0; list != NULL && list[i] != NULL; i++)
+        crypto_cert_free_matching_data(context, list[i]);
+    free(list);
+}
+
+/*
+ * Get certificate matching data for cert.
+ */
+static krb5_error_code
+get_matching_data(krb5_context context,
+                  pkinit_plg_crypto_context plg_cryptoctx,
+                  pkinit_req_crypto_context req_cryptoctx, X509 *cert,
+                  pkinit_cert_matching_data **md_out)
+{
+    krb5_error_code ret = ENOMEM;
+    pkinit_cert_matching_data *md = NULL;
+    krb5_principal *pkinit_sans = NULL, *upn_sans = NULL;
+    size_t i, j;
     char buf[DN_BUF_LEN];
     unsigned int bufsize = sizeof(buf);
 
-    if (cd == NULL || cd->magic != CERT_MAGIC)
-        return EINVAL;
-    if (ret_md == NULL)
-        return EINVAL;
+    *md_out = NULL;
 
     md = calloc(1, sizeof(*md));
     if (md == NULL)
-        return ENOMEM;
-
-    md->ch = ch;
+        goto cleanup;
 
-    /* get the subject name (in rfc2253 format) */
-    X509_NAME_oneline_ex(X509_get_subject_name(cd->cred->cert),
-                         buf, &bufsize, XN_FLAG_SEP_COMMA_PLUS);
+    /* Get the subject name (in rfc2253 format). */
+    X509_NAME_oneline_ex(X509_get_subject_name(cert), buf, &bufsize,
+                         XN_FLAG_SEP_COMMA_PLUS);
     md->subject_dn = strdup(buf);
     if (md->subject_dn == NULL) {
-        retval = ENOMEM;
+        ret = ENOMEM;
         goto cleanup;
     }
 
-    /* get the issuer name (in rfc2253 format) */
-    X509_NAME_oneline_ex(X509_get_issuer_name(cd->cred->cert),
-                         buf, &bufsize, XN_FLAG_SEP_COMMA_PLUS);
+    /* Get the issuer name (in rfc2253 format). */
+    X509_NAME_oneline_ex(X509_get_issuer_name(cert), buf, &bufsize,
+                         XN_FLAG_SEP_COMMA_PLUS);
     md->issuer_dn = strdup(buf);
     if (md->issuer_dn == NULL) {
-        retval = ENOMEM;
+        ret = ENOMEM;
         goto cleanup;
     }
 
-    /* get the san data */
-    retval = crypto_retrieve_X509_sans(context, cd->plgctx, cd->reqctx,
-                                       cd->cred->cert, &pkinit_sans,
-                                       &upn_sans, NULL);
-    if (retval)
+    /* Get the SAN data. */
+    ret = crypto_retrieve_X509_sans(context, plg_cryptoctx, req_cryptoctx,
+                                    cert, &pkinit_sans, &upn_sans, NULL);
+    if (ret)
         goto cleanup;
 
     j = 0;
@@ -5234,7 +5158,7 @@ crypto_cert_get_matching_data(krb5_context context,
     if (j != 0) {
         md->sans = calloc((size_t)j+1, sizeof(*md->sans));
         if (md->sans == NULL) {
-            retval = ENOMEM;
+            ret = ENOMEM;
             goto cleanup;
         }
         j = 0;
@@ -5252,88 +5176,96 @@ crypto_cert_get_matching_data(krb5_context context,
     } else
         md->sans = NULL;
 
-    /* get the KU and EKU data */
-
-    retval = crypto_retieve_X509_key_usage(context, cd->plgctx, cd->reqctx,
-                                           cd->cred->cert,
-                                           &md->ku_bits, &md->eku_bits);
-    if (retval)
+    /* Get the KU and EKU data. */
+    ret = crypto_retrieve_X509_key_usage(context, plg_cryptoctx,
+                                         req_cryptoctx, cert, &md->ku_bits,
+                                         &md->eku_bits);
+    if (ret)
         goto cleanup;
 
-    *ret_md = md;
-    retval = 0;
+    *md_out = md;
+    md = NULL;
+
 cleanup:
-    if (retval) {
-        if (md)
-            crypto_cert_free_matching_data(context, md);
-    }
-    return retval;
+    crypto_cert_free_matching_data(context, md);
+    return ret;
 }
 
-/*
- * Free certificate information
- */
 krb5_error_code
-crypto_cert_free_matching_data(krb5_context context,
-                               pkinit_cert_matching_data *md)
+crypto_cert_get_matching_data(krb5_context context,
+                              pkinit_plg_crypto_context plg_cryptoctx,
+                              pkinit_req_crypto_context req_cryptoctx,
+                              pkinit_identity_crypto_context id_cryptoctx,
+                              pkinit_cert_matching_data ***md_out)
 {
-    krb5_principal p;
-    int i;
+    krb5_error_code ret;
+    pkinit_cert_matching_data **md_list = NULL;
+    int count, i;
 
-    if (md == NULL)
-        return EINVAL;
-    if (md->subject_dn)
-        free(md->subject_dn);
-    if (md->issuer_dn)
-        free(md->issuer_dn);
-    if (md->sans) {
-        for (i = 0, p = md->sans[i]; p != NULL; p = md->sans[++i])
-            krb5_free_principal(context, p);
-        free(md->sans);
+    ret = crypto_cert_get_count(id_cryptoctx, &count);
+    if (ret)
+        goto cleanup;
+
+    md_list = calloc(count + 1, sizeof(*md_list));
+    if (md_list == NULL) {
+        ret = ENOMEM;
+        goto cleanup;
     }
-    free(md);
-    return 0;
+
+    for (i = 0; i < count; i++) {
+        ret = get_matching_data(context, plg_cryptoctx, req_cryptoctx,
+                                id_cryptoctx->creds[i]->cert, &md_list[i]);
+        if (ret) {
+            pkiDebug("%s: crypto_cert_get_matching_data error %d, %s\n",
+                     __FUNCTION__, ret, error_message(ret));
+            goto cleanup;
+        }
+    }
+
+    *md_out = md_list;
+    md_list = NULL;
+
+cleanup:
+    crypto_cert_free_matching_data_list(context, md_list);
+    return ret;
 }
 
 /*
- * Make this matching certificate "the chosen one"
+ * Set the certificate in idctx->creds[cred_index] as the selected certificate.
  */
 krb5_error_code
-crypto_cert_select(krb5_context context,
-                   pkinit_cert_matching_data *md)
+crypto_cert_select(krb5_context context, pkinit_identity_crypto_context idctx,
+                   size_t cred_index)
 {
-    struct _pkinit_cert_data *cd;
-    if (md == NULL)
-        return EINVAL;
+    pkinit_cred_info ci = NULL;
 
-    cd = (struct _pkinit_cert_data *)md->ch;
-    if (cd == NULL || cd->magic != CERT_MAGIC)
-        return EINVAL;
+    if (cred_index >= MAX_CREDS_ALLOWED || idctx->creds[cred_index] == NULL)
+        return ENOENT;
 
+    ci = idctx->creds[cred_index];
     /* copy the selected cert into our id_cryptoctx */
-    if (cd->idctx->my_certs != NULL) {
-        sk_X509_pop_free(cd->idctx->my_certs, X509_free);
-    }
-    cd->idctx->my_certs = sk_X509_new_null();
-    sk_X509_push(cd->idctx->my_certs, cd->cred->cert);
-    free(cd->idctx->identity);
+    if (idctx->my_certs != NULL)
+        sk_X509_pop_free(idctx->my_certs, X509_free);
+    idctx->my_certs = sk_X509_new_null();
+    sk_X509_push(idctx->my_certs, ci->cert);
+    free(idctx->identity);
     /* hang on to the selected credential name */
-    if (cd->idctx->creds[cd->index]->name != NULL)
-        cd->idctx->identity = strdup(cd->idctx->creds[cd->index]->name);
+    if (ci->name != NULL)
+        idctx->identity = strdup(ci->name);
     else
-        cd->idctx->identity = NULL;
-    cd->idctx->creds[cd->index]->cert = NULL;       /* Don't free it twice */
-    cd->idctx->cert_index = 0;
+        idctx->identity = NULL;
 
-    if (cd->idctx->pkcs11_method != 1) {
-        cd->idctx->my_key = cd->cred->key;
-        cd->idctx->creds[cd->index]->key = NULL;    /* Don't free it twice */
+    ci->cert = NULL;       /* Don't free it twice */
+    idctx->cert_index = 0;
+    if (idctx->pkcs11_method != 1) {
+        idctx->my_key = ci->key;
+        ci->key = NULL;    /* Don't free it twice */
     }
 #ifndef WITHOUT_PKCS11
     else {
-        cd->idctx->cert_id = cd->cred->cert_id;
-        cd->idctx->creds[cd->index]->cert_id = NULL; /* Don't free it twice */
-        cd->idctx->cert_id_len = cd->cred->cert_id_len;
+        idctx->cert_id = ci->cert_id;
+        ci->cert_id = NULL; /* Don't free it twice */
+        idctx->cert_id_len = ci->cert_id_len;
     }
 #endif
     return 0;
@@ -5349,15 +5281,12 @@ crypto_cert_select_default(krb5_context context,
                            pkinit_identity_crypto_context id_cryptoctx)
 {
     krb5_error_code retval;
-    int cert_count = 0;
+    int cert_count;
 
-    retval = crypto_cert_get_count(context, plg_cryptoctx, req_cryptoctx,
-                                   id_cryptoctx, &cert_count);
-    if (retval) {
-        pkiDebug("%s: crypto_cert_get_count error %d, %s\n",
-                 __FUNCTION__, retval, error_message(retval));
+    retval = crypto_cert_get_count(id_cryptoctx, &cert_count);
+    if (retval)
         goto errout;
-    }
+
     if (cert_count != 1) {
         pkiDebug("%s: ERROR: There are %d certs to choose from, "
                  "but there must be exactly one.\n",
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h
index 2fe357c..7411348 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h
@@ -115,23 +115,4 @@ struct _pkinit_req_crypto_context {
     DH *dh;
 };
 
-#define CERT_MAGIC 0x53534c43
-struct _pkinit_cert_data {
-    unsigned int magic;
-    pkinit_plg_crypto_context plgctx;
-    pkinit_req_crypto_context reqctx;
-    pkinit_identity_crypto_context idctx;
-    pkinit_cred_info cred;
-    unsigned int index;	    /* Index of this cred in the creds[] array */
-};
-
-#define ITER_MAGIC 0x53534c49
-struct _pkinit_cert_iter_data {
-    unsigned int magic;
-    pkinit_plg_crypto_context plgctx;
-    pkinit_req_crypto_context reqctx;
-    pkinit_identity_crypto_context idctx;
-    unsigned int index;
-};
-
 #endif	/* _PKINIT_CRYPTO_OPENSSL_H */
diff --git a/src/plugins/preauth/pkinit/pkinit_matching.c b/src/plugins/preauth/pkinit/pkinit_matching.c
index a50c50c..156caf3 100644
--- a/src/plugins/preauth/pkinit/pkinit_matching.c
+++ b/src/plugins/preauth/pkinit/pkinit_matching.c
@@ -544,7 +544,7 @@ check_all_certs(krb5_context context,
                 rule_set *rs,   /* rule to check */
                 pkinit_cert_matching_data **matchdata,
                 int *match_found,
-                pkinit_cert_matching_data **matching_cert)
+                size_t *match_index)
 {
     krb5_error_code retval;
     pkinit_cert_matching_data *md;
@@ -553,12 +553,12 @@ check_all_certs(krb5_context context,
     int total_cert_matches = 0;
     rule_component *rc;
     int certs_checked = 0;
-    pkinit_cert_matching_data *save_match = NULL;
+    size_t save_index = 0;
 
-    if (match_found == NULL || matching_cert == NULL)
+    if (match_found == NULL || match_index == NULL)
         return EINVAL;
 
-    *matching_cert = NULL;
+    *match_index = 0;
     *match_found = 0;
 
     pkiDebug("%s: matching rule relation is %s with %d components\n",
@@ -590,7 +590,7 @@ check_all_certs(krb5_context context,
                 pkiDebug("%s: cert matches rule (OR relation)\n",
                          __FUNCTION__);
                 total_cert_matches++;
-                save_match = md;
+                save_index = i;
                 goto nextcert;
             }
             if (!comp_match && rs->relation == relation_and) {
@@ -602,7 +602,7 @@ check_all_certs(krb5_context context,
         if (rc == NULL && comp_match) {
             pkiDebug("%s: cert matches rule (AND relation)\n", __FUNCTION__);
             total_cert_matches++;
-            save_match = md;
+            save_index = i;
         }
     nextcert:
         continue;
@@ -611,7 +611,7 @@ check_all_certs(krb5_context context,
              __FUNCTION__, certs_checked, total_cert_matches);
     if (total_cert_matches == 1) {
         *match_found = 1;
-        *matching_cert = save_match;
+        *match_index = save_index;
     }
 
     retval = 0;
@@ -621,111 +621,6 @@ check_all_certs(krb5_context context,
     return retval;
 }
 
-static krb5_error_code
-free_all_cert_matching_data(krb5_context context,
-                            pkinit_cert_matching_data **matchdata)
-{
-    krb5_error_code retval;
-    pkinit_cert_matching_data *md;
-    int i;
-
-    if (matchdata == NULL)
-        return EINVAL;
-
-    for (i = 0, md = matchdata[i]; md != NULL; md = matchdata[++i]) {
-        pkinit_cert_handle ch = md->ch;
-        retval = crypto_cert_free_matching_data(context, md);
-        if (retval) {
-            pkiDebug("%s: crypto_cert_free_matching_data error %d, %s\n",
-                     __FUNCTION__, retval, error_message(retval));
-            goto cleanup;
-        }
-        retval = crypto_cert_release(context, ch);
-        if (retval) {
-            pkiDebug("%s: crypto_cert_release error %d, %s\n",
-                     __FUNCTION__, retval, error_message(retval));
-            goto cleanup;
-        }
-    }
-    free(matchdata);
-    retval = 0;
-
-cleanup:
-    return retval;
-}
-
-static krb5_error_code
-obtain_all_cert_matching_data(krb5_context context,
-                              pkinit_plg_crypto_context plg_cryptoctx,
-                              pkinit_req_crypto_context req_cryptoctx,
-                              pkinit_identity_crypto_context id_cryptoctx,
-                              pkinit_cert_matching_data ***all_matching_data)
-{
-    krb5_error_code retval;
-    int i, cert_count;
-    pkinit_cert_iter_handle ih = NULL;
-    pkinit_cert_handle ch;
-    pkinit_cert_matching_data **matchdata = NULL;
-
-    retval = crypto_cert_get_count(context, plg_cryptoctx, req_cryptoctx,
-                                   id_cryptoctx, &cert_count);
-    if (retval) {
-        pkiDebug("%s: crypto_cert_get_count error %d, %s\n",
-                 __FUNCTION__, retval, error_message(retval));
-        goto cleanup;
-    }
-
-    pkiDebug("%s: crypto_cert_get_count says there are %d certs\n",
-             __FUNCTION__, cert_count);
-
-    matchdata = calloc((size_t)cert_count + 1, sizeof(*matchdata));
-    if (matchdata == NULL)
-        return ENOMEM;
-
-    retval = crypto_cert_iteration_begin(context, plg_cryptoctx, req_cryptoctx,
-                                         id_cryptoctx, &ih);
-    if (retval) {
-        pkiDebug("%s: crypto_cert_iteration_begin returned %d, %s\n",
-                 __FUNCTION__, retval, error_message(retval));
-        goto cleanup;
-    }
-
-    for (i = 0; i < cert_count; i++) {
-        retval = crypto_cert_iteration_next(context, ih, &ch);
-        if (retval) {
-            if (retval == PKINIT_ITER_NO_MORE)
-                pkiDebug("%s: We thought there were %d certs, but "
-                         "crypto_cert_iteration_next stopped after %d?\n",
-                         __FUNCTION__, cert_count, i);
-            else
-                pkiDebug("%s: crypto_cert_iteration_next error %d, %s\n",
-                         __FUNCTION__, retval, error_message(retval));
-            goto cleanup;
-        }
-
-        retval = crypto_cert_get_matching_data(context, ch, &matchdata[i]);
-        if (retval) {
-            pkiDebug("%s: crypto_cert_get_matching_data error %d, %s\n",
-                     __FUNCTION__, retval, error_message(retval));
-            goto cleanup;
-        }
-
-    }
-
-    *all_matching_data = matchdata;
-    retval = 0;
-cleanup:
-    if (ih != NULL)
-        crypto_cert_iteration_end(context, ih);
-    if (retval) {
-        if (matchdata != NULL)
-            free_all_cert_matching_data(context, matchdata);
-    }
-    pkiDebug("%s: returning %d, certinfo %p\n",
-             __FUNCTION__, retval, *all_matching_data);
-    return retval;
-}
-
 krb5_error_code
 pkinit_cert_matching(krb5_context context,
                      pkinit_plg_crypto_context plg_cryptoctx,
@@ -740,7 +635,7 @@ pkinit_cert_matching(krb5_context context,
     rule_set *rs = NULL;
     int match_found = 0;
     pkinit_cert_matching_data **matchdata = NULL;
-    pkinit_cert_matching_data *the_matching_cert = NULL;
+    size_t match_index = 0;
 
     /* If no matching rules, select the default cert and we're done */
     pkinit_libdefault_strings(context, krb5_princ_realm(context, princ),
@@ -777,7 +672,7 @@ pkinit_cert_matching(krb5_context context,
          * until we are done.
          */
         if (matchdata == NULL) {
-            retval = obtain_all_cert_matching_data(context, plg_cryptoctx,
+            retval = crypto_cert_get_matching_data(context, plg_cryptoctx,
                                                    req_cryptoctx, id_cryptoctx,
                                                    &matchdata);
             if (retval || matchdata == NULL) {
@@ -790,7 +685,7 @@ pkinit_cert_matching(krb5_context context,
 
         retval = check_all_certs(context, plg_cryptoctx, req_cryptoctx,
                                  id_cryptoctx, princ, rs, matchdata,
-                                 &match_found, &the_matching_cert);
+                                 &match_found, &match_index);
         if (retval) {
             pkiDebug("%s: Error %d, checking certs against rule '%s'\n",
                      __FUNCTION__, retval, rules[x]);
@@ -803,9 +698,9 @@ pkinit_cert_matching(krb5_context context,
         }
     }
 
-    if (match_found && the_matching_cert != NULL) {
+    if (match_found) {
         pkiDebug("%s: Selecting the matching cert!\n", __FUNCTION__);
-        retval = crypto_cert_select(context, the_matching_cert);
+        retval = crypto_cert_select(context, id_cryptoctx, match_index);
         if (retval) {
             pkiDebug("%s: crypto_cert_select error %d, %s\n",
                      __FUNCTION__, retval, error_message(retval));
@@ -817,12 +712,10 @@ pkinit_cert_matching(krb5_context context,
     }
 
     retval = 0;
+
 cleanup:
-    if (rules != NULL)
-        profile_free_list(rules);
-    if (rs != NULL)
-        free_rule_set(context, rs);
-    if (matchdata != NULL)
-        free_all_cert_matching_data(context, matchdata);
+    profile_free_list(rules);
+    free_rule_set(context, rs);
+    crypto_cert_free_matching_data_list(context, matchdata);
     return retval;
 }


More information about the cvs-krb5 mailing list