#4638 NSSConnection shutting down existing database
Closed: Fixed None Opened 9 years ago by edewata.

Currently the NSSConnection will shutdown the existing database if there is no other connections using the same database it's going to use. This is causing a conflict with applications that use a database without creating a connection (e.g. vault clients).

rpc.py:

class SSLTransport(...):

    def __nss_initialized(...):
        """
        If there is another connections open it may have already
        initialized NSS. This is likely to lead to an NSS shutdown
        failure.  One way to mitigate this is to tell NSS to not
        initialize if it has already been done in another open connection.

        Returns True if another connection is using the same db.
        """
        for value in context.__dict__.values():
            ...
            # check if the connection is using the same db
            if hasattr(value.conn._ServerProxy__transport, 'dbdir') and \
              value.conn._ServerProxy__transport.dbdir == dbdir:
                return True
        return False

    def make_connection(...):
        ...
        # If we an existing connection exists using the same NSS database
        # there is no need to re-initialize. Pass this into the NSS
        # connection creator.
        ....

        # if there is already a connection using the same db,
        # don't initialize the db again
        no_init = self.__nss_initialized(dbdir)
            ...
            conn = NSSConnection(host, 443, dbdir=dbdir, no_init=no_init)

        ...

nsslib.py:

class NSSConnection(...):

    def __init__(...):
        """
        ...
        :param no_init: do not initialize the NSS database. This requires
                        that the database has already been initialized or
                        the request will fail.
        """
        ...

        if not no_init and nss.nss_is_initialized():
            # close any open NSS database and use the new one
            ...
                nss.nss_shutdown()

        nss.nss_init(dbdir)
        ...

vault.py:

class vault_retrieve(...):

    def forward(...):

        ... initialize db ...

        session_key = ... generate session key ...
        nss_transport_cert = ... get transport cert from db ...
        wrapped_session_key = ... encrypt session key with transport cet ...

        ...

        # get encrypted data from server which will
        # use NSSConnection (which will shutdown the current db)
        response = super(vault_retrieve, self).forward(...)

        secret = ... decrypt data using the same session key ...

The code in vault.py will not work because the session key is needed to decrypt the secret, so the current database cannot be shutdown.


Hi Endi

I don't think your approach in the attached patch is correct. The problem is the context object where the Connection object with the NSS info is stored is thread local. But the NSS database is process global, not thread local. I realize the code has been this way for a while, but it was never correct and we've just been lucky it hasn't burned us yet.

The connection is thread specific, but the NSS database providing access to the necessary certs to support thread specific connects is process global. When this code gets reworked we need to very carefully draw the distinction between connections and the NSS database, they are not the same.

My inclination is to NAK the patch and work on getting the data scope correct, i.e. the right separation between thread/connection and process/database.

As discussed over IRC, the current NSSConnection design is incorrect, but fixing it will take a considerable effort with possibly broad implication which is beyond the scope of this ticket. The above patch maintains the existing behavior while providing a workaround for a new code using NSS database without connection (e.g. vault) and a transition mechanism for existing code to be migrated incrementally, which is a prerequisite for fixing the design issue.

See also the following tickets:

  • Ticket #4662: Centralizing NSS database initialization
  • Ticket #3227: handle NSS initialization and shutdown better, use context's

Metadata Update from @edewata:
- Issue assigned to edewata
- Issue set to the milestone: FreeIPA 4.2 Backlog

7 years ago

Login to comment on this ticket.

Metadata