Browse code

Do not load certificate from tls_ctx_use_external_private_key()

The cert and key loading logic surrounding management-external-key and
management-external cert was somewhat intertwined. Untangle these to
prepare for making the external key code more reusable.

The best part is that this even reduces the number of lines of code.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1536916459-25900-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17464.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Steffan Karger authored on 2018/09/14 18:14:17
Showing 4 changed files
... ...
@@ -657,41 +657,37 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx)
657 657
     }
658 658
 #endif
659 659
 #ifdef MANAGMENT_EXTERNAL_KEY
660
-    else if ((options->management_flags & MF_EXTERNAL_KEY)
661
-             && (options->cert_file || options->management_flags & MF_EXTERNAL_CERT))
660
+    else if (options->management_flags & MF_EXTERNAL_CERT)
662 661
     {
663
-        if (options->cert_file)
664
-        {
665
-            tls_ctx_use_external_private_key(new_ctx, options->cert_file,
666
-                                             options->cert_file_inline);
667
-        }
668
-        else
669
-        {
670
-            char *external_certificate = management_query_cert(management,
671
-                                                               options->management_certificate);
672
-            tls_ctx_use_external_private_key(new_ctx, INLINE_FILE_TAG,
673
-                                             external_certificate);
674
-            free(external_certificate);
675
-        }
662
+        char *cert = management_query_cert(management,
663
+                                           options->management_certificate);
664
+        tls_ctx_load_cert_file(new_ctx, INLINE_FILE_TAG, cert);
665
+        free(cert);
676 666
     }
677 667
 #endif
678
-    else
668
+    else if (options->cert_file)
669
+    {
670
+        tls_ctx_load_cert_file(new_ctx, options->cert_file, options->cert_file_inline);
671
+    }
672
+
673
+    if (options->priv_key_file)
679 674
     {
680
-        /* Load Certificate */
681
-        if (options->cert_file)
675
+        if (0 != tls_ctx_load_priv_file(new_ctx, options->priv_key_file,
676
+                                        options->priv_key_file_inline))
682 677
         {
683
-            tls_ctx_load_cert_file(new_ctx, options->cert_file, options->cert_file_inline);
678
+            goto err;
684 679
         }
685
-
686
-        /* Load Private Key */
687
-        if (options->priv_key_file)
680
+    }
681
+#ifdef MANAGMENT_EXTERNAL_KEY
682
+    else if (options->management_flags & MF_EXTERNAL_KEY)
683
+    {
684
+        if (tls_ctx_use_management_external_key(new_ctx))
688 685
         {
689
-            if (0 != tls_ctx_load_priv_file(new_ctx, options->priv_key_file, options->priv_key_file_inline))
690
-            {
691
-                goto err;
692
-            }
686
+            msg (M_WARN, "Cannot initialize mamagement-external-key");
687
+            goto err;
693 688
         }
694 689
     }
690
+#endif
695 691
 
696 692
     if (options->ca_file || options->ca_path)
697 693
     {
... ...
@@ -270,8 +270,7 @@ void tls_ctx_load_cert_file(struct tls_root_ctx *ctx, const char *cert_file,
270 270
  *                              successful.
271 271
  */
272 272
 int tls_ctx_load_priv_file(struct tls_root_ctx *ctx, const char *priv_key_file,
273
-                           const char *priv_key_file_inline
274
-                           );
273
+                           const char *priv_key_file_inline);
275 274
 
276 275
 #ifdef MANAGMENT_EXTERNAL_KEY
277 276
 
... ...
@@ -280,18 +279,12 @@ int tls_ctx_load_priv_file(struct tls_root_ctx *ctx, const char *priv_key_file,
280 280
  * private key matching the given certificate.
281 281
  *
282 282
  * @param ctx                   TLS context to use
283
- * @param cert_file             The file name to load the certificate from, or
284
- *                              "[[INLINE]]" in the case of inline files.
285
- * @param cert_file_inline      A string containing the certificate
286 283
  *
287
- * @return                      1 if an error occurred, 0 if parsing was
288
- *                              successful.
284
+ * @return                      1 if an error occurred, 0 if successful.
289 285
  */
290
-int tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
291
-                                     const char *cert_file, const char *cert_file_inline);
292
-
293
-#endif
286
+int tls_ctx_use_management_external_key(struct tls_root_ctx *ctx);
294 287
 
288
+#endif /* MANAGMENT_EXTERNAL_KEY */
295 289
 
296 290
 /**
297 291
  * Load certificate authority certificates from the given file or path.
... ...
@@ -621,15 +621,13 @@ external_key_len(void *vctx)
621 621
 }
622 622
 
623 623
 int
624
-tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
625
-                                 const char *cert_file, const char *cert_file_inline)
624
+tls_ctx_use_management_external_key(struct tls_root_ctx *ctx)
626 625
 {
627 626
     ASSERT(NULL != ctx);
628 627
 
629
-    tls_ctx_load_cert_file(ctx, cert_file, cert_file_inline);
630
-
631 628
     if (ctx->crt_chain == NULL)
632 629
     {
630
+        msg (M_WARN, "ERROR: external key requires a certificate.");
633 631
         return 1;
634 632
     }
635 633
 
... ...
@@ -795,11 +795,9 @@ tls_ctx_add_extra_certs(struct tls_root_ctx *ctx, BIO *bio)
795 795
     }
796 796
 }
797 797
 
798
-/* Like tls_ctx_load_cert, but returns a copy of the certificate in **X509 */
799
-static void
800
-tls_ctx_load_cert_file_and_copy(struct tls_root_ctx *ctx,
801
-                                const char *cert_file, const char *cert_file_inline, X509 **x509
802
-                                )
798
+void
799
+tls_ctx_load_cert_file(struct tls_root_ctx *ctx, const char *cert_file,
800
+                       const char *cert_file_inline)
803 801
 {
804 802
     BIO *in = NULL;
805 803
     X509 *x = NULL;
... ...
@@ -807,10 +805,6 @@ tls_ctx_load_cert_file_and_copy(struct tls_root_ctx *ctx,
807 807
     bool inline_file = false;
808 808
 
809 809
     ASSERT(NULL != ctx);
810
-    if (NULL != x509)
811
-    {
812
-        ASSERT(NULL == *x509);
813
-    }
814 810
 
815 811
     inline_file = (strcmp(cert_file, INLINE_FILE_TAG) == 0);
816 812
 
... ...
@@ -861,23 +855,12 @@ end:
861 861
     {
862 862
         BIO_free(in);
863 863
     }
864
-    if (x509)
865
-    {
866
-        *x509 = x;
867
-    }
868 864
     else if (x)
869 865
     {
870 866
         X509_free(x);
871 867
     }
872 868
 }
873 869
 
874
-void
875
-tls_ctx_load_cert_file(struct tls_root_ctx *ctx, const char *cert_file,
876
-                       const char *cert_file_inline)
877
-{
878
-    tls_ctx_load_cert_file_and_copy(ctx, cert_file, cert_file_inline, NULL);
879
-}
880
-
881 870
 int
882 871
 tls_ctx_load_priv_file(struct tls_root_ctx *ctx, const char *priv_key_file,
883 872
                        const char *priv_key_file_inline
... ...
@@ -1039,7 +1022,7 @@ rsa_priv_dec(int flen, const unsigned char *from, unsigned char *to, RSA *rsa, i
1039 1039
 static int
1040 1040
 openvpn_extkey_rsa_finish(RSA *rsa)
1041 1041
 {
1042
-    /* meth was allocated in tls_ctx_use_external_private_key() ; since
1042
+    /* meth was allocated in tls_ctx_use_management_external_key() ; since
1043 1043
      * this function is called when the parent RSA object is destroyed,
1044 1044
      * it is no longer used after this point so kill it. */
1045 1045
     const RSA_METHOD *meth = RSA_get_method(rsa);
... ...
@@ -1288,14 +1271,20 @@ err:
1288 1288
 #endif /* OPENSSL_VERSION_NUMBER > 1.1.0 dev */
1289 1289
 
1290 1290
 int
1291
-tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
1292
-                                 const char *cert_file, const char *cert_file_inline)
1291
+tls_ctx_use_management_external_key(struct tls_root_ctx *ctx)
1293 1292
 {
1294
-    X509 *cert = NULL;
1293
+    int ret = 1;
1295 1294
 
1296 1295
     ASSERT(NULL != ctx);
1297 1296
 
1298
-    tls_ctx_load_cert_file_and_copy(ctx, cert_file, cert_file_inline, &cert);
1297
+#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER)
1298
+    /* OpenSSL 1.0.2 and up */
1299
+    X509 *cert = SSL_CTX_get0_certificate(ctx->ctx);
1300
+#else
1301
+    /* OpenSSL 1.0.1 and earlier need an SSL object to get at the certificate */
1302
+    SSL *ssl = SSL_new(ctx->ctx);
1303
+    X509 *cert = SSL_get_certificate(ssl);
1304
+#endif
1299 1305
 
1300 1306
     ASSERT(NULL != cert);
1301 1307
 
... ...
@@ -1308,7 +1297,7 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
1308 1308
     {
1309 1309
         if (!tls_ctx_use_external_rsa_key(ctx, pkey))
1310 1310
         {
1311
-            goto err;
1311
+            goto cleanup;
1312 1312
         }
1313 1313
     }
1314 1314
 #if OPENSSL_VERSION_NUMBER > 0x10100000L && !defined(OPENSSL_NO_EC) && !defined(LIBRESSL_VERSION_NUMBER)
... ...
@@ -1316,26 +1305,35 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
1316 1316
     {
1317 1317
         if (!tls_ctx_use_external_ec_key(ctx, pkey))
1318 1318
         {
1319
-            goto err;
1319
+            goto cleanup;
1320 1320
         }
1321 1321
     }
1322 1322
     else
1323 1323
     {
1324 1324
         crypto_msg(M_WARN, "management-external-key requires an RSA or EC certificate");
1325
-        goto err;
1325
+        goto cleanup;
1326 1326
     }
1327 1327
 #else
1328 1328
     else
1329 1329
     {
1330 1330
         crypto_msg(M_WARN, "management-external-key requires an RSA certificate");
1331
-        goto err;
1331
+        goto cleanup;
1332 1332
     }
1333 1333
 #endif /* OPENSSL_VERSION_NUMBER > 1.1.0 dev */
1334
-    return 0;
1335 1334
 
1336
-err:
1337
-    crypto_msg(M_FATAL, "Cannot enable SSL external private key capability");
1338
-    return 1;
1335
+    ret = 0;
1336
+cleanup:
1337
+#if OPENSSL_VERSION_NUMBER < 0x10002000L || defined(LIBRESSL_VERSION_NUMBER)
1338
+    if (ssl)
1339
+    {
1340
+        SSL_free(ssl);
1341
+    }
1342
+#endif
1343
+    if (ret)
1344
+    {
1345
+        crypto_msg(M_FATAL, "Cannot enable SSL external private key capability");
1346
+    }
1347
+    return ret;
1339 1348
 }
1340 1349
 
1341 1350
 #endif /* ifdef MANAGMENT_EXTERNAL_KEY */