ServerRegistry not refreshing certificate information · Issue #924 · twitter/finagle · GitHub
Skip to content

ServerRegistry not refreshing certificate information #924

Open

Description

@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.

Activity

  1. heligw commented on Feb 4, 2022

    @heligw
    Contributor

    Can you create a small POC to show your setup?

  2. basert commented on Feb 18, 2022

    @basert
    Author

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions