Additional EAP-TLS Logging Option

Ross, Michael michael.ross2 at boeing.com
Tue Sep 7 16:53:29 CEST 2010


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 at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Corrections-to-certificate-logging-in-rlm_eap_tls.c.patch
Type: application/octet-stream
Size: 5155 bytes
Desc: 0001-Corrections-to-certificate-logging-in-rlm_eap_tls.c.patch
URL: <http://lists.freeradius.org/pipermail/freeradius-devel/attachments/20100907/1f93900e/attachment.obj>


More information about the Freeradius-Devel mailing list