Browse code

Move setting private key to a function in prep for EC support

- Also add reference counting to CAPI_DATA (application data):

When the application data is assigned to the private key
we free it in the key's finish method. Proper error handling
requires to keep track of whether data is assigned to the
key or not before an error occurs. For this purpose, add a
reference count to CAPI_DATA struct and increment it when it is
assigned to the key or its method.

CAPI_DATA_free now frees the data only if ref_count <= 0

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1519354999-18496-1-git-send-email-selva.nair@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16536.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Selva Nair authored on 2018/02/23 12:03:19
Showing 1 changed files
... ...
@@ -106,12 +106,13 @@ typedef struct _CAPI_DATA {
106 106
     HCRYPTPROV_OR_NCRYPT_KEY_HANDLE crypt_prov;
107 107
     DWORD key_spec;
108 108
     BOOL free_crypt_prov;
109
+    int ref_count;
109 110
 } CAPI_DATA;
110 111
 
111 112
 static void
112 113
 CAPI_DATA_free(CAPI_DATA *cd)
113 114
 {
114
-    if (!cd)
115
+    if (!cd || cd->ref_count-- > 0)
115 116
     {
116 117
         return;
117 118
     }
... ...
@@ -466,14 +467,85 @@ find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store)
466 466
     return rv;
467 467
 }
468 468
 
469
+static int
470
+ssl_ctx_set_rsakey(SSL_CTX *ssl_ctx, CAPI_DATA *cd, EVP_PKEY *pkey)
471
+{
472
+    RSA *rsa = NULL, *pub_rsa;
473
+    RSA_METHOD *my_rsa_method = NULL;
474
+    bool rsa_method_set = false;
475
+
476
+    my_rsa_method = RSA_meth_new("Microsoft Cryptography API RSA Method",
477
+                                  RSA_METHOD_FLAG_NO_CHECK);
478
+    check_malloc_return(my_rsa_method);
479
+    RSA_meth_set_pub_enc(my_rsa_method, rsa_pub_enc);
480
+    RSA_meth_set_pub_dec(my_rsa_method, rsa_pub_dec);
481
+    RSA_meth_set_priv_enc(my_rsa_method, rsa_priv_enc);
482
+    RSA_meth_set_priv_dec(my_rsa_method, rsa_priv_dec);
483
+    RSA_meth_set_init(my_rsa_method, NULL);
484
+    RSA_meth_set_finish(my_rsa_method, finish);
485
+    RSA_meth_set0_app_data(my_rsa_method, cd);
486
+
487
+    rsa = RSA_new();
488
+    if (rsa == NULL)
489
+    {
490
+        SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_FILE, ERR_R_MALLOC_FAILURE);
491
+        goto err;
492
+    }
493
+
494
+    pub_rsa = EVP_PKEY_get0_RSA(pkey);
495
+    if (!pub_rsa)
496
+    {
497
+        goto err;
498
+    }
499
+
500
+    /* Our private key is external, so we fill in only n and e from the public key */
501
+    const BIGNUM *n = NULL;
502
+    const BIGNUM *e = NULL;
503
+    RSA_get0_key(pub_rsa, &n, &e, NULL);
504
+    BIGNUM *rsa_n = BN_dup(n);
505
+    BIGNUM *rsa_e = BN_dup(e);
506
+    if (!rsa_n || !rsa_e || !RSA_set0_key(rsa, rsa_n, rsa_e, NULL))
507
+    {
508
+        BN_free(rsa_n); /* ok to free even if NULL */
509
+        BN_free(rsa_e);
510
+        msg(M_NONFATAL, "ERROR: %s: out of memory", __func__);
511
+        goto err;
512
+    }
513
+    RSA_set_flags(rsa, RSA_flags(rsa) | RSA_FLAG_EXT_PKEY);
514
+    if (!RSA_set_method(rsa, my_rsa_method))
515
+    {
516
+        goto err;
517
+    }
518
+    rsa_method_set = true; /* flag that method pointer will get freed with the key */
519
+    cd->ref_count++;       /* with method, cd gets assigned to the key as well */
520
+
521
+    if (!SSL_CTX_use_RSAPrivateKey(ssl_ctx, rsa))
522
+    {
523
+        goto err;
524
+    }
525
+    /* SSL_CTX_use_RSAPrivateKey() increased the reference count in 'rsa', so
526
+     * we decrease it here with RSA_free(), or it will never be cleaned up. */
527
+    RSA_free(rsa);
528
+    return 1;
529
+
530
+err:
531
+    if (rsa)
532
+    {
533
+        RSA_free(rsa);
534
+    }
535
+    if (my_rsa_method && !rsa_method_set)
536
+    {
537
+        RSA_meth_free(my_rsa_method);
538
+    }
539
+    return 0;
540
+}
541
+
469 542
 int
470 543
 SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
471 544
 {
472 545
     HCERTSTORE cs;
473 546
     X509 *cert = NULL;
474
-    RSA *rsa = NULL, *pub_rsa;
475 547
     CAPI_DATA *cd = calloc(1, sizeof(*cd));
476
-    RSA_METHOD *my_rsa_method = NULL;
477 548
 
478 549
     if (cd == NULL)
479 550
     {
... ...
@@ -548,30 +620,13 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
548 548
         }
549 549
     }
550 550
 
551
-    my_rsa_method = RSA_meth_new("Microsoft Cryptography API RSA Method",
552
-                                  RSA_METHOD_FLAG_NO_CHECK);
553
-    check_malloc_return(my_rsa_method);
554
-    RSA_meth_set_pub_enc(my_rsa_method, rsa_pub_enc);
555
-    RSA_meth_set_pub_dec(my_rsa_method, rsa_pub_dec);
556
-    RSA_meth_set_priv_enc(my_rsa_method, rsa_priv_enc);
557
-    RSA_meth_set_priv_dec(my_rsa_method, rsa_priv_dec);
558
-    RSA_meth_set_init(my_rsa_method, NULL);
559
-    RSA_meth_set_finish(my_rsa_method, finish);
560
-    RSA_meth_set0_app_data(my_rsa_method, cd);
561
-
562
-    rsa = RSA_new();
563
-    if (rsa == NULL)
564
-    {
565
-        SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_FILE, ERR_R_MALLOC_FAILURE);
566
-        goto err;
567
-    }
568
-
569 551
     /* Public key in cert is NULL until we call SSL_CTX_use_certificate(),
570 552
      * so we do it here then...  */
571 553
     if (!SSL_CTX_use_certificate(ssl_ctx, cert))
572 554
     {
573 555
         goto err;
574 556
     }
557
+
575 558
     /* the public key */
576 559
     EVP_PKEY *pkey = X509_get0_pubkey(cert);
577 560
 
... ...
@@ -580,52 +635,23 @@ SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
580 580
     X509_free(cert);
581 581
     cert = NULL;
582 582
 
583
-    if (!(pub_rsa = EVP_PKEY_get0_RSA(pkey)))
583
+    if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA)
584 584
     {
585
-        msg(M_WARN, "cryptoapicert requires an RSA certificate");
586
-        goto err;
587
-    }
588
-
589
-    /* Our private key is external, so we fill in only n and e from the public key */
590
-    const BIGNUM *n = NULL;
591
-    const BIGNUM *e = NULL;
592
-    RSA_get0_key(pub_rsa, &n, &e, NULL);
593
-    if (!RSA_set0_key(rsa, BN_dup(n), BN_dup(e), NULL))
594
-    {
595
-        goto err;
596
-    }
597
-    RSA_set_flags(rsa, RSA_flags(rsa) | RSA_FLAG_EXT_PKEY);
598
-    if (!RSA_set_method(rsa, my_rsa_method))
599
-    {
600
-        goto err;
585
+        if (!ssl_ctx_set_rsakey(ssl_ctx, cd, pkey))
586
+        {
587
+            goto err;
588
+        }
601 589
     }
602
-
603
-    if (!SSL_CTX_use_RSAPrivateKey(ssl_ctx, rsa))
590
+    else
604 591
     {
592
+        msg(M_WARN, "cryptoapicert requires an RSA certificate");
605 593
         goto err;
606 594
     }
607
-    /* SSL_CTX_use_RSAPrivateKey() increased the reference count in 'rsa', so
608
-    * we decrease it here with RSA_free(), or it will never be cleaned up. */
609
-    RSA_free(rsa);
595
+    CAPI_DATA_free(cd); /* this will do a ref_count-- */
610 596
     return 1;
611 597
 
612 598
 err:
613
-    if (cert)
614
-    {
615
-        X509_free(cert);
616
-    }
617
-    if (rsa)
618
-    {
619
-        RSA_free(rsa);
620
-    }
621
-    else
622
-    {
623
-        if (my_rsa_method)
624
-        {
625
-            free(my_rsa_method);
626
-        }
627
-        CAPI_DATA_free(cd);
628
-    }
599
+    CAPI_DATA_free(cd);
629 600
     return 0;
630 601
 }
631 602