From f75d6e53069227ebed95213723d5d31281755a76 Mon Sep 17 00:00:00 2001 From: Ramasai Date: Wed, 4 Jun 2025 22:06:45 -0400 Subject: [PATCH 1/2] GH-155: Delegate opening mTLS cert files to `build()` --- .../org/apache/arrow/flight/FlightServer.java | 47 ++++++++++++++++--- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java b/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java index ac761457f5..c4fd245a61 100644 --- a/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java +++ b/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java @@ -190,9 +190,18 @@ public static final class Builder { private int maxInboundMessageSize = MAX_GRPC_MESSAGE_SIZE; private int maxHeaderListSize = MAX_GRPC_MESSAGE_SIZE; private int backpressureThreshold = DEFAULT_BACKPRESSURE_THRESHOLD; + + /* + TODO: Remove certChain/key/mTlsCACert as instance vars and make them local vars in build() after they are + no longer being set in the builder methods. + */ + private File certChainFile; private InputStream certChain; + private File keyFile; private InputStream key; + private File mTlsCACertFile; private InputStream mTlsCACert; + private SslContext sslContext; private final List> interceptors; // Keep track of inserted interceptors @@ -213,6 +222,16 @@ public static final class Builder { /** Create the server for this builder. */ public FlightServer build() { + // Get TLS info in order if the server is being configured to use mTLS. + try { + prepareTlsSettings(); + } catch (IOException e) { + closeMTlsCACert(); + closeCertChain(); + closeKey(); + throw new RuntimeException("Could not create FlightServer with mTLS", e); + } + // Add the auth middleware if applicable. if (headerAuthenticator != CallHeaderAuthenticator.NO_OP) { this.middleware( @@ -442,11 +461,8 @@ private void closeMTlsCACert() { * @param key The private key to use. */ public Builder useTls(final File certChain, final File key) throws IOException { - closeCertChain(); - this.certChain = new FileInputStream(certChain); - - closeKey(); - this.key = new FileInputStream(key); + this.certChainFile = certChain; + this.keyFile = key; return this; } @@ -457,8 +473,8 @@ public Builder useTls(final File certChain, final File key) throws IOException { * @param mTlsCACert The CA certificate to use for verifying clients. */ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOException { - closeMTlsCACert(); - this.mTlsCACert = new FileInputStream(mTlsCACert); + this.mTlsCACertFile = mTlsCACert; + return this; } @@ -468,6 +484,7 @@ public Builder useMTlsClientVerification(final File mTlsCACert) throws IOExcepti * @param certChain The certificate chain to use. * @param key The private key to use. */ + @Deprecated(forRemoval = true, since = "18.4.0") public Builder useTls(final InputStream certChain, final InputStream key) throws IOException { closeCertChain(); this.certChain = certChain; @@ -483,6 +500,7 @@ public Builder useTls(final InputStream certChain, final InputStream key) throws * * @param mTlsCACert The CA certificate to use for verifying clients. */ + @Deprecated(forRemoval = true, since = "18.4.0") public Builder useMTlsClientVerification(final InputStream mTlsCACert) throws IOException { closeMTlsCACert(); this.mTlsCACert = mTlsCACert; @@ -552,5 +570,20 @@ public Builder producer(FlightProducer producer) { this.producer = Preconditions.checkNotNull(producer); return this; } + + private void prepareTlsSettings() throws IOException { + if (this.keyFile != null) { + closeKey(); + this.key = new FileInputStream(this.keyFile); + } + if (this.certChainFile != null) { + closeCertChain(); + this.certChain = new FileInputStream(this.certChainFile); + } + if (this.mTlsCACertFile != null) { + closeMTlsCACert(); + this.mTlsCACert = new FileInputStream(this.mTlsCACertFile); + } + } } } From ffb76f45f09b67d07b1cea81759fccdafea95d04 Mon Sep 17 00:00:00 2001 From: Ramasai Date: Wed, 4 Jun 2025 22:09:59 -0400 Subject: [PATCH 2/2] move code around to preserve closing order --- .../java/org/apache/arrow/flight/FlightServer.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java b/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java index c4fd245a61..728edd0442 100644 --- a/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java +++ b/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java @@ -572,17 +572,17 @@ public Builder producer(FlightProducer producer) { } private void prepareTlsSettings() throws IOException { - if (this.keyFile != null) { - closeKey(); - this.key = new FileInputStream(this.keyFile); + if (this.mTlsCACertFile != null) { + closeMTlsCACert(); + this.mTlsCACert = new FileInputStream(this.mTlsCACertFile); } if (this.certChainFile != null) { closeCertChain(); this.certChain = new FileInputStream(this.certChainFile); } - if (this.mTlsCACertFile != null) { - closeMTlsCACert(); - this.mTlsCACert = new FileInputStream(this.mTlsCACertFile); + if (this.keyFile != null) { + closeKey(); + this.key = new FileInputStream(this.keyFile); } } }