Refactored certificate hash lock checks

Signed-off-by: Adriaan de Jong <dejong@fox-it.com>
Acked-by: James Yonan <james@openvpn.net>
Signed-off-by: David Sommerseth <davids@redhat.com>

5 files changed, 184 insertions, 137 deletions

diff --git a/ssl.c b/ssl.c
index fe1c277..fc9e50f 100644
--- a/ssl.c
+++ b/ssl.c
@@ -297,104 +297,6 @@ ssl_put_auth_challenge (const char *cr_str)
 #endif
 
 /*
- * Cert hash functions
- */
-static void
-cert_hash_remember (struct tls_session *session, const int error_depth, const unsigned char *sha1_hash)
-{
-  if (error_depth >= 0 && error_depth < MAX_CERT_DEPTH)
-    {
-      if (!session->cert_hash_set)
-	ALLOC_OBJ_CLEAR (session->cert_hash_set, struct cert_hash_set);
-      if (!session->cert_hash_set->ch[error_depth])
-	ALLOC_OBJ (session->cert_hash_set->ch[error_depth], struct cert_hash);
-      {
-	struct cert_hash *ch = session->cert_hash_set->ch[error_depth];
-	memcpy (ch->sha1_hash, sha1_hash, SHA_DIGEST_LENGTH);
-      }
-    }
-}
-
-#if 0
-static void
-cert_hash_print (const struct cert_hash_set *chs, int msglevel)
-{
-  struct gc_arena gc = gc_new ();
-  msg (msglevel, "CERT_HASH");
-  if (chs)
-    {
-      int i;
-      for (i = 0; i < MAX_CERT_DEPTH; ++i)
-	{
-	  const struct cert_hash *ch = chs->ch[i];
-	  if (ch)
-	    msg (msglevel, "%d:%s", i, format_hex(ch->sha1_hash, SHA_DIGEST_LENGTH, 0, &gc));
-	}
-    }
-  gc_free (&gc);
-}
-#endif
-
-static void
-cert_hash_free (struct cert_hash_set *chs)
-{
-  if (chs)
-    {
-      int i;
-      for (i = 0; i < MAX_CERT_DEPTH; ++i)
-	free (chs->ch[i]);
-      free (chs);
-    }
-}
-
-static bool
-cert_hash_compare (const struct cert_hash_set *chs1, const struct cert_hash_set *chs2)
-{
-  if (chs1 && chs2)
-    {
-      int i;
-      for (i = 0; i < MAX_CERT_DEPTH; ++i)
-	{
-	  const struct cert_hash *ch1 = chs1->ch[i];
-	  const struct cert_hash *ch2 = chs2->ch[i];
-
-	  if (!ch1 && !ch2)
-	    continue;
-	  else if (ch1 && ch2 && !memcmp (ch1->sha1_hash, ch2->sha1_hash, SHA_DIGEST_LENGTH))
-	    continue;
-	  else
-	    return false;
-	}
-      return true;
-    }
-  else if (!chs1 && !chs2)
-    return true;
-  else
-    return false;
-}
-
-static struct cert_hash_set *
-cert_hash_copy (const struct cert_hash_set *chs)
-{
-  struct cert_hash_set *dest = NULL;
-  if (chs)
-    {
-      int i;
-      ALLOC_OBJ_CLEAR (dest, struct cert_hash_set);
-      for (i = 0; i < MAX_CERT_DEPTH; ++i)
-	{
-	  const struct cert_hash *ch = chs->ch[i];
-	  if (ch)
-	    {
-	      ALLOC_OBJ (dest->ch[i], struct cert_hash);
-	      memcpy (dest->ch[i]->sha1_hash, ch->sha1_hash, SHA_DIGEST_LENGTH);
-	    }
-	}
-    }
-  return dest;
-}
-
-/*
  * Extract a field from an X509 subject name.
  *
  * Example:
@@ -1240,14 +1142,6 @@ tls_lock_common_name (struct tls_multi *multi)
     multi->locked_cn = string_alloc (cn, NULL);
 }
 
-void
-tls_lock_cert_hash_set (struct tls_multi *multi)
-{
-  const struct cert_hash_set *chs = multi->session[TM_ACTIVE].cert_hash_set;
-  if (chs && !multi->locked_cert_hash_set)
-    multi->locked_cert_hash_set = cert_hash_copy (chs);
-}
-
 static bool
 tls_lock_username (struct tls_multi *multi, const char *username)
 {
@@ -3420,20 +3314,6 @@ key_method_2_read (struct buffer *buf, struct tls_multi *multi, struct tls_sessi
 	}
     }
 
-  /* Don't allow the cert hashes to change once they have been locked */
-  if (ks->authenticated && multi->locked_cert_hash_set)
-    {
-      const struct cert_hash_set *chs = session->cert_hash_set;
-      if (chs && !cert_hash_compare (chs, multi->locked_cert_hash_set))
-	{
-	  msg (D_TLS_ERRORS, "TLS Auth Error: TLS object CN=%s client-provided SSL certs unexpectedly changed during mid-session reauth",
-	       session->common_name);
-
-	  /* disable the tunnel */
-	  tls_deauthenticate (multi);
-	}
-    }
-
   /* Perform final authentication checks */
   if (ks->authenticated)
     {
diff --git a/ssl.h b/ssl.h
index 2032955..266c2f2 100644
--- a/ssl.h
+++ b/ssl.h
@@ -218,21 +218,6 @@
  */
 /* #define MEASURE_TLS_HANDSHAKE_STATS */
 
-/*
- * Keep track of certificate hashes at various depths
- */
-
-/* Maximum certificate depth we will allow */
-#define MAX_CERT_DEPTH 16
-
-struct cert_hash {
-  unsigned char sha1_hash[SHA_DIGEST_LENGTH];
-};
-
-struct cert_hash_set {
-  struct cert_hash *ch[MAX_CERT_DEPTH];
-};
-
 #ifdef ENABLE_X509_TRACK
 
 struct x509_track
@@ -540,14 +525,12 @@ const char *tls_common_name (const struct tls_multi* multi, const bool null);
 const char *tls_username(const struct tls_multi *multi, const bool null);
 void tls_set_common_name (struct tls_multi *multi, const char *common_name);
 void tls_lock_common_name (struct tls_multi *multi);
-void tls_lock_cert_hash_set (struct tls_multi *multi);
 
 #define TLS_AUTHENTICATION_SUCCEEDED  0
 #define TLS_AUTHENTICATION_FAILED     1
 #define TLS_AUTHENTICATION_DEFERRED   2
 #define TLS_AUTHENTICATION_UNDEFINED  3
 int tls_authentication_status (struct tls_multi *multi, const int latency);
-void tls_deauthenticate (struct tls_multi *multi);
 
 #ifdef MANAGEMENT_DEF_AUTH
 bool tls_authenticate_key (struct tls_multi *multi, const unsigned int mda_key_id, const bool auth, const char *client_reason);
diff --git a/ssl_verify.c b/ssl_verify.c
index 4d19359..37efafb 100644
--- a/ssl_verify.c
+++ b/ssl_verify.c
@@ -37,9 +37,140 @@
 #include "ssl_verify_openssl.h"
 #endif
 
+static void
+tls_deauthenticate (struct tls_multi *multi)
+{
+  if (multi)
+    {
+      int i, j;
+      for (i = 0; i < TM_SIZE; ++i)
+	for (j = 0; j < KS_SIZE; ++j)
+	  multi->session[i].key[j].authenticated = false;
+    }
+}
+
+
+void
+cert_hash_remember (struct tls_session *session, const int error_depth, const unsigned char *sha1_hash)
+{
+  if (error_depth >= 0 && error_depth < MAX_CERT_DEPTH)
+    {
+      if (!session->cert_hash_set)
+	ALLOC_OBJ_CLEAR (session->cert_hash_set, struct cert_hash_set);
+      if (!session->cert_hash_set->ch[error_depth])
+	ALLOC_OBJ (session->cert_hash_set->ch[error_depth], struct cert_hash);
+      {
+	struct cert_hash *ch = session->cert_hash_set->ch[error_depth];
+	memcpy (ch->sha1_hash, sha1_hash, SHA_DIGEST_LENGTH);
+      }
+    }
+}
+
+#if 0
+static void
+cert_hash_print (const struct cert_hash_set *chs, int msglevel)
+{
+  struct gc_arena gc = gc_new ();
+  msg (msglevel, "CERT_HASH");
+  if (chs)
+    {
+      int i;
+      for (i = 0; i < MAX_CERT_DEPTH; ++i)
+	{
+	  const struct cert_hash *ch = chs->ch[i];
+	  if (ch)
+	    msg (msglevel, "%d:%s", i, format_hex(ch->sha1_hash, SHA_DIGEST_LENGTH, 0, &gc));
+	}
+    }
+  gc_free (&gc);
+}
+#endif
+
+void
+cert_hash_free (struct cert_hash_set *chs)
+{
+  if (chs)
+    {
+      int i;
+      for (i = 0; i < MAX_CERT_DEPTH; ++i)
+	free (chs->ch[i]);
+      free (chs);
+    }
+}
+
+static bool
+cert_hash_compare (const struct cert_hash_set *chs1, const struct cert_hash_set *chs2)
+{
+  if (chs1 && chs2)
+    {
+      int i;
+      for (i = 0; i < MAX_CERT_DEPTH; ++i)
+	{
+	  const struct cert_hash *ch1 = chs1->ch[i];
+	  const struct cert_hash *ch2 = chs2->ch[i];
+
+	  if (!ch1 && !ch2)
+	    continue;
+	  else if (ch1 && ch2 && !memcmp (ch1->sha1_hash, ch2->sha1_hash, SHA_DIGEST_LENGTH))
+	    continue;
+	  else
+	    return false;
+	}
+      return true;
+    }
+  else if (!chs1 && !chs2)
+    return true;
+  else
+    return false;
+}
+
+static struct cert_hash_set *
+cert_hash_copy (const struct cert_hash_set *chs)
+{
+  struct cert_hash_set *dest = NULL;
+  if (chs)
+    {
+      int i;
+      ALLOC_OBJ_CLEAR (dest, struct cert_hash_set);
+      for (i = 0; i < MAX_CERT_DEPTH; ++i)
+	{
+	  const struct cert_hash *ch = chs->ch[i];
+	  if (ch)
+	    {
+	      ALLOC_OBJ (dest->ch[i], struct cert_hash);
+	      memcpy (dest->ch[i]->sha1_hash, ch->sha1_hash, SHA_DIGEST_LENGTH);
+	    }
+	}
+    }
+  return dest;
+}
+void
+tls_lock_cert_hash_set (struct tls_multi *multi)
+{
+  const struct cert_hash_set *chs = multi->session[TM_ACTIVE].cert_hash_set;
+  if (chs && !multi->locked_cert_hash_set)
+    multi->locked_cert_hash_set = cert_hash_copy (chs);
+}
+
+
 void
 verify_final_auth_checks(struct tls_multi *multi, struct tls_session *session)
 {
+
+  /* Don't allow the cert hashes to change once they have been locked */
+  if (multi->locked_cert_hash_set)
+    {
+      const struct cert_hash_set *chs = session->cert_hash_set;
+      if (chs && !cert_hash_compare (chs, multi->locked_cert_hash_set))
+	{
+	  msg (D_TLS_ERRORS, "TLS Auth Error: TLS object CN=%s client-provided SSL certs unexpectedly changed during mid-session reauth",
+	       session->common_name);
+
+	  /* disable the tunnel */
+	  tls_deauthenticate (multi);
+	}
+    }
+
   /* verify --client-config-dir based authentication */
   if (session->opt->client_config_dir_exclusive)
     {
diff --git a/ssl_verify.h b/ssl_verify.h
index 1944a0f..4440acd 100644
--- a/ssl_verify.h
+++ b/ssl_verify.h
@@ -40,6 +40,38 @@
 #include "ssl_verify_openssl.h"
 #endif
 
+/*
+ * Keep track of certificate hashes at various depths
+ */
+
+/** Maximum certificate depth we will allow */
+#define MAX_CERT_DEPTH 16
+
+/** Structure containing the hash for a single certificate */
+struct cert_hash {
+  unsigned char sha1_hash[SHA_DIGEST_LENGTH]; /**< The SHA1 hash for a certificate */
+};
+
+/** Structure containing the hashes for a full certificate chain */
+struct cert_hash_set {
+  struct cert_hash *ch[MAX_CERT_DEPTH]; /**< Array of certificate hashes */
+};
+
+
+/**
+ * Frees the given set of certificate hashes.
+ *
+ * @param chs	The certificate hash set to free.
+ */
+void cert_hash_free (struct cert_hash_set *chs);
+
+/**
+ * Locks the certificate hash set used in the given tunnel
+ *
+ * @param multi	The tunnel to lock
+ */
+void tls_lock_cert_hash_set (struct tls_multi *multi);
+
 /**
  * Perform final authentication checks, including locking of the cn, the allowed
  * certificate hashes, and whether a client config entry exists in the
diff --git a/ssl_verify_backend.h b/ssl_verify_backend.h
index 9dbfd7f..130256c 100644
--- a/ssl_verify_backend.h
+++ b/ssl_verify_backend.h
@@ -30,4 +30,25 @@
 #ifndef SSL_VERIFY_BACKEND_H_
 #define SSL_VERIFY_BACKEND_H_
 
+/*
+ * Backend support functions.
+ *
+ * The following functions are needed by the backend, but defined in the main
+ * file.
+ */
+
+/*
+ * Remember the given certificate hash, allowing the certificate chain to be
+ * locked between sessions.
+ *
+ * Must be called for every certificate in the verification chain, whether it
+ * is valid or not.
+ *
+ * @param session	TLS Session associated with this tunnel
+ * @param cert_depth	Depth of the current certificate
+ * @param sha1_hash	Hash of the current certificate
+ */
+void cert_hash_remember (struct tls_session *session, const int cert_depth,
+    const unsigned char *sha1_hash);
+
 #endif /* SSL_VERIFY_BACKEND_H_ */