Clean-up-options

Summary:
Right now we have a single option verifyPeer_(SSLVerifyPeerEnum) that
controls how verification is done regardless of how the ssl context is
inevitably used (client or server). This is confusing for our users since it
is 1. unclear of what the current values mean 2. It is unclear that setting an
option such as VERIFY_REQ_CLIENT_CERT also sets your verification mode when
running as a client.In order to make this api more clear for
our users we separate out this enum to two separate enums for client and
server. When this is passed to openSSL we still pass the same single mode int.

Reviewed By: mingtaoy

Differential Revision: D18308192

fbshipit-source-id: 5d49d3dc5117075a55681cf67b5f8e72b5e62c89

folly/io/async/SSLContext.cpp

@@ -99,7 +99,6 @@ void configureProtocolVersion(SSL_CTX* ctx, SSLContext::SSLVersion version) {
   DCHECK((newOpt & opt) == opt);
 #endif // FOLLY_OPENSSL_PREREQ(1, 1, 0)
 }
-
 } // namespace
 
 //
@@ -231,6 +230,50 @@ void SSLContext::setVerificationOption(
   verifyPeer_ = verifyPeer;
 }
 
+void SSLContext::setVerificationOption(
+    const SSLContext::VerifyClientCertificate& verifyClient) {
+  verifyClient_ = verifyClient;
+}
+
+void SSLContext::setVerificationOption(
+    const SSLContext::VerifyServerCertificate& verifyServer) {
+  verifyServer_ = verifyServer;
+}
+
+int SSLContext::getVerificationMode(
+    const SSLContext::VerifyClientCertificate& verifyClient) {
+  int mode = SSL_VERIFY_NONE;
+  switch (verifyClient) {
+    case VerifyClientCertificate::ALWAYS:
+      mode = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT;
+      break;
+
+    case VerifyClientCertificate::IF_PRESENTED:
+      mode = SSL_VERIFY_PEER;
+      break;
+
+    case VerifyClientCertificate::DO_NOT_REQUEST:
+      mode = SSL_VERIFY_NONE;
+      break;
+  }
+  return mode;
+}
+
+int SSLContext::getVerificationMode(
+    const SSLContext::VerifyServerCertificate& verifyServer) {
+  int mode = SSL_VERIFY_NONE;
+  switch (verifyServer) {
+    case VerifyServerCertificate::IF_PRESENTED:
+      mode = SSL_VERIFY_PEER;
+      break;
+
+    case VerifyServerCertificate::IGNORE_VERIFY_RESULT:
+      mode = SSL_VERIFY_NONE;
+      break;
+  }
+  return mode;
+}
+
 int SSLContext::getVerificationMode(
     const SSLContext::SSLVerifyPeerEnum& verifyPeer) {
   CHECK(verifyPeer != SSLVerifyPeerEnum::USE_CTX);
@@ -258,7 +301,11 @@ int SSLContext::getVerificationMode(
 }
 
 int SSLContext::getVerificationMode() {
-  return getVerificationMode(verifyPeer_);
+  // the below or'ing is incorrect unless VERIFY_NONE is 0
+  static_assert(SSL_VERIFY_NONE == 0);
+  return getVerificationMode(verifyClient_) |
+      getVerificationMode(verifyServer_) |
+      getVerificationMode(verifyPeer_);
 }
 
 void SSLContext::authenticate(

folly/io/async/SSLContext.h

@@ -98,6 +98,7 @@ class SSLContext {
 
   /**
    * Defines the way that peers are verified.
+   * TODO: remove this in favor of the specific client and server enums
    **/
   enum SSLVerifyPeerEnum {
     // Used by AsyncSSLSocket to delegate to the SSLContext's setting
@@ -115,6 +116,27 @@ class SSLContext {
     NO_VERIFY
   };
 
+  enum class VerifyClientCertificate {
+    // Request a cert and verify it. Fail if verification fails or no
+    // cert is presented
+    ALWAYS,
+    // Request a cert from the peer and verify if one is presented.
+    // Will fail if verification fails.
+    // Do not fail if no cert is presented.
+    IF_PRESENTED,
+    // No verification is done and no cert is requested.
+    DO_NOT_REQUEST
+  };
+
+  enum class VerifyServerCertificate {
+    // Server cert will be presented unless anon cipher,
+    // Verficiation WILL happen and a failure will result in termination
+    IF_PRESENTED,
+    // Server cert will be presented unless anon cipher,
+    // Verification WILL happen but the result will be ignored
+    IGNORE_VERIFY_RESULT
+  };
+
   struct NextProtocolsItem {
     NextProtocolsItem(int wt, const std::list<std::string>& ptcls)
         : weight(wt), protocols(ptcls) {}
@@ -237,6 +259,15 @@ class SSLContext {
    *                       method to use.
    */
   virtual void setVerificationOption(const SSLVerifyPeerEnum& verifyPeer);
+  /**
+   * Method to set verification options for client and server separately.
+   * This is highly recommended as these options are much clearer and the other
+   * way will be going away soon
+   */
+  virtual void setVerificationOption(
+      const VerifyClientCertificate& verifyClient);
+  virtual void setVerificationOption(
+      const VerifyServerCertificate& verifyServer);
 
   /**
    * Method to check if peer verfication is set.
@@ -245,9 +276,10 @@ class SSLContext {
    *
    */
   virtual bool needsPeerVerification() {
-    return (
-        verifyPeer_ == SSLVerifyPeerEnum::VERIFY ||
-        verifyPeer_ == SSLVerifyPeerEnum::VERIFY_REQ_CLIENT_CERT);
+    /* TODO this is ugly and i can't think of a reason this should exist
+     * will think of what i want to do with this later
+     */
+    return getVerificationMode() != SSL_VERIFY_NONE;
   }
 
   /**
@@ -261,6 +293,8 @@ class SSLContext {
    * @return mode flags that can be used with SSL_set_verify
    */
   static int getVerificationMode(const SSLVerifyPeerEnum& verifyPeer);
+  static int getVerificationMode(const VerifyClientCertificate& verifyClient);
+  static int getVerificationMode(const VerifyServerCertificate& verifyServer);
 
   /**
    * Method to fetch Verification mode determined by the options
@@ -583,8 +617,16 @@ class SSLContext {
   SSL_CTX* ctx_;
 
  private:
+  // TODO deprecate this, it's confusing and the default is bad
   SSLVerifyPeerEnum verifyPeer_{SSLVerifyPeerEnum::NO_VERIFY};
 
+  /* Set one of these values depending on whether you will use the context
+   * for a server or client.*/
+  VerifyClientCertificate verifyClient_{
+      VerifyClientCertificate::DO_NOT_REQUEST};
+  VerifyServerCertificate verifyServer_{
+      VerifyServerCertificate::IGNORE_VERIFY_RESULT};
+
   bool checkPeerName_;
   std::string peerFixedName_;
   std::shared_ptr<PasswordCollector> collector_;