[XrdCrypto] Avoid some repated calls of EVP_PKEY_check on the same key

smithdh · Apr 24, 2024 · 6c0ce8b · 6c0ce8b
1 parent fab4b32
commit 6c0ce8b
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 76 deletions.
diff --git a/src/XrdCrypto/XrdCryptosslAux.cc b/src/XrdCrypto/XrdCryptosslAux.cc
@@ -49,32 +49,6 @@
 static int gErrVerifyChain = 0;
 XrdOucTrace *sslTrace = 0;
 
-#if OPENSSL_VERSION_NUMBER < 0x10100000L
-static RSA *EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
-{
-    if (pkey->type != EVP_PKEY_RSA) {
-        return NULL;
-    }
-    return pkey->pkey.rsa;
-}
-#endif
-
-static int XrdCheckRSA (EVP_PKEY *pkey) {
-   int rc;
-#if OPENSSL_VERSION_NUMBER < 0x10101000L
-   RSA *rsa = EVP_PKEY_get0_RSA(pkey);
-   if (rsa)
-      rc = RSA_check_key(rsa);
-   else
-      rc = -2;
-#else
-   EVP_PKEY_CTX *ckctx = EVP_PKEY_CTX_new(pkey, 0);
-   rc = EVP_PKEY_check(ckctx);
-   EVP_PKEY_CTX_free(ckctx);
-#endif
-   return rc;
-}
-
 //____________________________________________________________________________
 int XrdCryptosslX509VerifyCB(int ok, X509_STORE_CTX *ctx)
 {
@@ -552,7 +526,7 @@ int XrdCryptosslX509ParseFile(FILE *fcer,
          DEBUG("found a RSA private key in file " << fname);
          // We need to complete the key
          // check all the public keys of the loaded certificates
-         if (XrdCheckRSA(rsa) == 1) {
+         if (rsa) {
             // Loop over the chain certificates
             XrdCryptoX509 *cert = chain->Begin();
             while (cert && cert->Opaque()) {
@@ -568,12 +542,12 @@ int XrdCryptosslX509ParseFile(FILE *fcer,
 #endif
                         EVP_PKEY_free(evpp);
                         if (rc == 1) {
-                           DEBUG("RSA key completed");
-                           // Update PKI in certificate
+                           // Update PKI in certificate; also tests if the key is complete
                            cert->SetPKI((XrdCryptoX509data)rsa);
-                           // Update status
-                           cert->PKI()->status = XrdCryptoRSA::kComplete;
-                           break;
+                           if (cert->PKI()->status == XrdCryptoRSA::kComplete) {
+                              DEBUG("RSA key completed");
+                              break;
+                           }
                         }
                   }
                }
@@ -662,7 +636,7 @@ int XrdCryptosslX509ParseBucket(XrdSutBucket *b, XrdCryptoX509Chain *chain)
          DEBUG("found a RSA private key in bucket");
          // We need to complete the key
          // check all the public keys of the loaded certificates
-         if (XrdCheckRSA(rsa) == 1) {
+         if (rsa) {
             // Loop over the chain certificates
             XrdCryptoX509 *cert = chain->Begin();
             while (cert && cert->Opaque()) {
@@ -678,12 +652,12 @@ int XrdCryptosslX509ParseBucket(XrdSutBucket *b, XrdCryptoX509Chain *chain)
 #endif
                         EVP_PKEY_free(evpp);
                         if (rc == 1) {
-                           DEBUG("RSA key completed");
-                           // Update PKI in certificate
+                           // Update PKI in certificate; also tests if the key is complete
                            cert->SetPKI((XrdCryptoX509data)rsa);
-                           // Update status
-                           cert->PKI()->status = XrdCryptoRSA::kComplete;
-                           break;
+                           if (cert->PKI()->status == XrdCryptoRSA::kComplete) {
+                              DEBUG("RSA key completed");
+                              break;
+                           }
                         }
                   }
                }

diff --git a/src/XrdCrypto/XrdCryptosslRSA.cc b/src/XrdCrypto/XrdCryptosslRSA.cc
@@ -236,10 +236,10 @@ XrdCryptosslRSA::XrdCryptosslRSA(const XrdCryptosslRSA &r) : XrdCryptoRSA()
             }
           } else {
             if ((fEVP = PEM_read_bio_PrivateKey(bcpy,0,0,0))) {
-               // Check consistency
-               if (XrdCheckRSA(fEVP) == 1) {
-                  // Update status
-                  status = kComplete;
+               // Check consistency only if original was not marked complete
+               if (r.status == kComplete || XrdCheckRSA(fEVP) == 1) {
+                 // Update status
+                 status = kComplete;
                }
             }
          }

diff --git a/src/XrdCrypto/XrdCryptosslX509.cc b/src/XrdCrypto/XrdCryptosslX509.cc
@@ -35,6 +35,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 #include <cerrno>
+#include <memory>
 
 #include "XrdCrypto/XrdCryptosslRSA.hh"
 #include "XrdCrypto/XrdCryptosslX509.hh"
@@ -43,32 +44,6 @@
 
 #include <openssl/pem.h>
 
-#if OPENSSL_VERSION_NUMBER < 0x10100000L
-static RSA *EVP_PKEY_get0_RSA(EVP_PKEY *pkey)
-{
-    if (pkey->type != EVP_PKEY_RSA) {
-        return NULL;
-    }
-    return pkey->pkey.rsa;
-}
-#endif
-
-static int XrdCheckRSA (EVP_PKEY *pkey) {
-   int rc;
-#if OPENSSL_VERSION_NUMBER < 0x10101000L
-   RSA *rsa = EVP_PKEY_get0_RSA(pkey);
-   if (rsa)
-      rc = RSA_check_key(rsa);
-   else
-      rc = -2;
-#else
-   EVP_PKEY_CTX *ckctx = EVP_PKEY_CTX_new(pkey, 0);
-   rc = EVP_PKEY_check(ckctx);
-   EVP_PKEY_CTX_free(ckctx);
-#endif
-   return rc;
-}
-
 #define BIO_PRINT(b,c) \
    BUF_MEM *bptr; \
    BIO_get_mem_ptr(b, &bptr); \
@@ -175,9 +150,10 @@ XrdCryptosslX509::XrdCryptosslX509(const char *cf, const char *kf)
       if ((evpp = PEM_read_PrivateKey(fk,0,0,0))) {
          DEBUG("RSA key completed ");
          // Test consistency
-         if (XrdCheckRSA(evpp) == 1) {
+         std::unique_ptr<XrdCryptoRSA> tmprsa(new XrdCryptosslRSA(evpp, 1));
+         if (tmprsa->status == XrdCryptoRSA::kComplete) {
             // Save it in pki
-            pki = new XrdCryptosslRSA(evpp);
+            pki = tmprsa.release();
          }
       } else {
          DEBUG("cannot read the key from file");
@@ -432,14 +408,17 @@ void XrdCryptosslX509::CertType()
 //_____________________________________________________________________________
 void XrdCryptosslX509::SetPKI(XrdCryptoX509data newpki)
 {
-   // Set PKI
+   if (!newpki) return;
 
-   // Cleanup key first
-   if (pki)
-      delete pki;
-   if (newpki)
-      pki = new XrdCryptosslRSA((EVP_PKEY *)newpki, 1);
+   std::unique_ptr<XrdCryptoRSA> tmprsa(new XrdCryptosslRSA((EVP_PKEY *)newpki, 1));
+   if (tmprsa->status == XrdCryptoRSA::kComplete) {
+      // Cleanup any existing key first
+      if (pki)
+         delete pki;
 
+      // Set PKI
+      pki = tmprsa.release();
+   }
 }
 
 //_____________________________________________________________________________