Add SHA256 fingerprint support

PATCH In review Steffan Karger 3 participants 6 messages May 5, 2016 – Sep 17, 2016 1 message not in the archive

Share & tools

Steffan Karger

author

2016-05-05 14:16 UTC #0 lore raw

PATCH 1/1 Add SHA256 fingerprint support

Add SHA256 fingerprint support for both the normal exported fingerprints (tls_digest_n -> tls_digest_sha256_n), as well as for --x509-track.

Also switch to using the SHA256 fingerprint instead of the SHA1 fingerprint internally, in cert_hash_remember() / cert_hash_compare(). And instead of updating an #if 0'd code block that has been disabled since 2009, just remove that.

This should take care of trac #675.

Signed-off-by Steffan Karger <[email protected]...>

---
 src/openvpn/ssl_verify.c         | 50 ++++++++++++++++------------------------
 src/openvpn/ssl_verify.h         |  2 +-
 src/openvpn/ssl_verify_backend.h | 25 +++++++++++++++-----
 src/openvpn/ssl_verify_mbedtls.c | 48 +++++++++++++++++++++++++++++---------
 src/openvpn/ssl_verify_openssl.c | 37 +++++++++++++++++++++--------
 5 files changed, 105 insertions(+), 57 deletions(-)

src/openvpn/ssl_verify.c +20 ~~-30~~

index b373bed..15a5de8 100644
@@ -191,7 +191,8 @@ tls_username (const struct tls_multi *multi, const bool null)
 }
 
 void
-cert_hash_remember (struct tls_session *session, const int error_depth, const unsigned char *sha1_hash)
+cert_hash_remember (struct tls_session *session, const int error_depth,
+    const struct buffer *cert_hash)
 {
   if (error_depth >= 0 && error_depth < MAX_CERT_DEPTH)
     {

@@ -201,31 +202,12 @@ cert_hash_remember (struct tls_session *session, const int error_depth, const un
 	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);
+	ASSERT (sizeof (ch->sha256_hash) == BLEN (cert_hash));
+	memcpy (ch->sha256_hash, BPTR (cert_hash), sizeof (ch->sha256_hash));
       }
     }
 }
 
-#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)
 {

@@ -251,7 +233,8 @@ cert_hash_compare (const struct cert_hash_set *chs1, const struct cert_hash_set
 
 	  if (!ch1 && !ch2)
 	    continue;
-	  else if (ch1 && ch2 && !memcmp (ch1->sha1_hash, ch2->sha1_hash, SHA_DIGEST_LENGTH))
+	  else if (ch1 && ch2 && !memcmp (ch1->sha256_hash, ch2->sha256_hash,
+	      sizeof(ch1->sha256_hash)))
 	    continue;
 	  else
 	    return false;

@@ -278,7 +261,8 @@ cert_hash_copy (const struct cert_hash_set *chs)
 	  if (ch)
 	    {
 	      ALLOC_OBJ (dest->ch[i], struct cert_hash);
-	      memcpy (dest->ch[i]->sha1_hash, ch->sha1_hash, SHA_DIGEST_LENGTH);
+	      memcpy (dest->ch[i]->sha256_hash, ch->sha256_hash,
+		  sizeof(dest->ch[i]->sha256_hash));
 	    }
 	}
     }

@@ -416,13 +400,19 @@ verify_cert_set_env(struct env_set *es, openvpn_x509_cert_t *peer_cert, int cert
   setenv_str (es, envname, common_name);
 #endif
 
-  /* export X509 cert SHA1 fingerprint */
+  /* export X509 cert fingerprints */
   {
-    unsigned char *sha1_hash = x509_get_sha1_hash(peer_cert, &gc);
+    struct buffer sha1 = x509_get_sha1_fingerprint(peer_cert, &gc);
+    struct buffer sha256 = x509_get_sha256_fingerprint(peer_cert, &gc);
 
     openvpn_snprintf (envname, sizeof(envname), "tls_digest_%d", cert_depth);
-    setenv_str (es, envname, format_hex_ex(sha1_hash, SHA_DIGEST_LENGTH, 0, 1,
-					  ":", &gc));
+    setenv_str (es, envname,
+	format_hex_ex(BPTR(&sha1), BLEN(&sha1), 0, 1, ":", &gc));
+
+    openvpn_snprintf (envname, sizeof(envname), "tls_digest_sha256_%d",
+	cert_depth);
+    setenv_str (es, envname,
+	format_hex_ex(BPTR(&sha256), BLEN(&sha256), 0, 1, ":", &gc));
   }
 
   /* export serial number as environmental variable */

@@ -638,8 +628,8 @@ verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int cert_dep
   /* verify level 1 cert, i.e. the CA that signed our leaf cert */
   if (cert_depth == 1 && opt->verify_hash)
     {
-      unsigned char *sha1_hash = x509_get_sha1_hash(cert, &gc);
-      if (memcmp (sha1_hash, opt->verify_hash, SHA_DIGEST_LENGTH))
+      struct buffer sha1_hash = x509_get_sha1_fingerprint(cert, &gc);
+      if (memcmp (BPTR (&sha1_hash), opt->verify_hash, BLEN(&sha1_hash)))
       {
 	msg (D_TLS_ERRORS, "TLS Error: level-1 certificate hash verification failed");
 	goto cleanup;

src/openvpn/ssl_verify.h +1 -1

index f693b2a..e5b5950 100644
@@ -55,7 +55,7 @@
 
 /** Structure containing the hash for a single certificate */
 struct cert_hash {
-  unsigned char sha1_hash[SHA_DIGEST_LENGTH]; /**< The SHA1 hash for a certificate */
+  unsigned char sha256_hash[256/8];
 };
 
 /** Structure containing the hashes for a full certificate chain */

src/openvpn/ssl_verify_backend.h +19 -6

index 9d2057b..91e6ec9 100644
@@ -66,10 +66,10 @@ result_t verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int
  *
  * @param session	TLS Session associated with this tunnel
  * @param cert_depth	Depth of the current certificate
- * @param sha1_hash	Hash of the current certificate
+ * @param cert_hash	Hash of the current certificate
  */
 void cert_hash_remember (struct tls_session *session, const int cert_depth,
-    const unsigned char *sha1_hash);
+    const struct buffer *cert_hash);
 
 /*
  * Library-specific functions.

@@ -87,14 +87,27 @@ void cert_hash_remember (struct tls_session *session, const int cert_depth,
  */
 char *x509_get_subject (openvpn_x509_cert_t *cert, struct gc_arena *gc);
 
-/* Retrieve the certificate's SHA1 hash.
+/**
+ * Retrieve the certificate's SHA1 fingerprint.
  *
- * @param cert		Certificate to retrieve the hash from.
+ * @param cert		Certificate to retrieve the fingerprint from.
  * @param gc		Garbage collection arena to use when allocating string.
  *
- * @return 		a string containing the SHA1 hash of the certificate
+ * @return 		a string containing the certificate fingerprint
  */
-unsigned char *x509_get_sha1_hash (openvpn_x509_cert_t *cert, struct gc_arena *gc);
+struct buffer x509_get_sha1_fingerprint (openvpn_x509_cert_t *cert,
+    struct gc_arena *gc);
+
+/**
+ * Retrieve the certificate's SHA256 fingerprint.
+ *
+ * @param cert		Certificate to retrieve the fingerprint from.
+ * @param gc		Garbage collection arena to use when allocating string.
+ *
+ * @return 		a string containing the certificate fingerprint
+ */
+struct buffer x509_get_sha256_fingerprint (openvpn_x509_cert_t *cert,
+    struct gc_arena *gc);
 
 /*
  * Retrieve the certificate's username from the specified field.

src/openvpn/ssl_verify_mbedtls.c +37 ~~-11~~

68 lines not on this page · raw message

src/openvpn/ssl_verify_openssl.c +28 -9

index 5817a05..a4b9432 100644
@@ -61,8 +61,8 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx)
   session = (struct tls_session *) SSL_get_ex_data (ssl, mydata_index);
   ASSERT (session);
 
-  cert_hash_remember (session, ctx->error_depth,
-      x509_get_sha1_hash(ctx->current_cert, &gc));
+  struct buffer cert_hash = x509_get_sha256_fingerprint(ctx->current_cert, &gc);
+  cert_hash_remember (session, ctx->error_depth, &cert_hash);
 
   /* did peer present cert which was signed by our root cert? */
   if (!preverify_ok)

@@ -248,11 +248,21 @@ backend_x509_get_serial_hex (openvpn_x509_cert_t *cert, struct gc_arena *gc)
   return format_hex_ex(asn1_i->data, asn1_i->length, 0, 1, ":", gc);
 }
 
-unsigned char *
-x509_get_sha1_hash (X509 *cert, struct gc_arena *gc)
+struct buffer
+x509_get_sha1_fingerprint (X509 *cert, struct gc_arena *gc)
 {
-  unsigned char *hash = gc_malloc(SHA_DIGEST_LENGTH, false, gc);
-  memcpy(hash, cert->sha1_hash, SHA_DIGEST_LENGTH);
+  struct buffer hash = alloc_buf_gc(sizeof(cert->sha1_hash), gc);
+  memcpy(BPTR(&hash), cert->sha1_hash, sizeof(cert->sha1_hash));
+  ASSERT (buf_inc_len(&hash, sizeof (cert->sha1_hash)));
+  return hash;
+}
+
+struct buffer
+x509_get_sha256_fingerprint (X509 *cert, struct gc_arena *gc)
+{
+  struct buffer hash = alloc_buf_gc((EVP_sha256())->md_size, gc);
+  X509_digest(cert, EVP_sha256(), BPTR(&hash), NULL);
+  ASSERT (buf_inc_len(&hash, (EVP_sha256())->md_size));
   return hash;
 }

@@ -376,10 +386,19 @@ x509_setenv_track (const struct x509_track *xt, struct env_set *es, const int de
 	  switch (xt->nid)
 	    {
 	    case NID_sha1:
+	    case NID_sha256:
 	      {
-		char *sha1_fingerprint = format_hex_ex(x509->sha1_hash,
-                                 SHA_DIGEST_LENGTH, 0, 1 | FHE_CAPS, ":", &gc);
-		do_setenv_x509(es, xt->name, sha1_fingerprint, depth);
+		struct buffer fp_buf;
+		char *fp_str = NULL;
+
+		if (xt->nid == NID_sha1)
+		  fp_buf = x509_get_sha1_fingerprint(x509, &gc);
+		else
+		  fp_buf = x509_get_sha256_fingerprint(x509, &gc);
+
+		fp_str = format_hex_ex(BPTR(&fp_buf), BLEN(&fp_buf), 0,
+		    1 | FHE_CAPS, ":", &gc);
+		do_setenv_x509(es, xt->name, fp_str, depth);
 	      }
 	      break;
 	    default:

Signature · 1 line

2.7.4

Steffan Karger

author

2016-05-05 20:14 UTC #1 lore raw

PATCH 1/1 Add SHA256 fingerprint support

Add SHA256 fingerprint support for both the normal exported fingerprints (tls_digest_n -> tls_digest_sha256_n), as well as for --x509-track.

This should take care of trac #675.

v2: update openvpn.8 accordingly

Signed-off-by Steffan Karger <[email protected]...>

---
 doc/openvpn.8                    |  5 ++--
 src/openvpn/ssl_verify.c         | 50 ++++++++++++++++------------------------
 src/openvpn/ssl_verify.h         |  2 +-
 src/openvpn/ssl_verify_backend.h | 25 +++++++++++++++-----
 src/openvpn/ssl_verify_mbedtls.c | 48 +++++++++++++++++++++++++++++---------
 src/openvpn/ssl_verify_openssl.c | 37 +++++++++++++++++++++--------
 6 files changed, 107 insertions(+), 60 deletions(-)

doc/openvpn.8 +2 -3

index 7d5dc5b..269803e 100644
@@ -6347,9 +6347,8 @@ Set prior to execution of the
 script.
 .\"*********************************************************
 .TP
-.B tls_digest_{n}
-Contains the certificate SHA1 fingerprint/digest hash value,
-where
+.B tls_digest_{n} / tls_digest_sha256_{n}
+Contains the certificate SHA1 / SHA256 fingerprint, where
 .B n
 is the verification level.  Only set for TLS connections.  Set prior
 to execution of

src/openvpn/ssl_verify.c +20 ~~-30~~

93 lines not on this page · raw message

src/openvpn/ssl_verify.h +1 -1

index f693b2a..e5b5950 100644
@@ -55,7 +55,7 @@
 
 /** Structure containing the hash for a single certificate */
 struct cert_hash {
-  unsigned char sha1_hash[SHA_DIGEST_LENGTH]; /**< The SHA1 hash for a certificate */
+  unsigned char sha256_hash[256/8];
 };
 
 /** Structure containing the hashes for a full certificate chain */

src/openvpn/ssl_verify_backend.h +19 -6

index 9d2057b..91e6ec9 100644
@@ -66,10 +66,10 @@ result_t verify_cert(struct tls_session *session, openvpn_x509_cert_t *cert, int
  *
  * @param session	TLS Session associated with this tunnel
  * @param cert_depth	Depth of the current certificate
- * @param sha1_hash	Hash of the current certificate
+ * @param cert_hash	Hash of the current certificate
  */
 void cert_hash_remember (struct tls_session *session, const int cert_depth,
-    const unsigned char *sha1_hash);
+    const struct buffer *cert_hash);
 
 /*
  * Library-specific functions.

@@ -87,14 +87,27 @@ void cert_hash_remember (struct tls_session *session, const int cert_depth,
  */
 char *x509_get_subject (openvpn_x509_cert_t *cert, struct gc_arena *gc);
 
-/* Retrieve the certificate's SHA1 hash.
+/**
+ * Retrieve the certificate's SHA1 fingerprint.
  *
- * @param cert		Certificate to retrieve the hash from.
+ * @param cert		Certificate to retrieve the fingerprint from.
  * @param gc		Garbage collection arena to use when allocating string.
  *
- * @return 		a string containing the SHA1 hash of the certificate
+ * @return 		a string containing the certificate fingerprint
  */
-unsigned char *x509_get_sha1_hash (openvpn_x509_cert_t *cert, struct gc_arena *gc);
+struct buffer x509_get_sha1_fingerprint (openvpn_x509_cert_t *cert,
+    struct gc_arena *gc);
+
+/**
+ * Retrieve the certificate's SHA256 fingerprint.
+ *
+ * @param cert		Certificate to retrieve the fingerprint from.
+ * @param gc		Garbage collection arena to use when allocating string.
+ *
+ * @return 		a string containing the certificate fingerprint
+ */
+struct buffer x509_get_sha256_fingerprint (openvpn_x509_cert_t *cert,
+    struct gc_arena *gc);
 
 /*
  * Retrieve the certificate's username from the specified field.

src/openvpn/ssl_verify_mbedtls.c +37 ~~-11~~

68 lines not on this page · raw message

src/openvpn/ssl_verify_openssl.c +28 -9

index 5817a05..a4b9432 100644
@@ -61,8 +61,8 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx)
   session = (struct tls_session *) SSL_get_ex_data (ssl, mydata_index);
   ASSERT (session);
 
-  cert_hash_remember (session, ctx->error_depth,
-      x509_get_sha1_hash(ctx->current_cert, &gc));
+  struct buffer cert_hash = x509_get_sha256_fingerprint(ctx->current_cert, &gc);
+  cert_hash_remember (session, ctx->error_depth, &cert_hash);
 
   /* did peer present cert which was signed by our root cert? */
   if (!preverify_ok)

@@ -248,11 +248,21 @@ backend_x509_get_serial_hex (openvpn_x509_cert_t *cert, struct gc_arena *gc)
   return format_hex_ex(asn1_i->data, asn1_i->length, 0, 1, ":", gc);
 }
 
-unsigned char *
-x509_get_sha1_hash (X509 *cert, struct gc_arena *gc)
+struct buffer
+x509_get_sha1_fingerprint (X509 *cert, struct gc_arena *gc)
 {
-  unsigned char *hash = gc_malloc(SHA_DIGEST_LENGTH, false, gc);
-  memcpy(hash, cert->sha1_hash, SHA_DIGEST_LENGTH);
+  struct buffer hash = alloc_buf_gc(sizeof(cert->sha1_hash), gc);
+  memcpy(BPTR(&hash), cert->sha1_hash, sizeof(cert->sha1_hash));
+  ASSERT (buf_inc_len(&hash, sizeof (cert->sha1_hash)));
+  return hash;
+}
+
+struct buffer
+x509_get_sha256_fingerprint (X509 *cert, struct gc_arena *gc)
+{
+  struct buffer hash = alloc_buf_gc((EVP_sha256())->md_size, gc);
+  X509_digest(cert, EVP_sha256(), BPTR(&hash), NULL);
+  ASSERT (buf_inc_len(&hash, (EVP_sha256())->md_size));
   return hash;
 }

@@ -376,10 +386,19 @@ x509_setenv_track (const struct x509_track *xt, struct env_set *es, const int de
 	  switch (xt->nid)
 	    {
 	    case NID_sha1:
+	    case NID_sha256:
 	      {
-		char *sha1_fingerprint = format_hex_ex(x509->sha1_hash,
-                                 SHA_DIGEST_LENGTH, 0, 1 | FHE_CAPS, ":", &gc);
-		do_setenv_x509(es, xt->name, sha1_fingerprint, depth);
+		struct buffer fp_buf;
+		char *fp_str = NULL;
+
+		if (xt->nid == NID_sha1)
+		  fp_buf = x509_get_sha1_fingerprint(x509, &gc);
+		else
+		  fp_buf = x509_get_sha256_fingerprint(x509, &gc);
+
+		fp_str = format_hex_ex(BPTR(&fp_buf), BLEN(&fp_buf), 0,
+		    1 | FHE_CAPS, ":", &gc);
+		do_setenv_x509(es, xt->name, fp_str, depth);
 	      }
 	      break;
 	    default:

Signature · 1 line

2.7.4

A message that is not in the archive (sent off-list or not yet received): [email protected]...

Steffan Karger

author

2016-08-26 07:19 UTC #3 lore raw

Hi,

Quoted 1 line · David Sommerseth

Finally some time to review this :)

Great! You having more time to spend on the codebase is really helping the project move forward.

Quoted 13 lines

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];
        ASSERT (sizeof (ch->sha256_hash) == BLEN (cert_hash));
        memcpy (ch->sha256_hash, BPTR (cert_hash), sizeof (ch->sha256_hash));
      }
    }

Quoted 6 lines

Look careful, the * marked if-line is only doing the ALLOC_OBJ, but it looks like the {} below is part of the if statement at first glance. It actually took me a few reads to see that they were not related. I'm concerned this style can hide bugs in future. Ideally we should avoid such local scopes when we need to declare new variables, especially so close to for/while/if blocks.

I agree that the code is confusing this way, but I'd blame the missing scope of the if statements, rather than the local scope. Do you want me to send a follow-up patch for cleanup, or resend in ta 1/2 cleanup
+ 2/2 SHA256?

Quoted 19 lines

src/openvpn/ssl_verify.c cert_hash_compare (const struct cert_hash_set *chs1, const struct cert_hash_set · line 240
Quoted 13 lines
void
cert_hash_free (struct cert_hash_set *chs)
{
src/openvpn/ssl_verify.c +2 -1
@@ -251,7 +233,8 @@ cert_hash_compare (const struct cert_hash_set *chs1, const struct cert_hash_set
 
        if (!ch1 && !ch2)
          continue;
-       else if (ch1 && ch2 && !memcmp (ch1->sha1_hash, ch2->sha1_hash, SHA_DIGEST_LENGTH))
+       else if (ch1 && ch2 && !memcmp (ch1->sha256_hash, ch2->sha256_hash,
+           sizeof(ch1->sha256_hash)))
          continue;
        else
          return false;
This looks sane. I just got uncertain if we will always have SHA256 hashes from certificates, or do they need be signed with a SHA256
signature by the CA? My gut feeling says me we will always have have
this, as it should just be a hash of the certificate itself as is.

Quoted 2 lines

I have only glared at the code so far, and it looks reasonable. Will test it a bit more thorough in the coming days.

This hash is internal to openvpn, and has nothing to do with the CA signatures. We can use whatever we want. I'm not aware of any reason to assume SHA256 will be broken anytime soon, and if there is some day, we will just switch to SHA-newest ;) (We can't do SHA3 now, because our backends do not support is yet.)

-Steffan

David Sommerseth

2016-08-26 11:08 UTC #4 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 27 lines · Steffan Karger

Hi,
Quoted 1 line · David Sommerseth
Finally some time to review this :)
Great! You having more time to spend on the codebase is really helping the project move forward.
Quoted 10 lines
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]; ASSERT (sizeof (ch->sha256_hash) == BLEN (cert_hash)); memcpy (ch->sha256_hash, BPTR (cert_hash), sizeof (ch->sha256_hash)); } }
Quoted 7 lines
Look careful, the * marked if-line is only doing the ALLOC_OBJ, but it looks like the {} below is part of the if statement at first glance. It actually took me a few reads to see that they were not related. I'm concerned this style can hide bugs in future. Ideally we should avoid such local scopes when we need to declare new variables, especially so close to for/while/if blocks.

Quoted 4 lines

I agree that the code is confusing this way, but I'd blame the missing scope of the if statements, rather than the local scope. Do you want me to send a follow-up patch for cleanup, or resend in ta 1/2 cleanup + 2/2 SHA256?

Depends on how you clean it up ;-) I've been thinking about this, and I think just adding an extra blank line after each if() statement and perhaps adding a comment above the {} local scope would suffice.

But if you do more thorough clean-up, it's probably better to split it up into separate patches.

Does this make sense?

Quoted 17 lines

Quoted 17 lines
Quoted 10 lines
void cert_hash_free (struct cert_hash_set *chs) { @@ -251,7 +233,8 @@ cert_hash_compare (const struct cert_hash_set *chs1, const struct cert_hash_set
if (!ch1 && !ch2) continue; - else if (ch1 && ch2 &&
!memcmp (ch1->sha1_hash, ch2->sha1_hash, SHA_DIGEST_LENGTH)) + else if (ch1 && ch2 && !memcmp (ch1->sha256_hash,
ch2->sha256_hash, + sizeof(ch1->sha256_hash)))
continue; else return false;
This looks sane. I just got uncertain if we will always have SHA256 hashes from certificates, or do they need be signed with a
SHA256 signature by the CA? My gut feeling says me we will
always have have this, as it should just be a hash of the certificate itself as is.

Quoted 8 lines

Quoted 2 lines
I have only glared at the code so far, and it looks reasonable. Will test it a bit more thorough in the coming days.
This hash is internal to openvpn, and has nothing to do with the CA signatures. We can use whatever we want. I'm not aware of any reason to assume SHA256 will be broken anytime soon, and if there is some day, we will just switch to SHA-newest ;) (We can't do SHA3 now, because our backends do not support is yet.)

Ahh, good, then I won't worry about this. We could do SHA512 though, but not sure it's really worth it in this context ;-)

David S.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJXwCNLAAoJEIbPlEyWcf3yuSQQAKyS25Dk8Ipvb2IPIYZvZPgO QOQRYjuq7Nv3AdG9fMq7sEs7AymHP5vNRZUoGR0ev5AbhX6gEysIxvrg+gxuqBDx wUKvplomkqUSxBwYNWTsTcqkW4IpduYNhJdpiXfFN30bRcePBm7HZvkShG9FsgzR hfAHPXH/TD5ConJPB8WiYO2sbkD86o6Ik7Ur6Iu5aI2vWneTFCU/gPOz/oj6Ad07 ERGyBk3DSbyVuQ9BjvCRWZ6m/BSlt4NF86wWkijCbcgNRoZufyO2PAHZ0o59Yzpa ZdaFPl044zV9FHhbSL4Zx09Mw99+DrA333gbbIajWRQGtkjAjIasNb3R7O7R1+Xm E90aUemfQikC4aRG1rkgMCyUGy9H9Ywe7Thpvi72xtxvxRH34XtCFibFfdDuc1KO ufYuyFyAahkarAmhjsV/Dm3Loy6Fedmcv31Ice8heZJ/4GSyexp3M7Aimpo9RIck 9UkJ7lha7ExlpoGnwq15BsARLDhXMC0sjuQ9PCv4ExezofBmHgo+1Vxg7L7DCSia zi3e3PqX81eYgG1uKZJEiTa8lqo11cuNOM3+N9ruWJBLDqZu7YgTa/MCoPDfQXoe x8yFIRwqOSW++7L9ebCwBxaoxY/5ADeVcDxdV7KhYMIgYsvtsLnLRWG6jPrpDGl8 R9jFm+XTNuCYQyfHhlti
=94yV
-----END PGP SIGNATURE-----

Steffan Karger

author

2016-09-16 19:53 UTC #5 lore raw

PATCH 1/1 cleanup cert_hash_remember scoping

Add scopes for the conditional code, remove local scope that's only needed for c89 support (which we dropped).

This patch should be applied after the SHA256 fingerprint support patch.

Signed-off-by Steffan Karger <[email protected]...>

---
 src/openvpn/ssl_verify.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

src/openvpn/ssl_verify.c +10 -7

index 15a5de8..d0c22b8 100644
@@ -197,14 +197,17 @@ cert_hash_remember (struct tls_session *session, const int error_depth,
   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);
+	{
+	  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];
-	ASSERT (sizeof (ch->sha256_hash) == BLEN (cert_hash));
-	memcpy (ch->sha256_hash, BPTR (cert_hash), sizeof (ch->sha256_hash));
-      }
+	{
+	  ALLOC_OBJ (session->cert_hash_set->ch[error_depth], struct cert_hash);
+	}
+
+      struct cert_hash *ch = session->cert_hash_set->ch[error_depth];
+      ASSERT (sizeof (ch->sha256_hash) == BLEN (cert_hash));
+      memcpy (ch->sha256_hash, BPTR (cert_hash), sizeof (ch->sha256_hash));
     }
 }

Signature · 1 line

2.7.4

David Sommerseth

2016-09-17 12:29 UTC #6 lore raw

In reply to Steffan Karger

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

ACK!

Your patch has been applied to the master branch
This patch includes also the clean cert_hash_remember scoping patch.

commit af1e4d26ab65bd71de168ea621ca55d0e40a0bc1

Author: Steffan Karger
Date:   Thu May 5 22:14:07 2016 +0200

     Add SHA256 fingerprint support

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: David Sommerseth <[email protected]...>
     Message-Id: [email protected]...
     Message-Id: [email protected]...
     URL: https://www.mail-archive.com/[email protected]/msg11859.html
     URL: https://www.mail-archive.com/[email protected]/msg12464.html
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJX3TbFAAoJEIbPlEyWcf3yKqYQALRt5SbxcPaDQomeghpigX/v NhZGdfWhyvTPoV3uxr17OPJh+Z8CFExGtJQbIgJD2jO5v6ADO5tRr2xniuQsWP8d YyMgR9YDKxZ/x7P27X/gWsuBKo9DqMaQDU+pEqMftO1iUWu7O1pQRuYXENpotzEj lBh8bbC0K0NUM+rDKREtv54qGgzjlROCwtDVkrAdH10KMxwvL1IHVgF9GR1XyYTB hZ2Tm0IHscZ0EBkdALIT01PBdm/N/DatSyD9gCAWQjUWzZpJDgeXz49x2Rzk4bn+ 9vG42eaAI6F+z6aIimd3C37qQQ/sq6N1k60o8po9yGisavth3UaGfmsQ8/HpE2g9 3yVZ9TmfA4r8kk9eP2M7FrKMW7dZF95z/FXWbsrT3eYa+BZuKvn9ukXP4egtyxRS rp2mqAl01BN0z9kcdzUr+amtAPBdgyfGcvpiAPrhyppV9nmI9FmPBJUppZpFzwdq A5obbdPczJUEzC0bRyIB4lrZyDDbdwWfVH8QvePaVgUpCZY6/01kuWZKAe39CXEu fg8RsFiNKJ/WwVCmUB+LOAcWIW68bAZt9DDJ1p3kDBV9vUV4kwrpF9D9O4APHGXV LvHEf3hU3ksVeTNFyjWBPxN03bw9g3bVqSe9ZSHDPZWLAGVIYCStv0lfyWgssPVW s7Tvc101nT1bFy4xp8fq
=RWXq
-----END PGP SIGNATURE-----