Skip to contentSkip to conversation

Print ec bit details, refuse management-external-key if key is not RSA

PATCH Superseded Arne Schwabe 3 participants 9 messages Jul 23, 2017 – Mar 7, 2018 1 message not in the archive

lore
Share & tools
lore

Arne Schwabe

author
#1 lore raw

PATCH v2 1/1 Print ec bit details, refuse management-external-key if key is not RSA

In reply to a message not in the archive

V2: Print also curve details, add missing ifdef

---
 src/openvpn/ssl_openssl.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
src/openvpn/ssl_openssl.c +31 -0
index 11f4a567..a8e428ea 100644
@@ -1077,6 +1077,13 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
     ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
     pub_rsa = EVP_PKEY_get0_RSA(pkey);
 
+    /* Certificate might not be RSA but DSA or EC */
+    if (!pub_rsa)
+    {
+        crypto_msg (M_FATAL, "management-external-key requires a RSA certificate");
+    }
+        
+
     /* initialize RSA object */
     const BIGNUM *n = NULL;
     const BIGNUM *e = NULL;

@@ -1695,6 +1702,30 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
                                  DSA_bits(dsa));
             }
+#ifndef OPENSSL_NO_EC            
+            else if (EVP_PKEY_id(pkey) == EVP_PKEY_EC && EVP_PKEY_get0_EC_KEY(pkey) != NULL)
+            {
+                EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
+                const EC_GROUP *group = EC_KEY_get0_group(ec);
+                BIO *bio = BIO_new(BIO_s_mem());
+                char ecparamstr[1024];
+
+                CLEAR(ecparamstr);
+
+                ECPKParameters_print(bio, group, 0);
+
+                BIO_read(bio, ecparamstr, sizeof(ecparamstr));
+                /* replace '\n' with ' ' to have a one-line string */
+                for (int i=0; i < sizeof(ecparamstr); i++)
+                    if (ecparamstr[i] == '\n')
+                        ecparamstr[i] = ' ';
+                
+                openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, %s",
+                                 EC_GROUP_order_bits(group), ecparamstr);
+                BIO_free(bio);
+                
+            }
+#endif
             EVP_PKEY_free(pkey);
         }
         X509_free(cert);
Signature · 1 line
2.13.3
#2 lore raw

Hi,

Conceptually fine, but the patch needs a bit more work:

Quoted 10 lines · Arne Schwabe

V2: Print also curve details, add missing ifdef

---
 src/openvpn/ssl_openssl.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
src/openvpn/ssl_openssl.c +0 -0
index 11f4a567..a8e428ea 100644

src/openvpn/ssl_openssl.c tls_ctx_use_external_private_key(struct tls_root_ctx *ctx, · line 1083

Quoted 7 lines
src/openvpn/ssl_openssl.c +4 -0
@@ -1077,6 +1077,13 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
     ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
     pub_rsa = EVP_PKEY_get0_RSA(pkey);
 
+    /* Certificate might not be RSA but DSA or EC */
+    if (!pub_rsa)
+    {
+        crypto_msg (M_FATAL, "management-external-key requires a RSA certificate");

If you make the error non-fatal and add a "goto err;", that will clean up memory nicely.

src/openvpn/ssl_openssl.c tls_ctx_use_external_private_key(struct tls_root_ctx *ctx, · line 1086

Quoted 3 lines
src/openvpn/ssl_openssl.c +3 -0
+    }
+        
+

Looks like one newline too many.

Quoted 6 lines
src/openvpn/ssl_openssl.c +0 -0
     /* initialize RSA object */
     const BIGNUM *n = NULL;
     const BIGNUM *e = NULL;

@@ -1695,6 +1702,30 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
                                  DSA_bits(dsa));

src/openvpn/ssl_openssl.c print_details(struct key_state_ssl *ks_ssl, const char *prefix) · line 1713

Quoted 10 lines
src/openvpn/ssl_openssl.c +9 -0
             }
+#ifndef OPENSSL_NO_EC            
+            else if (EVP_PKEY_id(pkey) == EVP_PKEY_EC && EVP_PKEY_get0_EC_KEY(pkey) != NULL)
+            {
+                EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
+                const EC_GROUP *group = EC_KEY_get0_group(ec);
+                BIO *bio = BIO_new(BIO_s_mem());
+                char ecparamstr[1024];
+
+                CLEAR(ecparamstr);

Just use "char ecparamstr[1024] = { 0 };". Saved us a call to secure_memzero().

src/openvpn/ssl_openssl.c print_details(struct key_state_ssl *ks_ssl, const char *prefix) · line 1721

Quoted 8 lines
src/openvpn/ssl_openssl.c +8 -0
+
+                ECPKParameters_print(bio, group, 0);
+
+                BIO_read(bio, ecparamstr, sizeof(ecparamstr));
+                /* replace '\n' with ' ' to have a one-line string */
+                for (int i=0; i < sizeof(ecparamstr); i++)
+                    if (ecparamstr[i] == '\n')
+                        ecparamstr[i] = ' ';

Please add braces to both the for() and if(). Or consider using EC_GROUP_get_curve_name() + OBJ_nid2sn().

src/openvpn/ssl_openssl.c print_details(struct key_state_ssl *ks_ssl, const char *prefix) · line 1724

Quoted 3 lines
src/openvpn/ssl_openssl.c +3 -0
+                
+                openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, %s",
+                                 EC_GROUP_order_bits(group), ecparamstr);

A number of these functions are OpenSSL 1.1-specific, and will need wrappers in openssl-compat.h to work with OpenSSL 1.0.1 / 1.0.2 (which we still support);

ssl_openssl.o: In function `print_details':
./src/openvpn/ssl_openssl.c:1704: undefined reference to EVP_PKEY_get0_EC_KEY' ./src/openvpn/ssl_openssl.c:1706: undefined reference to EVP_PKEY_get0_EC_KEY' ./src/openvpn/ssl_openssl.c:1721: undefined reference to `EC_GROUP_order_bits'

src/openvpn/ssl_openssl.c print_details(struct key_state_ssl *ks_ssl, const char *prefix) · line 1731

Quoted 8 lines
src/openvpn/ssl_openssl.c +4 -0
+                BIO_free(bio);
+                
+            }
+#endif
             EVP_PKEY_free(pkey);
         }
         X509_free(cert);

-Steffan

author
#3 lore raw

PATCH v3 1/1 Print ec bit details, refuse management-external-key if key is not RSA

In reply to a message not in the archive

V2: Print also curve details, add missing ifdef
V3: Goto err instead of using M_FATAL, format fixes, use EC_GROUP_get_curve_name + OBJ_nid2sn instead of ECPKParameters_print, add compat headers for 1.0.2

---
 configure.ac                 |  2 ++
 src/openvpn/openssl_compat.h | 34 ++++++++++++++++++++++++++++++++++
 src/openvpn/ssl_openssl.c    | 29 +++++++++++++++++++++++++++--
 3 files changed, 63 insertions(+), 2 deletions(-)
configure.ac +2 -0
index 39d992c0..65084a8b 100644
@@ -934,6 +934,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 			EVP_PKEY_id \
 			EVP_PKEY_get0_RSA \
 			EVP_PKEY_get0_DSA \
+            EVP_PKEY_get0_EC_KEY \
 			RSA_set_flags \
 			RSA_bits \
 			RSA_get0_key \

@@ -949,6 +950,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 			RSA_meth_set_init \
 			RSA_meth_set_finish \
 			RSA_meth_set0_app_data \
+            EC_GROUP_order_bits
 		]
 	)
src/openvpn/openssl_compat.h +34 -0
index 36f68b01..68e43510 100644
@@ -244,6 +244,22 @@ EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
 }
 #endif
 
+
+#if !defined(HAVE_EVP_PKEY_GET0_EC_KEY) && !defined(OPENSSL_NO_EC)
+/**
+ * Get the EC_KEY object of a public key
+ *
+ * @param pkey                Public key object
+ * @return                    The underlying EC_KEY object
+ */
+static inline EC_KEY *
+EVP_PKEY_get0_EC_KEY(EVP_PKEY *pkey)
+{
+    return pkey ? pkey->pkey.ec : NULL;
+}
+#endif
+
+
 #if !defined(HAVE_EVP_PKEY_ID)
 /**
  * Get the PKEY type

@@ -610,6 +626,24 @@ RSA_meth_set0_app_data(RSA_METHOD *meth, void *app_data)
 }
 #endif
 
+#if !defined(HAVE_EC_GROUP_ORDER_BITS) && !defined(OPENSSL_NO_EC)
+/**
+ * Gets the number of bits of the order of an EC_GROUP
+ *
+ *  @param  group               EC_GROUP object
+ *  @return                     number of bits of group order.
+ */
+static inline int
+EC_GROUP_order_bits(const EC_GROUP *group)
+{
+    BIGNUM* order = BN_new();
+    EC_GROUP_get_order(group, order, NULL);
+    int bits = BN_num_bits(order);
+    BN_free(order);
+    return bits;
+}
+#endif
+
 /* SSLeay symbols have been renamed in OpenSSL 1.1 */
 #if !defined(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT)
 #define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT       RSA_F_RSA_EAY_PRIVATE_ENCRYPT
src/openvpn/ssl_openssl.c +27 -2
index 11f4a567..27d233ee 100644
@@ -1077,6 +1077,13 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
     ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
     pub_rsa = EVP_PKEY_get0_RSA(pkey);
 
+    /* Certificate might not be RSA but DSA or EC */
+    if (!pub_rsa)
+    {
+        crypto_msg(M_ERR, "management-external-key requires a RSA certificate");
+        goto err;
+    }
+        
     /* initialize RSA object */
     const BIGNUM *n = NULL;
     const BIGNUM *e = NULL;

@@ -1683,18 +1690,36 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
         EVP_PKEY *pkey = X509_get_pubkey(cert);
         if (pkey != NULL)
         {
-            if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) != NULL)
+            if ((EVP_PKEY_id(pkey) == EVP_PKEY_RSA) && (EVP_PKEY_get0_RSA(pkey) != NULL))
             {
                 RSA *rsa = EVP_PKEY_get0_RSA(pkey);
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
                                  RSA_bits(rsa));
             }
-            else if (EVP_PKEY_id(pkey) == EVP_PKEY_DSA && EVP_PKEY_get0_DSA(pkey) != NULL)
+            else if ((EVP_PKEY_id(pkey) == EVP_PKEY_DSA) && (EVP_PKEY_get0_DSA(pkey) != NULL))
             {
                 DSA *dsa = EVP_PKEY_get0_DSA(pkey);
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
                                  DSA_bits(dsa));
             }
+#ifndef OPENSSL_NO_EC
+            else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) && (EVP_PKEY_get0_EC_KEY(pkey) != NULL))
+            {
+                EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
+                const EC_GROUP *group = EC_KEY_get0_group(ec);
+                const char* curve;
+                
+                int nid = EC_GROUP_get_curve_name(group);
+                if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL)
+                {
+                    curve ="Error getting curve name";
+                }
+                
+                openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, curve: %s",
+                                 EC_GROUP_order_bits(group), curve);
+                
+            }
+#endif
             EVP_PKEY_free(pkey);
         }
         X509_free(cert);
Signature · 1 line
2.13.3
#4 lore raw

Hi,

Thanks. This now compiles and works as expected with both OpenSSL 1.0 and 1.1. Some final nits though:

Quoted 21 lines · Arne Schwabe

V2: Print also curve details, add missing ifdef
V3: Goto err instead of using M_FATAL, format fixes, use EC_GROUP_get_curve_name + OBJ_nid2sn instead of ECPKParameters_print, add compat headers for 1.0.2

---
 configure.ac                 |  2 ++
 src/openvpn/openssl_compat.h | 34 ++++++++++++++++++++++++++++++++++
 src/openvpn/ssl_openssl.c    | 29 +++++++++++++++++++++++++++--
 3 files changed, 63 insertions(+), 2 deletions(-)
configure.ac +1 -0
index 39d992c0..65084a8b 100644
@@ -934,6 +934,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 			EVP_PKEY_id \
 			EVP_PKEY_get0_RSA \
 			EVP_PKEY_get0_DSA \
+            EVP_PKEY_get0_EC_KEY \
 			RSA_set_flags \
 			RSA_bits \
 			RSA_get0_key \

configure.ac if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then · line 953

Quoted 4 lines
configure.ac +1 -0
@@ -949,6 +950,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 			RSA_meth_set_init \
 			RSA_meth_set_finish \
 			RSA_meth_set0_app_data \
+            EC_GROUP_order_bits

configure.ac should be indented with tabs, not spaces.

Quoted 17 lines
configure.ac +0 -0
 		]
 	)
src/openvpn/openssl_compat.h +6 -0
index 36f68b01..68e43510 100644
@@ -244,6 +244,22 @@ EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
 }
 #endif
 
+
+#if !defined(HAVE_EVP_PKEY_GET0_EC_KEY) && !defined(OPENSSL_NO_EC)
+/**
+ * Get the EC_KEY object of a public key
+ *
+ * @param pkey                Public key object

src/openvpn/openssl_compat.h EVP_PKEY_get0_RSA(EVP_PKEY *pkey) · line 262

Quoted 10 lines
src/openvpn/openssl_compat.h +10 -0
+ * @return                    The underlying EC_KEY object
+ */
+static inline EC_KEY *
+EVP_PKEY_get0_EC_KEY(EVP_PKEY *pkey)
+{
+    return pkey ? pkey->pkey.ec : NULL;
+}
+#endif
+
+

All other helper function have just 1 newline between them, let's do that here too.

Quoted 33 lines
src/openvpn/openssl_compat.h +18 -0
 #if !defined(HAVE_EVP_PKEY_ID)
 /**
  * Get the PKEY type

@@ -610,6 +626,24 @@ RSA_meth_set0_app_data(RSA_METHOD *meth, void *app_data)
 }
 #endif
 
+#if !defined(HAVE_EC_GROUP_ORDER_BITS) && !defined(OPENSSL_NO_EC)
+/**
+ * Gets the number of bits of the order of an EC_GROUP
+ *
+ *  @param  group               EC_GROUP object
+ *  @return                     number of bits of group order.
+ */
+static inline int
+EC_GROUP_order_bits(const EC_GROUP *group)
+{
+    BIGNUM* order = BN_new();
+    EC_GROUP_get_order(group, order, NULL);
+    int bits = BN_num_bits(order);
+    BN_free(order);
+    return bits;
+}
+#endif
+
 /* SSLeay symbols have been renamed in OpenSSL 1.1 */
 #if !defined(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT)
 #define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT       RSA_F_RSA_EAY_PRIVATE_ENCRYPT
src/openvpn/ssl_openssl.c +0 -0
index 11f4a567..27d233ee 100644

src/openvpn/ssl_openssl.c tls_ctx_use_external_private_key(struct tls_root_ctx *ctx, · line 1083

Quoted 7 lines
src/openvpn/ssl_openssl.c +4 -0
@@ -1077,6 +1077,13 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
     ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
     pub_rsa = EVP_PKEY_get0_RSA(pkey);
 
+    /* Certificate might not be RSA but DSA or EC */
+    if (!pub_rsa)
+    {
+        crypto_msg(M_ERR, "management-external-key requires a RSA certificate");

M_ERR still carries the M_FATAL flag, and we do not expect an meaningful errno here, do we? Let's just use M_WARN.

Quoted 25 lines
src/openvpn/ssl_openssl.c +6 -2
+        goto err;
+    }
+        
     /* initialize RSA object */
     const BIGNUM *n = NULL;
     const BIGNUM *e = NULL;

@@ -1683,18 +1690,36 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
         EVP_PKEY *pkey = X509_get_pubkey(cert);
         if (pkey != NULL)
         {
-            if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) != NULL)
+            if ((EVP_PKEY_id(pkey) == EVP_PKEY_RSA) && (EVP_PKEY_get0_RSA(pkey) != NULL))
             {
                 RSA *rsa = EVP_PKEY_get0_RSA(pkey);
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
                                  RSA_bits(rsa));
             }
-            else if (EVP_PKEY_id(pkey) == EVP_PKEY_DSA && EVP_PKEY_get0_DSA(pkey) != NULL)
+            else if ((EVP_PKEY_id(pkey) == EVP_PKEY_DSA) && (EVP_PKEY_get0_DSA(pkey) != NULL))
             {
                 DSA *dsa = EVP_PKEY_get0_DSA(pkey);
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
                                  DSA_bits(dsa));
             }
+#ifndef OPENSSL_NO_EC

src/openvpn/ssl_openssl.c print_details(struct key_state_ssl *ks_ssl, const char *prefix) · line 1715

Quoted 10 lines
src/openvpn/ssl_openssl.c +10 -0
+            else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) && (EVP_PKEY_get0_EC_KEY(pkey) != NULL))
+            {
+                EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
+                const EC_GROUP *group = EC_KEY_get0_group(ec);
+                const char* curve;
+                
+                int nid = EC_GROUP_get_curve_name(group);
+                if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL)
+                {
+                    curve ="Error getting curve name";

Can I get an extra space after the = ?

src/openvpn/ssl_openssl.c print_details(struct key_state_ssl *ks_ssl, const char *prefix) · line 1725

Quoted 11 lines
src/openvpn/ssl_openssl.c +7 -0
+                }
+                
+                openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, curve: %s",
+                                 EC_GROUP_order_bits(group), curve);
+                
+            }
+#endif
             EVP_PKEY_free(pkey);
         }
         X509_free(cert);

Thanks,
-Steffan

author
#5 lore raw

PATCH v4 1/1 Print ec bit details, refuse management-external-key if key is not RSA

In reply to a message not in the archive

V2: Print also curve details, add missing ifdef
V3: Goto err instead of using M_FATAL, format fixes, use EC_GROUP_get_curve_name + OBJ_nid2sn instead of ECPKParameters_print, add compat headers for 1.0.2
V4: Formatting changes and change M_ERR to M_WARN

---
 configure.ac                 |  2 ++
 src/openvpn/openssl_compat.h | 32 ++++++++++++++++++++++++++++++++
 src/openvpn/ssl_openssl.c    | 29 +++++++++++++++++++++++++++--
 3 files changed, 61 insertions(+), 2 deletions(-)
configure.ac +2 -0
index 39d992c0..f6eeb40d 100644
@@ -934,6 +934,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 			EVP_PKEY_id \
 			EVP_PKEY_get0_RSA \
 			EVP_PKEY_get0_DSA \
+			EVP_PKEY_get0_EC_KEY \
 			RSA_set_flags \
 			RSA_bits \
 			RSA_get0_key \

@@ -949,6 +950,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 			RSA_meth_set_init \
 			RSA_meth_set_finish \
 			RSA_meth_set0_app_data \
+			EC_GROUP_order_bits
 		]
 	)
src/openvpn/openssl_compat.h +32 -0
index 36f68b01..70b19aea 100644
@@ -244,6 +244,20 @@ EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
 }
 #endif
 
+#if !defined(HAVE_EVP_PKEY_GET0_EC_KEY) && !defined(OPENSSL_NO_EC)
+/**
+ * Get the EC_KEY object of a public key
+ *
+ * @param pkey                Public key object
+ * @return                    The underlying EC_KEY object
+ */
+static inline EC_KEY *
+EVP_PKEY_get0_EC_KEY(EVP_PKEY *pkey)
+{
+    return pkey ? pkey->pkey.ec : NULL;
+}
+#endif
+
 #if !defined(HAVE_EVP_PKEY_ID)
 /**
  * Get the PKEY type

@@ -610,6 +624,24 @@ RSA_meth_set0_app_data(RSA_METHOD *meth, void *app_data)
 }
 #endif
 
+#if !defined(HAVE_EC_GROUP_ORDER_BITS) && !defined(OPENSSL_NO_EC)
+/**
+ * Gets the number of bits of the order of an EC_GROUP
+ *
+ *  @param  group               EC_GROUP object
+ *  @return                     number of bits of group order.
+ */
+static inline int
+EC_GROUP_order_bits(const EC_GROUP *group)
+{
+    BIGNUM* order = BN_new();
+    EC_GROUP_get_order(group, order, NULL);
+    int bits = BN_num_bits(order);
+    BN_free(order);
+    return bits;
+}
+#endif
+
 /* SSLeay symbols have been renamed in OpenSSL 1.1 */
 #if !defined(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT)
 #define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT       RSA_F_RSA_EAY_PRIVATE_ENCRYPT
src/openvpn/ssl_openssl.c +27 -2
index 11f4a567..459fe371 100644
@@ -1077,6 +1077,13 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
     ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
     pub_rsa = EVP_PKEY_get0_RSA(pkey);
 
+    /* Certificate might not be RSA but DSA or EC */
+    if (!pub_rsa)
+    {
+        crypto_msg(M_WARN, "management-external-key requires a RSA certificate");
+        goto err;
+    }
+        
     /* initialize RSA object */
     const BIGNUM *n = NULL;
     const BIGNUM *e = NULL;

@@ -1683,18 +1690,36 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
         EVP_PKEY *pkey = X509_get_pubkey(cert);
         if (pkey != NULL)
         {
-            if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) != NULL)
+            if ((EVP_PKEY_id(pkey) == EVP_PKEY_RSA) && (EVP_PKEY_get0_RSA(pkey) != NULL))
             {
                 RSA *rsa = EVP_PKEY_get0_RSA(pkey);
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
                                  RSA_bits(rsa));
             }
-            else if (EVP_PKEY_id(pkey) == EVP_PKEY_DSA && EVP_PKEY_get0_DSA(pkey) != NULL)
+            else if ((EVP_PKEY_id(pkey) == EVP_PKEY_DSA) && (EVP_PKEY_get0_DSA(pkey) != NULL))
             {
                 DSA *dsa = EVP_PKEY_get0_DSA(pkey);
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
                                  DSA_bits(dsa));
             }
+#ifndef OPENSSL_NO_EC
+            else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) && (EVP_PKEY_get0_EC_KEY(pkey) != NULL))
+            {
+                EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
+                const EC_GROUP *group = EC_KEY_get0_group(ec);
+                const char* curve;
+                
+                int nid = EC_GROUP_get_curve_name(group);
+                if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL)
+                {
+                    curve = "Error getting curve name";
+                }
+                
+                openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, curve: %s",
+                                 EC_GROUP_order_bits(group), curve);
+                
+            }
+#endif
             EVP_PKEY_free(pkey);
         }
         X509_free(cert);
Signature · 1 line
2.13.3
#6 lore raw

Hi,

Quoted 127 lines · Arne Schwabe

V2: Print also curve details, add missing ifdef
V3: Goto err instead of using M_FATAL, format fixes, use EC_GROUP_get_curve_name + OBJ_nid2sn instead of ECPKParameters_print, add compat headers for 1.0.2
V4: Formatting changes and change M_ERR to M_WARN

---
 configure.ac                 |  2 ++
 src/openvpn/openssl_compat.h | 32 ++++++++++++++++++++++++++++++++
 src/openvpn/ssl_openssl.c    | 29 +++++++++++++++++++++++++++--
 3 files changed, 61 insertions(+), 2 deletions(-)
configure.ac +2 -0
index 39d992c0..f6eeb40d 100644
@@ -934,6 +934,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 			EVP_PKEY_id \
 			EVP_PKEY_get0_RSA \
 			EVP_PKEY_get0_DSA \
+			EVP_PKEY_get0_EC_KEY \
 			RSA_set_flags \
 			RSA_bits \
 			RSA_get0_key \

@@ -949,6 +950,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 			RSA_meth_set_init \
 			RSA_meth_set_finish \
 			RSA_meth_set0_app_data \
+			EC_GROUP_order_bits
 		]
 	)
src/openvpn/openssl_compat.h +32 -0
index 36f68b01..70b19aea 100644
@@ -244,6 +244,20 @@ EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
 }
 #endif
 
+#if !defined(HAVE_EVP_PKEY_GET0_EC_KEY) && !defined(OPENSSL_NO_EC)
+/**
+ * Get the EC_KEY object of a public key
+ *
+ * @param pkey                Public key object
+ * @return                    The underlying EC_KEY object
+ */
+static inline EC_KEY *
+EVP_PKEY_get0_EC_KEY(EVP_PKEY *pkey)
+{
+    return pkey ? pkey->pkey.ec : NULL;
+}
+#endif
+
 #if !defined(HAVE_EVP_PKEY_ID)
 /**
  * Get the PKEY type

@@ -610,6 +624,24 @@ RSA_meth_set0_app_data(RSA_METHOD *meth, void *app_data)
 }
 #endif
 
+#if !defined(HAVE_EC_GROUP_ORDER_BITS) && !defined(OPENSSL_NO_EC)
+/**
+ * Gets the number of bits of the order of an EC_GROUP
+ *
+ *  @param  group               EC_GROUP object
+ *  @return                     number of bits of group order.
+ */
+static inline int
+EC_GROUP_order_bits(const EC_GROUP *group)
+{
+    BIGNUM* order = BN_new();
+    EC_GROUP_get_order(group, order, NULL);
+    int bits = BN_num_bits(order);
+    BN_free(order);
+    return bits;
+}
+#endif
+
 /* SSLeay symbols have been renamed in OpenSSL 1.1 */
 #if !defined(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT)
 #define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT       RSA_F_RSA_EAY_PRIVATE_ENCRYPT
src/openvpn/ssl_openssl.c +20 -2
index 11f4a567..459fe371 100644
@@ -1077,6 +1077,13 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
     ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
     pub_rsa = EVP_PKEY_get0_RSA(pkey);
 
+    /* Certificate might not be RSA but DSA or EC */
+    if (!pub_rsa)
+    {
+        crypto_msg(M_WARN, "management-external-key requires a RSA certificate");
+        goto err;
+    }
+        
     /* initialize RSA object */
     const BIGNUM *n = NULL;
     const BIGNUM *e = NULL;

@@ -1683,18 +1690,36 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
         EVP_PKEY *pkey = X509_get_pubkey(cert);
         if (pkey != NULL)
         {
-            if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) != NULL)
+            if ((EVP_PKEY_id(pkey) == EVP_PKEY_RSA) && (EVP_PKEY_get0_RSA(pkey) != NULL))
             {
                 RSA *rsa = EVP_PKEY_get0_RSA(pkey);
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
                                  RSA_bits(rsa));
             }
-            else if (EVP_PKEY_id(pkey) == EVP_PKEY_DSA && EVP_PKEY_get0_DSA(pkey) != NULL)
+            else if ((EVP_PKEY_id(pkey) == EVP_PKEY_DSA) && (EVP_PKEY_get0_DSA(pkey) != NULL))
             {
                 DSA *dsa = EVP_PKEY_get0_DSA(pkey);
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
                                  DSA_bits(dsa));
             }
+#ifndef OPENSSL_NO_EC
+            else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) && (EVP_PKEY_get0_EC_KEY(pkey) != NULL))
+            {
+                EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
+                const EC_GROUP *group = EC_KEY_get0_group(ec);
+                const char* curve;
+                
+                int nid = EC_GROUP_get_curve_name(group);
+                if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL)
+                {
+                    curve = "Error getting curve name";

src/openvpn/ssl_openssl.c print_details(struct key_state_ssl *ks_ssl, const char *prefix) · line 1725

Quoted 10 lines
src/openvpn/ssl_openssl.c +7 -0
+                }
+                
+                openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, curve: %s",
+                                 EC_GROUP_order_bits(group), curve);
+                
+            }
+#endif
             EVP_PKEY_free(pkey);
         }
         X509_free(cert);

Patch looks good now, thanks. ACK.

(Some trailing whitespace in the patch, but I guess that's easy enough to fix when applying.)

-Steffan

#7 lore raw
In reply to Arne Schwabe

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Your patch has been applied to the following branches

commit bb23eca847c8edac9c3979b7f35468b74db00459 (master) commit 4b8d654d1339b9adb1f7d554b1f5c16e05123f18 (release/2.4)

Author: Arne Schwabe
Date:   Sun Jul 23 18:45:36 2017 +0200
     Print ec bit details, refuse management-external-key if key is not RSA

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

- --
kind regards,

David Sommerseth

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

iQIcBAEBCgAGBQJZjg98AAoJEIbPlEyWcf3yPeoP/2wafGf4dlzWiZsRTn1aZtj8 gqsxP7JG4a58SxKJb/YrR7RVaFQWYi2dz+f8r6lvabQRd7NDkQzKcEnkkYKjwcmO x8c97IAAhtRwXGY+ChRDgxCiH4PTqWSUv0gk/gjm7K4ijn7HUvU6dZQeDNqRsVSQ D0l3Zx50I6mZCdyiORK7I63aLs0yOJZylI8/5M/pQu9IaANwhnrp0m3JbQPZKwyI IzYEP+Q4SWQq6BHDRYtzU9SPjbyDNeKqUM77mPeX7Z8BbFp9+l3AjKTjqpESyhoS NQX9DXWLjIyzq9SV8/czVsPxhKPDfu2d4QtQldY4Ji/llZqBiCighk+cXBVT8efW A11sFAoI8VTm3C+ftbdH9ndYG8l8Vm2q8lSSke7S04Zgcnn2hMdgoRjPTax+9kM1 /jIjoOP7/NhXdKWeYcHKKTTJIluWcUS/WfTcRkOMda3xg1uDjrlZbMoyr7yWEqpH sOFXRsCDZQde2jup1nRb/+A0hHQIkKBf2GywTAQJi3hH6FgKRiUoIYddRcMMiWQu 4PHBtjx4kz5LkHG2rVlatQm9HMzNk3bD202I52KLjmD9z+56dsSGN03KyvBIfXsP HJeD9HyHk7Y0M3yX0AAWSdvuv2bKTYpP88UzBwAD5Ba/wsMJVUAuad3d6r79UxW1 9xuL94xvDQrvNwiD6Muy
=kFT5
-----END PGP SIGNATURE-----

author
#8 lore raw

PATCH v4 1/1 Print ec bit details, refuse management-external-key if key is not RSA

In reply to a message not in the archive

V2: Print also curve details, add missing ifdef
V3: Goto err instead of using M_FATAL, format fixes, use EC_GROUP_get_curve_name + OBJ_nid2sn instead of ECPKParameters_print, add compat headers for 1.0.2
V4: Formatting changes and change M_ERR to M_WARN

---
 configure.ac                 |  2 ++
 src/openvpn/openssl_compat.h | 32 ++++++++++++++++++++++++++++++++
 src/openvpn/ssl_openssl.c    | 29 +++++++++++++++++++++++++++--
 3 files changed, 61 insertions(+), 2 deletions(-)
configure.ac +2 -0
index 39d992c0..f6eeb40d 100644
@@ -934,6 +934,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 			EVP_PKEY_id \
 			EVP_PKEY_get0_RSA \
 			EVP_PKEY_get0_DSA \
+			EVP_PKEY_get0_EC_KEY \
 			RSA_set_flags \
 			RSA_bits \
 			RSA_get0_key \

@@ -949,6 +950,7 @@ if test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "openssl"; then
 			RSA_meth_set_init \
 			RSA_meth_set_finish \
 			RSA_meth_set0_app_data \
+			EC_GROUP_order_bits
 		]
 	)
src/openvpn/openssl_compat.h +32 -0
index 36f68b01..70b19aea 100644
@@ -244,6 +244,20 @@ EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
 }
 #endif
 
+#if !defined(HAVE_EVP_PKEY_GET0_EC_KEY) && !defined(OPENSSL_NO_EC)
+/**
+ * Get the EC_KEY object of a public key
+ *
+ * @param pkey                Public key object
+ * @return                    The underlying EC_KEY object
+ */
+static inline EC_KEY *
+EVP_PKEY_get0_EC_KEY(EVP_PKEY *pkey)
+{
+    return pkey ? pkey->pkey.ec : NULL;
+}
+#endif
+
 #if !defined(HAVE_EVP_PKEY_ID)
 /**
  * Get the PKEY type

@@ -610,6 +624,24 @@ RSA_meth_set0_app_data(RSA_METHOD *meth, void *app_data)
 }
 #endif
 
+#if !defined(HAVE_EC_GROUP_ORDER_BITS) && !defined(OPENSSL_NO_EC)
+/**
+ * Gets the number of bits of the order of an EC_GROUP
+ *
+ *  @param  group               EC_GROUP object
+ *  @return                     number of bits of group order.
+ */
+static inline int
+EC_GROUP_order_bits(const EC_GROUP *group)
+{
+    BIGNUM* order = BN_new();
+    EC_GROUP_get_order(group, order, NULL);
+    int bits = BN_num_bits(order);
+    BN_free(order);
+    return bits;
+}
+#endif
+
 /* SSLeay symbols have been renamed in OpenSSL 1.1 */
 #if !defined(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT)
 #define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT       RSA_F_RSA_EAY_PRIVATE_ENCRYPT
src/openvpn/ssl_openssl.c +27 -2
index 11f4a567..459fe371 100644
@@ -1077,6 +1077,13 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
     ASSERT(pkey); /* NULL before SSL_CTX_use_certificate() is called */
     pub_rsa = EVP_PKEY_get0_RSA(pkey);
 
+    /* Certificate might not be RSA but DSA or EC */
+    if (!pub_rsa)
+    {
+        crypto_msg(M_WARN, "management-external-key requires a RSA certificate");
+        goto err;
+    }
+        
     /* initialize RSA object */
     const BIGNUM *n = NULL;
     const BIGNUM *e = NULL;

@@ -1683,18 +1690,36 @@ print_details(struct key_state_ssl *ks_ssl, const char *prefix)
         EVP_PKEY *pkey = X509_get_pubkey(cert);
         if (pkey != NULL)
         {
-            if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA && EVP_PKEY_get0_RSA(pkey) != NULL)
+            if ((EVP_PKEY_id(pkey) == EVP_PKEY_RSA) && (EVP_PKEY_get0_RSA(pkey) != NULL))
             {
                 RSA *rsa = EVP_PKEY_get0_RSA(pkey);
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
                                  RSA_bits(rsa));
             }
-            else if (EVP_PKEY_id(pkey) == EVP_PKEY_DSA && EVP_PKEY_get0_DSA(pkey) != NULL)
+            else if ((EVP_PKEY_id(pkey) == EVP_PKEY_DSA) && (EVP_PKEY_get0_DSA(pkey) != NULL))
             {
                 DSA *dsa = EVP_PKEY_get0_DSA(pkey);
                 openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
                                  DSA_bits(dsa));
             }
+#ifndef OPENSSL_NO_EC
+            else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) && (EVP_PKEY_get0_EC_KEY(pkey) != NULL))
+            {
+                EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
+                const EC_GROUP *group = EC_KEY_get0_group(ec);
+                const char* curve;
+                
+                int nid = EC_GROUP_get_curve_name(group);
+                if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL)
+                {
+                    curve = "Error getting curve name";
+                }
+                
+                openvpn_snprintf(s2, sizeof(s2), ", %d bit EC, curve: %s",
+                                 EC_GROUP_order_bits(group), curve);
+                
+            }
+#endif
             EVP_PKEY_free(pkey);
         }
         X509_free(cert);
Signature · 1 line
2.13.3
author
#9 lore raw

Am 07.03.18 um 13:29 schrieb Arne Schwabe:

Quoted 3 lines

V2: Print also curve details, add missing ifdef
V3: Goto err instead of using M_FATAL, format fixes, use EC_GROUP_get_curve_name + OBJ_nid2sn instead of ECPKParameters_print, add compat headers for 1.0.2
V4: Formatting changes and change M_ERR to M_WARN

Resend by accident please ignore.

Arne