Alan DeKok wrote:
Sorry for not responding earlier. A variant of the patch has been added. See the v2.1.x branch on http://git.freeradius.org
The patch creates client/server attributes from the certificate fields. These attributes can be used for anything: policies, *or* logging.
I've been playing around with this and the attached patch fixes a few issues. The issues were: - missing 'i' in Expiration - sprintf in serial number printing was using buf instead of p - array index for subject and issuer storage were reversed I also changed the lookup logic to only log the issueing CA and the client CA, except for when the session is going to fail due to a certificate error. The previous logic only recorded the root CA certificate, which in my opinion isn't as valuable as the issueing CA since most servers will be set up allowing a limited number of root CAs. A few root CAs could expand into a large number of issuing CAs. Thanks, Mike Ross
From ed6d8c770758e38208d522faf73a4c7e31216138 Mon Sep 17 00:00:00 2001 From: Mike Ross <michael.ross2@boeing.com> Date: Mon, 6 Sep 2010 21:51:10 -0700 Subject: [PATCH] Corrections to certificate logging in rlm_eap_tls.c
--- .../rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c | 115 ++++++++++---------- 1 files changed, 59 insertions(+), 56 deletions(-) diff --git a/src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c b/src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c index 6e080d5..65dbdd7 100644 --- a/src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c +++ b/src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c @@ -228,7 +228,7 @@ static SSL_SESSION *cbtls_get_session(UNUSED SSL *s, */ static const char *cert_attr_names[5][2] = { { "TLS-Client-Cert-Serial", "TLS-Cert-Serial" }, - { "TLS-Client-Cert-Expiration", "TLS-Cert-Expiraton" }, + { "TLS-Client-Cert-Expiration", "TLS-Cert-Expiration" }, { "TLS-Client-Cert-Issuer", "TLS-Cert-Issuer" }, { "TLS-Client-Cert-Subject", "TLS-Cert-Subject" }, { "TLS-Client-Cert-Common-Name", "TLS-Cert-Common-Name" } @@ -282,7 +282,7 @@ static int cbtls_verify(int ok, X509_STORE_CTX *ctx) depth = X509_STORE_CTX_get_error_depth(ctx); lookup = depth; - if (lookup > 1) lookup = 1; + if (lookup > 1 && !my_ok) lookup = 1; /* * Retrieve the pointer to the SSL of the connection currently treated @@ -293,66 +293,69 @@ static int cbtls_verify(int ok, X509_STORE_CTX *ctx) request = handler->request; conf = (EAP_TLS_CONF *)SSL_get_ex_data(ssl, 1); - /* - * Get the Serial Number - */ - buf[0] = '\0'; - sn = X509_get_serialNumber(client_cert); - if (sn && (sn->length < (sizeof(buf) / 2))) { - char *p = buf; - int i; - - for (i = 0; i < sn->length; i++) { - sprintf(buf, "%02x", (unsigned int)sn->data[i]); - p += 2; + if ( lookup <= 1 ) { + /* + * Get the Serial Number + */ + buf[0] = '\0'; + sn = X509_get_serialNumber(client_cert); + if (sn && (sn->length < (sizeof(buf) / 2))) { + char *p = buf; + int i; + + for (i = 0; i < sn->length; i++) { + sprintf(p, "%02x", (unsigned int)sn->data[i]); + p += 2; + } + pairadd(&handler->certs, + pairmake(cert_attr_names[0][lookup], buf, T_OP_SET)); } - pairadd(&handler->certs, - pairmake(cert_attr_names[0][lookup], buf, T_OP_SET)); - } - /* - * Get the Expiration Date - */ - buf[0] = '\0'; - asn_time = X509_get_notAfter(client_cert); - if (asn_time && (asn_time->length < MAX_STRING_LEN)) { - memcpy(buf, (char*) asn_time->data, asn_time->length); - buf[asn_time->length] = '\0'; - pairadd(&handler->certs, - pairmake(cert_attr_names[1][lookup], buf, T_OP_SET)); - } + /* + * Get the Expiration Date + */ + buf[0] = '\0'; + asn_time = X509_get_notAfter(client_cert); + if (asn_time && (asn_time->length < MAX_STRING_LEN)) { + memcpy(buf, (char*) asn_time->data, asn_time->length); + buf[asn_time->length] = '\0'; + pairadd(&handler->certs, + pairmake(cert_attr_names[1][lookup], buf, T_OP_SET)); + } - /* - * Get the Subject & Issuer - */ - subject[0] = issuer[0] = '\0'; - X509_NAME_oneline(X509_get_subject_name(client_cert), subject, - sizeof(subject)); - subject[sizeof(subject) - 1] = '\0'; - if (subject[0] && (strlen(subject) < MAX_STRING_LEN)) { - pairadd(&handler->certs, - pairmake(cert_attr_names[2][lookup], subject, T_OP_SET)); - } + /* + * Get the Subject & Issuer + */ + subject[0] = issuer[0] = '\0'; + X509_NAME_oneline(X509_get_subject_name(client_cert), subject, + sizeof(subject)); + subject[sizeof(subject) - 1] = '\0'; + if (subject[0] && (strlen(subject) < MAX_STRING_LEN)) { + pairadd(&handler->certs, + pairmake(cert_attr_names[3][lookup], subject, T_OP_SET)); + } - X509_NAME_oneline(X509_get_issuer_name(ctx->current_cert), issuer, - sizeof(issuer)); - issuer[sizeof(issuer) - 1] = '\0'; - if (issuer[0] && (strlen(issuer) < MAX_STRING_LEN)) { - pairadd(&handler->certs, - pairmake(cert_attr_names[3][lookup], issuer, T_OP_SET)); - } + X509_NAME_oneline(X509_get_issuer_name(ctx->current_cert), issuer, + sizeof(issuer)); + issuer[sizeof(issuer) - 1] = '\0'; + if (issuer[0] && (strlen(issuer) < MAX_STRING_LEN)) { + pairadd(&handler->certs, + pairmake(cert_attr_names[2][lookup], issuer, T_OP_SET)); + } - /* - * Get the Common Name - */ - X509_NAME_get_text_by_NID(X509_get_subject_name(client_cert), - NID_commonName, common_name, sizeof(common_name)); - common_name[sizeof(common_name) - 1] = '\0'; - if (common_name[0] && (strlen(common_name) < MAX_STRING_LEN)) { - pairadd(&handler->certs, - pairmake(cert_attr_names[4][lookup], common_name, T_OP_SET)); - } + /* + * Get the Common Name + */ + X509_NAME_get_text_by_NID(X509_get_subject_name(client_cert), + NID_commonName, common_name, sizeof(common_name)); + common_name[sizeof(common_name) - 1] = '\0'; + if (common_name[0] && (strlen(common_name) < MAX_STRING_LEN)) { + pairadd(&handler->certs, + pairmake(cert_attr_names[4][lookup], common_name, T_OP_SET)); + } + + } /* lookup <= 1 */ if (!my_ok) { const char *p = X509_verify_cert_error_string(err); -- 1.7.0.4