Cleanup SSL implementation

Remove duplicate code
Merge common functions
Enforce MySQL coding standard

include/violite.h

@@ -105,33 +105,22 @@ void	vio_timeout(Vio *vio,uint which, uint timeout);
 #include <openssl/ssl.h>
 #include <openssl/err.h>
 
-struct st_VioSSLAcceptorFd 
+struct st_VioSSLFd
 {
   SSL_CTX *ssl_context;
-  SSL_METHOD *ssl_method;
-  struct st_VioSSLAcceptorFd *session_id_context;
 };
 
-/* One copy for client */
-struct st_VioSSLConnectorFd
-{
-  SSL_CTX *ssl_context;
-  /* function pointers which are only once for SSL client */ 
-  SSL_METHOD *ssl_method;
-};
-
-int sslaccept(struct st_VioSSLAcceptorFd*, Vio *, long timeout);
-int sslconnect(struct st_VioSSLConnectorFd*, Vio *, long timeout);
+int sslaccept(struct st_VioSSLFd*, Vio *, long timeout);
+int sslconnect(struct st_VioSSLFd*, Vio *, long timeout);
 
-struct st_VioSSLConnectorFd
+struct st_VioSSLFd
 *new_VioSSLConnectorFd(const char *key_file, const char *cert_file,
 		       const char *ca_file,  const char *ca_path,
 		       const char *cipher);
-struct st_VioSSLAcceptorFd
+struct st_VioSSLFd
 *new_VioSSLAcceptorFd(const char *key_file, const char *cert_file,
 		      const char *ca_file,const char *ca_path,
 		      const char *cipher);
-Vio *new_VioSSL(struct st_VioSSLAcceptorFd *fd, Vio *sd, int state);
 #endif /* HAVE_OPENSSL */
 
 #ifdef HAVE_SMEM

sql-common/client.c

@@ -1514,8 +1514,7 @@ mysql_ssl_set(MYSQL *mysql __attribute__((unused)) ,
 static void
 mysql_ssl_free(MYSQL *mysql __attribute__((unused)))
 {
-  struct st_VioSSLConnectorFd *st= 
-    (struct st_VioSSLConnectorFd*) mysql->connector_fd;
+  struct st_VioSSLFd *ssl_fd= (struct st_VioSSLFd*) mysql->connector_fd;
   DBUG_ENTER("mysql_ssl_free");
 
   my_free(mysql->options.ssl_key, MYF(MY_ALLOW_ZERO_PTR));
@@ -1523,8 +1522,8 @@ mysql_ssl_free(MYSQL *mysql __attribute__((unused)))
   my_free(mysql->options.ssl_ca, MYF(MY_ALLOW_ZERO_PTR));
   my_free(mysql->options.ssl_capath, MYF(MY_ALLOW_ZERO_PTR));
   my_free(mysql->options.ssl_cipher, MYF(MY_ALLOW_ZERO_PTR));  
-  if (st)
-    SSL_CTX_free(st->ssl_context);
+  if (ssl_fd)
+    SSL_CTX_free(ssl_fd->ssl_context);
   my_free(mysql->connector_fd,MYF(MY_ALLOW_ZERO_PTR));
   mysql->options.ssl_key = 0;
   mysql->options.ssl_cert = 0;
@@ -1568,6 +1567,63 @@ static MYSQL_METHODS client_methods=
 #endif
 };
 
+int ssl_verify_server_cert(Vio *vio, const char* server_host)
+{
+  SSL *ssl;
+  X509 *server_cert;
+  char *cp1, *cp2;
+  char buf[256];
+  DBUG_ENTER("ssl_verify_server_cert");
+  DBUG_PRINT("enter", ("server_host: %s", server_host));
+
+  if (!(ssl= (SSL*)vio->ssl_arg))
+  {
+    DBUG_PRINT("error", ("No SSL pointer found"));
+    return 1;
+  }
+
+  if (!server_host)
+  {
+    DBUG_PRINT("error", ("No server hostname supplied"));
+    return 1;
+  }
+
+  if (!(server_cert= SSL_get_peer_certificate(ssl)))
+  {
+    DBUG_PRINT("error", ("Could not get server certificate"));
+    return 1;
+  }
+
+  /*
+    We already know that the certificate exchanged was valid; the SSL library
+    handled that. Now we need to verify that the contents of the certificate
+    are what we expect.
+  */
+
+  X509_NAME_oneline(X509_get_subject_name(server_cert), buf, sizeof(buf));
+  X509_free (server_cert);
+
+//  X509_NAME_get_text_by_NID(x509_get_subject_name(server_cert), NID_commonName, buf, sizeof(buf));... does the same thing
+
+  DBUG_PRINT("info", ("hostname in cert: %s", buf));
+  cp1 = strstr(buf, "/CN=");
+  if (cp1)
+  {
+    cp1 += 4; // Skip the "/CN=" that we found
+    cp2 = strchr(cp1, '/');
+    if (cp2)
+      *cp2 = '\0';
+    DBUG_PRINT("info", ("Server hostname in cert: ", cp1));
+    if (!strcmp(cp1, server_host))
+    {
+      /* Success */
+      DBUG_RETURN(0);
+    }
+  }
+  DBUG_PRINT("error", ("SSL certificate validation failure"));
+  DBUG_RETURN(1);
+}
+
 
 MYSQL *
 CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user,
@@ -2013,37 +2069,53 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user,
   mysql->client_flag=client_flag;
 
 #ifdef HAVE_OPENSSL
-  /*
-    Oops.. are we careful enough to not send ANY information without
-    encryption?
-  */
   if (client_flag & CLIENT_SSL)
   {
+    /* Do the SSL layering. */
     struct st_mysql_options *options= &mysql->options;
+    struct st_VioSSLFd *ssl_fd;
+
+    /*
+      Send client_flag, max_packet_size - unencrypted otherwise
+      the server does not know we want to do SSL
+    */
     if (my_net_write(net,buff,(uint) (end-buff)) || net_flush(net))
     {
       set_mysql_error(mysql, CR_SERVER_LOST, unknown_sqlstate);
       goto error;
     }
-    /* Do the SSL layering. */
-    if (!(mysql->connector_fd=
-	  (gptr) new_VioSSLConnectorFd(options->ssl_key,
-				       options->ssl_cert,
-				       options->ssl_ca,
-				       options->ssl_capath,
-				       options->ssl_cipher)))
+
+    /* Create the VioSSLConnectorFd - init SSL and load certs */
+    if (!(ssl_fd= new_VioSSLConnectorFd(options->ssl_key,
+                                        options->ssl_cert,
+                                        options->ssl_ca,
+                                        options->ssl_capath,
+                                        options->ssl_cipher)))
     {
       set_mysql_error(mysql, CR_SSL_CONNECTION_ERROR, unknown_sqlstate);
       goto error;
     }
+    mysql->connector_fd= (void*)ssl_fd;
+
+    /* Connect to the server */
     DBUG_PRINT("info", ("IO layer change in progress..."));
-    if (sslconnect((struct st_VioSSLConnectorFd*)(mysql->connector_fd),
-		   mysql->net.vio, (long) (mysql->options.connect_timeout)))
+    if (sslconnect(ssl_fd, mysql->net.vio,
+                   (long) (mysql->options.connect_timeout)))
     {
       set_mysql_error(mysql, CR_SSL_CONNECTION_ERROR, unknown_sqlstate);
       goto error;
     }
     DBUG_PRINT("info", ("IO layer change done!"));
+
+#if 0
+    /* Verify server cert */
+    if (mysql->options.ssl_verify_cert &&
+        ssl_verify_server_cert(mysql->net.vio, mysql->host))
+    {
+      set_mysql_error(mysql, CR_SSL_CONNECTION_ERROR, unknown_sqlstate);
+      goto error;
+    }
+#endif
   }
 #endif /* HAVE_OPENSSL */
 

sql/mysql_priv.h

@@ -1258,7 +1258,7 @@ extern pthread_t signal_thread;
 #endif
 
 #ifdef HAVE_OPENSSL
-extern struct st_VioSSLAcceptorFd * ssl_acceptor_fd;
+extern struct st_VioSSLFd * ssl_acceptor_fd;
 #endif /* HAVE_OPENSSL */
 
 MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **table, uint count,

sql/mysqld.cc

@@ -599,7 +599,7 @@ static void openssl_lock(int, openssl_lock_t *, const char *, int);
 static unsigned long openssl_id_function();
 #endif
 char *des_key_file;
-struct st_VioSSLAcceptorFd *ssl_acceptor_fd;
+struct st_VioSSLFd *ssl_acceptor_fd;
 #endif /* HAVE_OPENSSL */
 
 
@@ -1110,7 +1110,10 @@ void clean_up(bool print_message)
 #endif
 #ifdef HAVE_OPENSSL
   if (ssl_acceptor_fd)
-    my_free((gptr) ssl_acceptor_fd, MYF(MY_ALLOW_ZERO_PTR));
+  {
+    SSL_CTX_free(ssl_acceptor_fd->ssl_context);
+    my_free((gptr) ssl_acceptor_fd, MYF(0));
+  }
 #endif /* HAVE_OPENSSL */
 #ifdef USE_REGEX
   my_regex_end();

sql/sql_acl.cc

@@ -858,8 +858,8 @@ int acl_getroot(THD *thd, USER_RESOURCES  *mqh,
       if (acl_user->x509_issuer)
       {
         DBUG_PRINT("info",("checkpoint 3"));
-	char *ptr = X509_NAME_oneline(X509_get_issuer_name(cert), 0, 0);
-	DBUG_PRINT("info",("comparing issuers: '%s' and '%s'",
+        char *ptr = X509_NAME_oneline(X509_get_issuer_name(cert), 0, 0);
+        DBUG_PRINT("info",("comparing issuers: '%s' and '%s'",
 			   acl_user->x509_issuer, ptr));
         if (strcmp(acl_user->x509_issuer, ptr))
         {

vio/vio.c

@@ -88,19 +88,19 @@ static void vio_init(Vio* vio, enum enum_vio_type type,
   if (type == VIO_TYPE_SSL)
   {
     vio->viodelete	=vio_delete;
-    vio->vioerrno	=vio_ssl_errno;
+    vio->vioerrno	=vio_errno;
     vio->read		=vio_ssl_read;
     vio->write		=vio_ssl_write;
-    vio->fastsend	=vio_ssl_fastsend;
-    vio->viokeepalive	=vio_ssl_keepalive;
-    vio->should_retry	=vio_ssl_should_retry;
-    vio->was_interrupted=vio_ssl_was_interrupted;
+    vio->fastsend	=vio_fastsend;
+    vio->viokeepalive	=vio_keepalive;
+    vio->should_retry	=vio_should_retry;
+    vio->was_interrupted=vio_was_interrupted;
     vio->vioclose	=vio_ssl_close;
-    vio->peer_addr	=vio_ssl_peer_addr;
-    vio->in_addr	=vio_ssl_in_addr;
+    vio->peer_addr	=vio_peer_addr;
+    vio->in_addr	=vio_in_addr;
     vio->vioblocking	=vio_ssl_blocking;
     vio->is_blocking	=vio_is_blocking;
-    vio->timeout	=vio_ssl_timeout;
+    vio->timeout	=vio_timeout;
   }
   else					/* default is VIO_TYPE_TCPIP */
 #endif /* HAVE_OPENSSL */

vio/vio_priv.h

@@ -30,28 +30,10 @@ void	vio_ignore_timeout(Vio *vio, uint which, uint timeout);
 
 int	vio_ssl_read(Vio *vio,gptr buf,	int size);
 int	vio_ssl_write(Vio *vio,const gptr buf,int size);
-void	vio_ssl_timeout(Vio *vio, uint which, uint timeout);
-
-/* setsockopt TCP_NODELAY at IPPROTO_TCP level, when possible. */
-int vio_ssl_fastsend(Vio *vio);
-/* setsockopt SO_KEEPALIVE at SOL_SOCKET level, when possible. */
-int vio_ssl_keepalive(Vio *vio, my_bool onoff);
-/* Whenever we should retry the last read/write operation. */
-my_bool vio_ssl_should_retry(Vio *vio);
-/* Check that operation was timed out */
-my_bool	vio_ssl_was_interrupted(Vio *vio);
+
 /* When the workday is over... */
 int vio_ssl_close(Vio *vio);
-/* Return last error number */
-int vio_ssl_errno(Vio *vio);
-my_bool vio_ssl_peer_addr(Vio *vio, char *buf, uint16 *port);
-void vio_ssl_in_addr(Vio *vio, struct in_addr *in);
+
 int vio_ssl_blocking(Vio *vio, my_bool set_blocking_mode, my_bool *old_mode);
 
-/* Single copy for server */
-enum vio_ssl_acceptorfd_state
-{
-  state_connect       = 1,
-  state_accept        = 2
-};
 #endif /* HAVE_OPENSSL */