Restructure PeerDelegatedCredentialImpl to avoid double construction …

…of peer cert signature

Summary:
* Remove `PeerDelegatedCredentialImpl::InternalPeerCert` internal class
* Store the credential's signature as a member of `PeerDelegatedCredentialImpl`
* Call `openssl::CertUtils::verify` to verify the credential's signature

This way we don't need to re-construct the parent cert, which was doing a redundant `signature_.setKey()` operation.

Reviewed By: AjanthanAsogamoorthy

Differential Revision: D64788257

fbshipit-source-id: 68b97c620c8dee652c26448d4961f02763ac0fa6

fizz/extensions/delegatedcred/PeerDelegatedCredential-inl.h

@@ -13,24 +13,17 @@ namespace fizz {
 namespace extensions {
 
 template <openssl::KeyType T>
-PeerDelegatedCredentialImpl<T>::InternalPeerCert::InternalPeerCert(
+PeerDelegatedCredentialImpl<T>::PeerDelegatedCredentialImpl(
     folly::ssl::X509UniquePtr cert,
-    folly::ssl::EvpPkeyUniquePtr pubKey)
-    : openssl::OpenSSLPeerCertImpl<T>(std::move(cert)) {
+    folly::ssl::EvpPkeyUniquePtr pubKey,
+    DelegatedCredential credential)
+    : peerCert_(std::move(cert)), credential_(std::move(credential)) {
   if (openssl::CertUtils::getKeyType(pubKey) != T) {
     throw std::runtime_error("Key and credential type don't match");
   }
-  signature_.setKey(std::move(pubKey));
+  credentialSignature_.setKey(std::move(pubKey));
 }
 
-template <openssl::KeyType T>
-PeerDelegatedCredentialImpl<T>::PeerDelegatedCredentialImpl(
-    folly::ssl::X509UniquePtr cert,
-    folly::ssl::EvpPkeyUniquePtr pubKey,
-    DelegatedCredential credential)
-    : peerCertImpl_(std::move(cert), std::move(pubKey)),
-      credential_(std::move(credential)) {}
-
 template <openssl::KeyType T>
 void PeerDelegatedCredentialImpl<T>::verify(
     SignatureScheme scheme,
@@ -43,7 +36,7 @@ void PeerDelegatedCredentialImpl<T>::verify(
         "certificate verify didn't use credential's algorithm",
         AlertDescription::illegal_parameter);
   }
-  auto x509 = peerCertImpl_.getX509();
+  auto x509 = peerCert_.getX509();
   // Check extensions on cert
   DelegatedCredentialUtils::checkExtensions(x509);
   DelegatedCredentialUtils::checkCredentialTimeValidity(
@@ -52,11 +45,9 @@ void PeerDelegatedCredentialImpl<T>::verify(
   // Verify signature
   auto credSignBuf = DelegatedCredentialUtils::prepareSignatureBuffer(
       credential_, folly::ssl::OpenSSLCertUtils::derEncode(*x509));
-  auto parentCert =
-      std::make_unique<openssl::OpenSSLPeerCertImpl<T>>(std::move(x509));
 
   try {
-    parentCert->verify(
+    peerCert_.verify(
         credential_.credential_scheme,
         context == CertificateVerifyContext::Server
             ? CertificateVerifyContext::ServerDelegatedCredential
@@ -71,8 +62,12 @@ void PeerDelegatedCredentialImpl<T>::verify(
   }
 
   // Call the parent verify method
-  peerCertImpl_.verify(
-      scheme, context, std::move(toBeSigned), std::move(signature));
+  openssl::CertUtils::verify(
+      credentialSignature_,
+      scheme,
+      context,
+      std::move(toBeSigned),
+      std::move(signature));
 }
 
 template <openssl::KeyType T>

fizz/extensions/delegatedcred/PeerDelegatedCredential.h

@@ -9,6 +9,7 @@
 #pragma once
 
 #include <fizz/backend/openssl/certificate/OpenSSLPeerCertImpl.h>
+#include <fizz/backend/openssl/crypto/signature/Signature.h>
 #include <fizz/extensions/delegatedcred/Types.h>
 #include <fizz/protocol/clock/SystemClock.h>
 
@@ -41,11 +42,11 @@ class PeerDelegatedCredentialImpl : public PeerDelegatedCredential {
       folly::ByteRange signature) const override;
 
   folly::ssl::X509UniquePtr getX509() const override {
-    return peerCertImpl_.getX509();
+    return peerCert_.getX509();
   }
 
   std::string getIdentity() const override {
-    return peerCertImpl_.getIdentity();
+    return peerCert_.getIdentity();
   }
 
   const DelegatedCredential& getDelegatedCredential() const override {
@@ -60,19 +61,9 @@ class PeerDelegatedCredentialImpl : public PeerDelegatedCredential {
   }
 
  private:
-  class InternalPeerCert : public openssl::OpenSSLPeerCertImpl<T> {
-   public:
-    ~InternalPeerCert() override = default;
-
-    explicit InternalPeerCert(
-        folly::ssl::X509UniquePtr cert,
-        folly::ssl::EvpPkeyUniquePtr pubKey);
-
-    using openssl::OpenSSLPeerCertImpl<T>::signature_;
-    using openssl::OpenSSLPeerCertImpl<T>::cert_;
-  };
-  InternalPeerCert peerCertImpl_;
+  openssl::OpenSSLPeerCertImpl<T> peerCert_;
   DelegatedCredential credential_;
+  openssl::OpenSSLSignature<T> credentialSignature_;
   std::shared_ptr<Clock> clock_ = std::make_shared<SystemClock>();
 };
 } // namespace extensions