ServerRegistry not refreshing certificate information

[basert]

Describe the bug
We are using thrift-mux TLS connections to introduce mTLS between our clients & servers. Recently we switched to dynamic certificate provisioning using cert-manager. These certificates are only valid for a few hours and are rotated frequently. We implemented a custom SslEngineFactory that provides a new SslContext after each reload.

We noticed that /admin/servers/connections/ shows expired certificate dates, because the certificate information does not get updated during the lifetime of a connection.

To Reproduce
Steps to reproduce the behavior:

1. Create a ThriftMux.client.withTransport.tls()
2. Provide a SslClientEngineFactory that allows to you update the underlying SslContext
3. Connect to a server, reload the ssl certificate
4. Connection is using the new certificate and validation works as expected
5. ServerRegistry shows outdated certificate information

Expected behavior
The ServerRegistry should show the latest certificate information for a connection.

If needed I can try to create a small POC that shows our setup.

[heligw]

Can you create a small POC to show your setup?

[basert]

Hey, sorry for the late reply. Here's how our ReloadingEngineFactory looks like:

package com.eslgaming.util.thrift

import com.eslgaming.util.Config
import com.twitter.finagle.Address
import com.twitter.finagle.ssl.{Engine, KeyCredentials, TrustCredentials}
import com.twitter.finagle.ssl.KeyCredentials.{CertAndKey, CertsAndKey, KeyManagerFactory}
import com.twitter.finagle.ssl.client.{SslClientConfiguration, SslClientEngineFactory}
import com.twitter.util.logging.Logging
import com.twitter.util.{Closable, Future, JavaTimer, Time}
import io.netty.buffer.PooledByteBufAllocator
import io.netty.handler.ssl.util.InsecureTrustManagerFactory
import io.netty.handler.ssl.{SslContext, SslContextBuilder}

import scala.util.control.NonFatal

private[thrift] object ReloadingSslEngineFactory {

  private val allocator = PooledByteBufAllocator.DEFAULT

  class ReloadingClientEngineFactory(config: SslClientConfiguration) extends SslClientEngineFactory {
    private val reloader = new Reloader(Builder.build(config))
    override def apply(address: Address, config: SslClientConfiguration): Engine = {
      val engine = Engine(reloader.context.newEngine(allocator))
      SslClientEngineFactory.configureEngine(engine, config)
      engine
    }
  }

  private object Builder {
    def build(config: SslClientConfiguration): SslContext = {
      val builder = SslContextBuilder.forClient()
      val withKey = configureKeyManager(config.keyCredentials, builder)
      val withTrust = configureTrust(config.trustCredentials, withKey)
      withTrust.build()
    }

    private def configureKeyManager(keyCredentials: KeyCredentials, builder: SslContextBuilder): SslContextBuilder =
      keyCredentials match {
        case CertAndKey(certificateFile, keyFile)   => builder.keyManager(certificateFile, keyFile)
        case CertsAndKey(certificatesFile, keyFile) => builder.keyManager(certificatesFile, keyFile)
        case KeyManagerFactory(keyManagerFactory)   => builder.keyManager(keyManagerFactory)
        case _                                      => throw new RuntimeException(s"Cannot build keyManager for keyCredentials $keyCredentials")
      }

    private def configureTrust(trustCredentials: TrustCredentials, builder: SslContextBuilder): SslContextBuilder =
      trustCredentials match {
        case TrustCredentials.Unspecified                              => builder
        case TrustCredentials.Insecure                                 => builder.trustManager(InsecureTrustManagerFactory.INSTANCE)
        case TrustCredentials.CertCollection(file)                     => builder.trustManager(file)
        case TrustCredentials.X509Certificates(x509Certs)              => builder.trustManager(x509Certs: _*)
        case TrustCredentials.TrustManagerFactory(trustManagerFactory) => builder.trustManager(trustManagerFactory)
      }
  }

  private class Reloader(ctxFactory: => SslContext) extends Closable with Logging {

    @volatile var context: SslContext = ctxFactory

    private val timer = new JavaTimer
    private val reloader = timer.schedule(Config.duration("thriftTls.reloadInterval")) {
      try context = ctxFactory
      catch {
        case NonFatal(e) => logger.error("Failed to reload SslContext", e)
      }
    }
    override def close(deadline: Time): Future[Unit] = reloader.close(deadline)
  }

}

It's used in the client by providing an instance of SslClientConfiguration which has the ssl certificate File set.

ThriftMux.client.withTransport.tls(sslClientConfiguration, new ReloadingClientEngineFactory(sslClientConfiguration))

I'll check if I can setup some repo with an example in the next days if needed.