[v7r0] M2Crypto various fixes

.github/workflows/basic.yml

@@ -13,9 +13,9 @@ jobs:
       matrix:
         command:
           - pytest
-          - DIRAC_USE_M2CRYPTO=Yes pytest
+          - DIRAC_USE_M2CRYPTO=Yes DIRAC_M2CRYPTO_SPLIT_HANDSHAKE=Yes pytest
           - pytest Core/Security/test
-          - DIRAC_USE_M2CRYPTO=Yes pytest Core/Security/test
+          - DIRAC_USE_M2CRYPTO=Yes DIRAC_M2CRYPTO_SPLIT_HANDSHAKE=Yes pytest Core/Security/test
           - tests/checkDocs.sh
           # TODO This should cover more than just tests/CI
           # Excluded codes related to sourcing files

Core/DISET/private/Transports/M2SSLTransport.py

@@ -8,6 +8,7 @@
 import os
 import socket
 from M2Crypto import SSL, threading as M2Threading
+from M2Crypto.SSL.Checker import SSLVerificationError
 
 from DIRAC.Core.Utilities.ReturnValues import S_OK, S_ERROR
 from DIRAC.Core.DISET.private.Transports.BaseTransport import BaseTransport
@@ -22,9 +23,6 @@
 # to M2Crypto: Quite a few functions will need mapping through from OpenSSL to
 # allow the CRL stack to be set on the X509 CTX used for verification.
 
-# TODO: Catch exceptions (from M2 itself and my M2Utils module) and convert them
-# into proper DIRAC style errors.
-
 # TODO: Log useful messages to the logger
 
 
@@ -93,10 +91,8 @@ def initAsClient(self):
         self.remoteAddress = self.oSocket.getpeername()
 
         return S_OK()
-      except socket.error as e:
-        # Other exception are probably SSL-related, in that case we
-        # abort and the exception is forwarded to the caller.
-        error = e
+      except (socket.error, SSLVerificationError, SSL.SSLError) as e:
+        error = "%s:%s" % (e, repr(e))
 
         if self.oSocket is not None:
           self.oSocket.close()
@@ -148,36 +144,124 @@ def renewServerContext(self):
     self.__ctx = getM2SSLContext(self.__ctx, **self.__kwargs)
     return S_OK()
 
-  def handshake(self):
+  def handshake_singleStep(self):
     """ Used to perform SSL handshakes.
         These are now done automatically.
     """
     # This isn't used any more, the handshake is done inside the M2Crypto library
     return S_OK()
 
-  def setClientSocket(self, oSocket):
+  def handshake_multipleSteps(self):
+    """ Perform SSL handshakes.
+        This has to be called after the connection was accepted (acceptConnection_multipleSteps)
+
+        The remote credentials are gathered here
+    """
+    try:
+      # M2Crypto does not provide public method to
+      # accept and handshake in two steps.
+      # So we have to do it manually
+      # The following lines are basically a copy/paste
+      # of the end of SSL.Connection.accept method
+      self.oSocket.setup_ssl()
+      self.oSocket.set_accept_state()
+      self.oSocket.accept_ssl()
+      check = getattr(self.oSocket, 'postConnectionCheck',
+                      self.oSocket.serverPostConnectionCheck)
+      if check is not None:
+        if not check(self.oSocket.get_peer_cert(), self.oSocket.addr[0]):
+          raise SSL.Checker.SSLVerificationError(
+              'post connection check failed')
+
+      self.peerCredentials = getM2PeerInfo(self.oSocket)
+
+      return S_OK()
+    except (socket.error, SSL.SSLError, SSLVerificationError) as e:
+      return S_ERROR("Error in handhsake: %s %s" % (e, repr(e)))
+
+  def setClientSocket_singleStep(self, oSocket):
     """ Set the inner socket (i.e. SSL.Connection object) of this instance
         to the value of oSocket.
+        We also gather the remote peer credentials
         This method is intended to be used to create client connection objects
         from a server and should be considered to be an internal function.
 
         :param oSocket: client socket SSL.Connection object
 
     """
+
+    # TODO: The calling method (ServiceReactor.__acceptIncomingConnection) expects
+    # socket.error to be thrown in case of issue. Maybe we should catch the M2Crypto
+    # errors here and raise socket.error instead
+
     self.oSocket = oSocket
     self.remoteAddress = self.oSocket.getpeername()
     self.peerCredentials = getM2PeerInfo(self.oSocket)
 
-  def acceptConnection(self):
+  def setClientSocket_multipleSteps(self, oSocket):
+    """ Set the inner socket (i.e. SSL.Connection object) of this instance
+        to the value of oSocket.
+        This method is intended to be used to create client connection objects
+        from a server and should be considered to be an internal function.
+
+        :param oSocket: client socket SSL.Connection object
+
+    """
+    # warning: do NOT catch socket.error here, because for who knows what reason
+    # exceptions are actually properly used for once, and the calling method
+    # relies on it (ServiceReactor.__acceptIncomingConnection)
+    self.oSocket = oSocket
+    self.remoteAddress = self.oSocket.getpeername()
+
+  def acceptConnection_multipleSteps(self):
+    """ Accept a new client, returns a new SSLTransport object representing
+        the client connection.
+
+        The connection is accepted, but no SSL handshake is performed
+
+        :returns: S_OK(SSLTransport object)
+    """
+    # M2Crypto does not provide public method to
+    # accept and handshake in two steps.
+    # So we have to do it manually
+    # The following lines are basically a copy/paste
+    # of the begining of SSL.Connection.accept method
+    try:
+      sock, addr = self.oSocket.socket.accept()
+      oClient = SSL.Connection(self.oSocket.ctx, sock)
+      oClient.addr = addr
+      oClientTrans = SSLTransport(self.stServerAddress, ctx=self.__ctx)
+      oClientTrans.setClientSocket(oClient)
+      return S_OK(oClientTrans)
+    except (socket.error, SSL.SSLError, SSLVerificationError) as e:
+      return S_ERROR("Error in acceptConnection: %s %s" % (e, repr(e)))
+
+  def acceptConnection_singleStep(self):
     """ Accept a new client, returns a new SSLTransport object representing
         the client connection.
 
+        The SSL handshake is performed here.
+
         :returns: S_OK(SSLTransport object)
     """
-    oClient, _ = self.oSocket.accept()
-    oClientTrans = SSLTransport(self.stServerAddress, ctx=self.__ctx)
-    oClientTrans.setClientSocket(oClient)
-    return S_OK(oClientTrans)
+    try:
+      oClient, _ = self.oSocket.accept()
+      oClientTrans = SSLTransport(self.stServerAddress, ctx=self.__ctx)
+      oClientTrans.setClientSocket(oClient)
+      return S_OK(oClientTrans)
+    except (socket.error, SSL.SSLError, SSLVerificationError) as e:
+      return S_ERROR("Error in acceptConnection: %s %s" % (e, repr(e)))
+
+  # Depending on the DIRAC_M2CRYPTO_SPLIT_HANDSHAKE we either do the
+  # handshake separately or not
+  if os.getenv('DIRAC_M2CRYPTO_SPLIT_HANDSHAKE', 'NO').lower() in ('yes', 'true'):
+    acceptConnection = acceptConnection_multipleSteps
+    handshake = handshake_multipleSteps
+    setClientSocket = setClientSocket_multipleSteps
+  else:
+    acceptConnection = acceptConnection_singleStep
+    handshake = handshake_singleStep
+    setClientSocket = setClientSocket_singleStep
 
   def _read(self, bufSize=4096, skipReadyCheck=False):
     """ Read bufSize bytes from the buffer.
@@ -188,8 +272,11 @@ def _read(self, bufSize=4096, skipReadyCheck=False):
 
         :returns: S_OK(number of byte read)
     """
-    read = self.oSocket.read(bufSize)
-    return S_OK(read)
+    try:
+      read = self.oSocket.read(bufSize)
+      return S_OK(read)
+    except (socket.error, SSL.SSLError, SSLVerificationError) as e:
+      return S_ERROR("Error in _read: %s %s" % (e, repr(e)))
 
   def isLocked(self):
     """ Returns if this instance is locked.
@@ -207,5 +294,8 @@ def _write(self, buf):
 
         :returns: S_OK(number of bytes written)
     """
-    wrote = self.oSocket.write(buf)
-    return S_OK(wrote)
+    try:
+      wrote = self.oSocket.write(buf)
+      return S_OK(wrote)
+    except (socket.error, SSL.SSLError, SSLVerificationError) as e:
+      return S_ERROR("Error in _write: %s %s" % (e, repr(e)))

Core/DISET/private/Transports/SSL/M2Utils.py

@@ -136,7 +136,7 @@ def getM2SSLContext(ctx=None, **kwargs):
 def getM2PeerInfo(conn):
   """ Gets the details of the current peer as a standard dict. The peer
       details are obtained from the supplied M2 SSL Connection obj "conn".
-      The details returned are those from ~X509Chain.getCredentials:
+      The details returned are those from ~X509Chain.getCredentials, without Registry info:
 
          DN - Full peer DN as string
          x509Chain - Full chain of peer
@@ -147,7 +147,7 @@ def getM2PeerInfo(conn):
       Returns a dict of details.
   """
   chain = X509Chain.generateX509ChainFromSSLConnection(conn)
-  creds = chain.getCredentials()
+  creds = chain.getCredentials(withRegistryInfo=False)
   if not creds['OK']:
     raise RuntimeError("Failed to get SSL peer info (%s)." % creds['Message'])
   peer = creds['Value']

Core/Security/m2crypto/X509Chain.py

@@ -941,10 +941,13 @@ def isPUSP(self):
     return S_OK(False)
 
   @needCertList
-  def getCredentials(self, ignoreDefault=False):
+  def getCredentials(self, ignoreDefault=False, withRegistryInfo=True):
     """ Returns a summary of the credentials contained in the current chain
 
         :params ignoreDefault: (default False) If True and if no DIRAC group is found in the proxy, lookup the CS
+        :params withRegistryInfo: (default True) if set to True, will enhance the returned dict with info
+                                  from the registry
+
 
         :returns: S_OK with the credential dict. Some parameters of the dict are always there, other depends
                 on the nature of the Chain
@@ -962,21 +965,21 @@ def getCredentials(self, ignoreDefault=False):
                 Only for proxy:
                   * identity: If it is a normal proxy, it is the DN of the certificate.
                               If it is a PUSP, it contains the identity as in :py:meth:`.isPUSP`
-                  * username: DIRAC username associated to the DN
+                  * username: DIRAC username associated to the DN (needs withRegistryInfo)
                               (see :py:func:`DIRAC.ConfigurationSystem.Client.Helpers.Registry.getUsernameForDN`)
                   * group: DIRAC group, depending on ignoreDefault param(see :py:meth:`.getDIRACGroup`)
                   * validGroup: True if the group found is in the list of groups the user belongs to
                   * groupProperty: (only if validGroup) get the properties of the group
 
-                For Host certificate:
+                For Host certificate (needs withRegistryInfo):
                   * group: always `hosts`
                   * hostname: name of the host as registered in the CS
                              (see :py:func:`DIRAC.ConfigurationSystem.Client.Helpers.Registry.getHostnameForDN`)
                   * validGroup: True
                   * groupProperties: host options
                                     (see :py:func:`DIRAC.ConfigurationSystem.Client.Helpers.Registry.getHostOption`)
 
-                If it is a user certificate:
+                If it is a user certificate (needs withRegistryInfo):
                   * username: like for proxy
                   * validDN: like proxy
     """
@@ -996,20 +999,22 @@ def getCredentials(self, ignoreDefault=False):
         credDict['identity'] = result['Identity']
         credDict['subproxyUser'] = result['SubproxyUser']
 
-      retVal = Registry.getUsernameForDN(credDict['identity'])
-      if not retVal['OK']:
-        return S_OK(credDict)
-      credDict['username'] = retVal['Value']
-      credDict['validDN'] = True
+      if withRegistryInfo:
+        retVal = Registry.getUsernameForDN(credDict['identity'])
+        if not retVal['OK']:
+          return S_OK(credDict)
+        credDict['username'] = retVal['Value']
+        credDict['validDN'] = True
       retVal = self.getDIRACGroup(ignoreDefault=ignoreDefault)
       if retVal['OK']:
         diracGroup = retVal['Value']
         credDict['group'] = diracGroup
-        retVal = Registry.getGroupsForUser(credDict['username'])
-        if retVal['OK'] and diracGroup in retVal['Value']:
-          credDict['validGroup'] = True
-          credDict['groupProperties'] = Registry.getPropertiesForGroup(diracGroup)
-    else:
+        if withRegistryInfo:
+          retVal = Registry.getGroupsForUser(credDict['username'])
+          if retVal['OK'] and diracGroup in retVal['Value']:
+            credDict['validGroup'] = True
+            credDict['groupProperties'] = Registry.getPropertiesForGroup(diracGroup)
+    elif withRegistryInfo:
       retVal = Registry.getHostnameForDN(credDict['subject'])
       if retVal['OK']:
         credDict['group'] = 'hosts'

docs/source/AdministratorGuide/ServerInstallations/InstallingDiracServer.rst

@@ -189,19 +189,8 @@ Reload the configuration and restart::
 Server Certificates
 -------------------
 
-Server certificates are used for validating the identity of the host a given client is connecting to. By default
-grid host certificate include host/ in the CN (common name) field. This is not a problem for DIRAC components
-since DISET only keeps the host name after the **/** if present.
-
-However if the certificate is used for the Web Portal, the client validating the certificate is your browser. All browsers
-will rise a security alarm if the host name in the url does not match the CN field in the certificate presented by the server.
-In particular this means that *host/*, or other similar parts should nto be present, and that it is preferable to use
-DNS aliases and request a certificate under this alias in order to be able to migrate the server to a new host without
-having to change your URLs. DIRAC will accept both real host names and any valid aliases without complains.
-
-Finally, you will have to instruct you users on the procedure to upload the public key of the CA signing the certificate
-of the host where the Web Portal is running. This depends from CA to CA, but typically only means clicking on a certain
-link on the web portal of the CA.
+Server certificates are used for validating the identity of the host a given client is connecting to. We follow the RFC 6125.
+Basically, that means that the DNS name used to contact the host must be present in the ``SubjectAlternativeName``. 
 
 -----------------
 Using your own CA

docs/source/AdministratorGuide/ServerInstallations/environment_variable_configuration.rst

@@ -25,5 +25,8 @@ DIRAC_GFAL_GRIDFTP_SESSION_REUSE
 DIRAC_USE_M2CRYPTO
   If ``true`` or ``yes`` DIRAC uses m2crypto instead of pyGSI for handling certificates, proxies, etc.
 
+DIRAC_M2CRYPTO_SPLIT_HANDSHAKE
+  If ``true`` or ``yes`` the SSL handshake is done in a new thread (default No)
+
 DIRAC_VOMSES
   Can be set to point to a folder containing VOMSES information. See :ref:`multi_vo_dirac`

docs/source/AdministratorGuide/technologyPreviews.rst

@@ -12,13 +12,16 @@ M2Crypto
 ========
 
 We aim at replacing the home made wrapper of openssl pyGSI with the standard M2Crypto library. It is by default disabled.
-You can enable it by setting the environment variable `DIRAC_USE_M2CRYPTO` to `Yes`.
+You can enable it by setting the environment variable ``DIRAC_USE_M2CRYPTO`` to ``Yes``.
+There is also a change of behavior in the way sockets connections are handled server side. The current (new) behavior is to perform the SSL handshake in the main thread and only delegate the execution of the method to another thread. The pyGSI behavior was to delegate the SSL handshake to the new thread, which is probably a better thing to do. You can revert back this behavior by setting `DIRAC_M2CRYPTO_SPLIT_HANDSHAKE` to `Yes`, but this has not been heavily tested yet. 
 
 Possible issues
 ---------------
 
 M2Crypto (or any standard tool that respects TLS..) will be stricter than PyGSI. So you may need to adapt your environment a bit. Here are a few hints:
 
 * SAN in your certificates: if you are contacting a machine using its aliases, make sure that all the aliases are in the SubjectAlternativeName (SAN) field of the certificates
-* FQDN in the configuration: SAN normally contains only FQDN, so make sure you use the FQDN in the CS as well (e.g. `mymachine.cern.ch` and not `mymachine`)
+* FQDN in the configuration: SAN normally contains only FQDN, so make sure you use the FQDN in the CS as well (e.g. ``mymachine.cern.ch`` and not ``mymachine``)
 * ComponentInstaller screwed: like any change you do on your hosts, the ComponentInstaller will duplicate the entry. So if you change the CS to put FQDN, the machine will appear twice. 
+
+In case your services are not fast enough, and the socket backlog is full (``ss -pnl``), try setting ``DIRAC_M2CRYPTO_SPLIT_HANDSHAKE`` to ``Yes``.

tests/CI/run_docker_setup.sh

@@ -156,10 +156,12 @@ function prepareEnvironment() {
 
   if [ -n "${SERVER_USE_M2CRYPTO+x}" ]; then
     echo "export DIRAC_USE_M2CRYPTO=${SERVER_USE_M2CRYPTO}" >> "${SERVERCONFIG}"
+    echo "export DIRAC_M2CRYPTO_SPLIT_HANDSHAKE=Yes" >> "${SERVERCONFIG}"
   fi
 
   if [ -n "${CLIENT_USE_M2CRYPTO+x}" ]; then
     echo "export DIRAC_USE_M2CRYPTO=${CLIENT_USE_M2CRYPTO}" >> "${CLIENTCONFIG}"
+    echo "export DIRAC_M2CRYPTO_SPLIT_HANDSHAKE=Yes" >> "${CLIENTCONFIG}"
   fi
 
   docker-compose -f ./docker-compose.yml up -d