Skip to content

Commit 8ed79c7

Browse files
tballisontballison
authored
improve tls (#2973)
1 parent cbb8df2 commit 8ed79c7

11 files changed

Lines changed: 363 additions & 5 deletions

File tree

‎tika-grpc/src/main/java/org/apache/tika/pipes/grpc/TikaGrpcServer.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,21 @@ public void start() throws Exception {
8585
if (secure) {
8686
TlsServerCredentials.Builder channelCredBuilder = TlsServerCredentials.newBuilder();
8787
channelCredBuilder.keyManager(certChain, privateKey, privateKeyPassword);
88-
if (trustCertCollection != null && trustCertCollection.exists()) {
89-
channelCredBuilder.trustManager(trustCertCollection);
90-
if (clientAuthRequired) {
91-
channelCredBuilder.clientAuth(TlsServerCredentials.ClientAuth.REQUIRE);
88+
if (clientAuthRequired) {
89+
if (trustCertCollection == null || !trustCertCollection.isFile() || !trustCertCollection.canRead()) {
90+
throw new IllegalArgumentException("--client-auth-required is set but --trust-cert-collection is " +
91+
"missing, not a file, or unreadable; refusing to start");
9292
}
93+
channelCredBuilder.trustManager(trustCertCollection);
94+
channelCredBuilder.clientAuth(TlsServerCredentials.ClientAuth.REQUIRE);
95+
} else if (trustCertCollection != null && trustCertCollection.exists()) {
96+
channelCredBuilder.trustManager(trustCertCollection);
9397
}
9498
creds = channelCredBuilder.build();
9599
} else {
100+
if (clientAuthRequired) {
101+
throw new IllegalArgumentException("--client-auth-required requires --secure; refusing to start");
102+
}
96103
creds = InsecureServerCredentials.create();
97104
}
98105
if (tikaConfig == null) {

‎tika-grpc/src/test/java/org/apache/tika/pipes/grpc/PipesBiDirectionalStreamingIntegrationTest.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import com.fasterxml.jackson.databind.ObjectMapper;
3737
import io.grpc.Grpc;
3838
import io.grpc.ManagedChannel;
39+
import io.grpc.StatusRuntimeException;
3940
import io.grpc.TlsChannelCredentials;
4041
import io.grpc.netty.shaded.io.netty.handler.ssl.util.InsecureTrustManagerFactory;
4142
import io.grpc.stub.StreamObserver;
@@ -236,4 +237,28 @@ public void onCompleted() {
236237
Assertions.assertEquals(files.size(), numParsed.get());
237238
Assertions.assertEquals(files.size(), result.size());
238239
}
240+
241+
@Test
242+
void testClientAuthRequiredRejectsCertlessClient() throws Exception {
243+
// Same running server as the other tests, but this channel presents no client
244+
// certificate, so the call should fail at the handshake.
245+
String target = InetAddress
246+
.getByName("localhost")
247+
.getHostAddress() + ":" + grpcPort;
248+
TlsChannelCredentials.Builder channelCredBuilder = TlsChannelCredentials.newBuilder();
249+
channelCredBuilder.trustManager(InsecureTrustManagerFactory.INSTANCE.getTrustManagers());
250+
ManagedChannel certlessChannel = Grpc
251+
.newChannelBuilder(target, channelCredBuilder.build())
252+
.build();
253+
try {
254+
TikaGrpc.TikaBlockingStub certlessStub = TikaGrpc.newBlockingStub(certlessChannel);
255+
Assertions.assertThrows(StatusRuntimeException.class, () -> certlessStub.fetchAndParse(FetchAndParseRequest
256+
.newBuilder()
257+
.setFetcherId(httpFetcherId)
258+
.setFetchKey(httpServerUrl + "/" + files.get(0))
259+
.build()));
260+
} finally {
261+
certlessChannel.shutdownNow();
262+
}
263+
}
239264
}

‎tika-grpc/src/test/java/org/apache/tika/pipes/grpc/TikaGrpcServerTlsTest.java‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.tika.pipes.grpc;
18+
19+
import static org.junit.jupiter.api.Assertions.assertThrows;
20+
21+
import java.io.File;
22+
import java.nio.file.Files;
23+
import java.nio.file.Path;
24+
import java.nio.file.Paths;
25+
26+
import org.junit.jupiter.api.Assumptions;
27+
import org.junit.jupiter.api.Test;
28+
import org.junit.jupiter.api.io.TempDir;
29+
30+
/**
31+
* Covers the trust-cert-collection states that {@link TikaGrpcServer#start()} should refuse
32+
* to start on when {@code --client-auth-required} is set: omitted, nonexistent, unreadable,
33+
* and invalid/corrupt content, plus {@code --client-auth-required} without {@code --secure}.
34+
*/
35+
class TikaGrpcServerTlsTest {
36+
private static final File CERT_CHAIN = Paths.get("src", "test", "resources", "certs", "server1.pem").toFile();
37+
private static final File PRIVATE_KEY = Paths.get("src", "test", "resources", "certs", "server1.key").toFile();
38+
private static final File VALID_TRUST_COLLECTION = Paths.get("src", "test", "resources", "certs", "ca.pem").toFile();
39+
40+
@Test
41+
void clientAuthRequiredWithoutSecureRefusesToStart() {
42+
TikaGrpcServer server = new TikaGrpcServer()
43+
.setSecure(false)
44+
.setClientAuthRequired(true);
45+
assertThrows(IllegalArgumentException.class, server::start);
46+
}
47+
48+
@Test
49+
void clientAuthRequiredWithOmittedTrustCollectionRefusesToStart() {
50+
TikaGrpcServer server = new TikaGrpcServer()
51+
.setSecure(true)
52+
.setCertChain(CERT_CHAIN)
53+
.setPrivateKey(PRIVATE_KEY)
54+
.setClientAuthRequired(true);
55+
// trustCertCollection intentionally left unset
56+
assertThrows(IllegalArgumentException.class, server::start);
57+
}
58+
59+
@Test
60+
void clientAuthRequiredWithNonexistentTrustCollectionRefusesToStart() {
61+
TikaGrpcServer server = new TikaGrpcServer()
62+
.setSecure(true)
63+
.setCertChain(CERT_CHAIN)
64+
.setPrivateKey(PRIVATE_KEY)
65+
.setTrustCertCollection(new File("does-not-exist-" + System.nanoTime() + ".pem"))
66+
.setClientAuthRequired(true);
67+
assertThrows(IllegalArgumentException.class, server::start);
68+
}
69+
70+
@Test
71+
void clientAuthRequiredWithUnreadableTrustCollectionRefusesToStart(@TempDir Path tempDir) throws Exception {
72+
Path unreadable = tempDir.resolve("unreadable-ca.pem");
73+
Files.copy(VALID_TRUST_COLLECTION.toPath(), unreadable);
74+
boolean changed = unreadable.toFile().setReadable(false, false);
75+
Assumptions.assumeTrue(changed && !unreadable.toFile().canRead(),
76+
"cannot simulate an unreadable file as the current user (likely running as root)");
77+
78+
TikaGrpcServer server = new TikaGrpcServer()
79+
.setSecure(true)
80+
.setCertChain(CERT_CHAIN)
81+
.setPrivateKey(PRIVATE_KEY)
82+
.setTrustCertCollection(unreadable.toFile())
83+
.setClientAuthRequired(true);
84+
assertThrows(IllegalArgumentException.class, server::start);
85+
}
86+
87+
@Test
88+
void clientAuthRequiredWithCorruptTrustCollectionRefusesToStart(@TempDir Path tempDir) throws Exception {
89+
Path corrupt = tempDir.resolve("corrupt-ca.pem");
90+
Files.write(corrupt, "this is not a valid PEM certificate".getBytes(java.nio.charset.StandardCharsets.UTF_8));
91+
92+
TikaGrpcServer server = new TikaGrpcServer()
93+
.setSecure(true)
94+
.setCertChain(CERT_CHAIN)
95+
.setPrivateKey(PRIVATE_KEY)
96+
.setTrustCertCollection(corrupt.toFile())
97+
.setClientAuthRequired(true);
98+
// Content validation happens deeper in grpc's TLS credential building, so this
99+
// surfaces as a propagated exception rather than our explicit IllegalArgumentException.
100+
assertThrows(Exception.class, server::start);
101+
}
102+
}

‎tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/TikaServerConfig.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ private void validateConsistency(Set<String> settings) throws TikaConfigExceptio
205205
"files and reach network resources.");
206206
}
207207
}
208+
tlsConfig.checkInitialization();
208209
}
209210

210211
public String getHost() {

‎tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/TikaServerProcess.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,14 @@ private static ServerDetails initServer(TikaServerConfig tikaServerConfig) throw
240240
return details;
241241
}
242242

243-
private static TLSServerParameters getTlsParams(TlsConfig tlsConfig) throws GeneralSecurityException, IOException {
243+
private static TLSServerParameters getTlsParams(TlsConfig tlsConfig)
244+
throws GeneralSecurityException, IOException, TikaConfigException {
245+
// Also checked in TlsConfig.checkInitialization() at config-load time; kept here too
246+
// since this is where the TLS credentials are actually built.
247+
if (tlsConfig.isClientAuthenticationRequired() && !tlsConfig.hasTrustStore()) {
248+
throw new TikaConfigException(
249+
"requiring client authentication, but no trust store has been specified");
250+
}
244251
KeyStoreType keyStore = new KeyStoreType();
245252
keyStore.setType(tlsConfig.getKeyStoreType());
246253
keyStore.setPassword(tlsConfig.getKeyStorePassword());

‎tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerConfigTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,4 +157,76 @@ public void testTlsConfig() throws Exception {
157157
assertEquals("pass2", tlsConfig.getTrustStorePassword());
158158
assertEquals("/something/or/other2", tlsConfig.getTrustStoreFile());
159159
}
160+
161+
// The four cases below exercise TLS config validation through the CommandLine-based
162+
// TikaServerConfig.load() overload, rather than the Path-based overload testTlsConfig()
163+
// above uses.
164+
165+
@Test
166+
public void testClientAuthRequiredWithoutTrustStoreRefusesToLoad() throws Exception {
167+
CommandLineParser parser = new DefaultParser();
168+
Path path = getConfigPath(getClass(), "tika-config-server-tls-client-auth-no-truststore.json");
169+
CommandLine commandLine = parser.parse(new Options()
170+
.addOption(Option
171+
.builder("c")
172+
.longOpt("config")
173+
.hasArg()
174+
.get()), new String[]{"-c", ProcessUtils.escapeCommandLine(path
175+
.toAbsolutePath()
176+
.toString())});
177+
TikaConfigException ex = assertThrows(TikaConfigException.class,
178+
() -> TikaServerConfig.load(commandLine));
179+
assertContains("client authentication", ex.getMessage());
180+
assertContains("no trust store", ex.getMessage());
181+
}
182+
183+
@Test
184+
public void testPartialTrustStoreConfigRefusesToLoad() throws Exception {
185+
CommandLineParser parser = new DefaultParser();
186+
Path path = getConfigPath(getClass(), "tika-config-server-tls-partial-truststore.json");
187+
CommandLine commandLine = parser.parse(new Options()
188+
.addOption(Option
189+
.builder("c")
190+
.longOpt("config")
191+
.hasArg()
192+
.get()), new String[]{"-c", ProcessUtils.escapeCommandLine(path
193+
.toAbsolutePath()
194+
.toString())});
195+
TikaConfigException ex = assertThrows(TikaConfigException.class,
196+
() -> TikaServerConfig.load(commandLine));
197+
assertContains("Partial truststore configuration", ex.getMessage());
198+
}
199+
200+
@Test
201+
public void testMissingKeyStoreFileRefusesToLoad() throws Exception {
202+
CommandLineParser parser = new DefaultParser();
203+
Path path = getConfigPath(getClass(), "tika-config-server-tls-missing-keystore.json");
204+
CommandLine commandLine = parser.parse(new Options()
205+
.addOption(Option
206+
.builder("c")
207+
.longOpt("config")
208+
.hasArg()
209+
.get()), new String[]{"-c", ProcessUtils.escapeCommandLine(path
210+
.toAbsolutePath()
211+
.toString())});
212+
TikaConfigException ex = assertThrows(TikaConfigException.class,
213+
() -> TikaServerConfig.load(commandLine));
214+
assertContains("keyStoreFile does not exist", ex.getMessage());
215+
}
216+
217+
@Test
218+
public void testClientAuthRequiredWithValidTrustStoreLoadsSuccessfully() throws Exception {
219+
CommandLineParser parser = new DefaultParser();
220+
Path path = getConfigPath(getClass(), "tika-config-server-tls-client-auth-valid.json");
221+
CommandLine commandLine = parser.parse(new Options()
222+
.addOption(Option
223+
.builder("c")
224+
.longOpt("config")
225+
.hasArg()
226+
.get()), new String[]{"-c", ProcessUtils.escapeCommandLine(path
227+
.toAbsolutePath()
228+
.toString())});
229+
TikaServerConfig config = TikaServerConfig.load(commandLine);
230+
assertTrue(config.getTlsConfig().isClientAuthenticationRequired());
231+
}
160232
}

‎tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerProcessTlsGuardTest.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.tika.server.core;
18+
19+
import static org.junit.jupiter.api.Assertions.assertThrows;
20+
import static org.junit.jupiter.api.Assertions.assertTrue;
21+
22+
import java.lang.reflect.InvocationTargetException;
23+
import java.lang.reflect.Method;
24+
import java.nio.file.Paths;
25+
26+
import org.apache.cxf.configuration.jsse.TLSServerParameters;
27+
import org.junit.jupiter.api.Test;
28+
29+
import org.apache.tika.exception.TikaConfigException;
30+
31+
/**
32+
* Exercises TikaServerProcess.getTlsParams() directly via reflection, independent of
33+
* TikaServerConfig.load(), to confirm its own TLS config checks hold on their own.
34+
*/
35+
class TikaServerProcessTlsGuardTest {
36+
37+
private static TLSServerParameters invokeGetTlsParams(TlsConfig tlsConfig) throws Throwable {
38+
try {
39+
Method m = TikaServerProcess.class.getDeclaredMethod("getTlsParams", TlsConfig.class);
40+
m.setAccessible(true);
41+
return (TLSServerParameters) m.invoke(null, tlsConfig);
42+
} catch (InvocationTargetException e) {
43+
throw e.getCause();
44+
}
45+
}
46+
47+
private static TlsConfig validKeyStoreOnlyConfig() {
48+
TlsConfig tlsConfig = new TlsConfig();
49+
tlsConfig.setActive(true);
50+
tlsConfig.setKeyStoreType("PKCS12");
51+
tlsConfig.setKeyStorePassword("tika-secret");
52+
tlsConfig.setKeyStoreFile(Paths
53+
.get("src", "test", "resources", "ssl-keys", "tika-server-keystore.p12")
54+
.toString());
55+
return tlsConfig;
56+
}
57+
58+
@Test
59+
void getTlsParamsRefusesClientAuthRequiredWithoutTrustStore() throws Throwable {
60+
TlsConfig tlsConfig = validKeyStoreOnlyConfig();
61+
tlsConfig.setClientAuthenticationRequired(true);
62+
// trust store intentionally left unset
63+
64+
TikaConfigException ex = assertThrows(TikaConfigException.class,
65+
() -> invokeGetTlsParams(tlsConfig));
66+
assertTrue(ex.getMessage().contains("no trust store"));
67+
}
68+
69+
@Test
70+
void getTlsParamsAllowsClientAuthRequiredWithTrustStore() throws Throwable {
71+
TlsConfig tlsConfig = validKeyStoreOnlyConfig();
72+
tlsConfig.setTrustStoreType("PKCS12");
73+
tlsConfig.setTrustStorePassword("tika-secret");
74+
tlsConfig.setTrustStoreFile(Paths
75+
.get("src", "test", "resources", "ssl-keys", "tika-server-truststore.p12")
76+
.toString());
77+
tlsConfig.setClientAuthenticationRequired(true);
78+
79+
TLSServerParameters params = invokeGetTlsParams(tlsConfig);
80+
assertTrue(params.getClientAuthentication().isRequired());
81+
}
82+
}

‎tika-server/tika-server-core/src/test/resources/configs/tika-config-server-tls-client-auth-no-truststore.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"server": {
3+
"port": 9999,
4+
"endpoints": [
5+
"status"
6+
],
7+
"tlsConfig": {
8+
"active": true,
9+
"keyStoreType": "PKCS12",
10+
"keyStorePassword": "tika-secret",
11+
"keyStoreFile": "src/test/resources/ssl-keys/tika-server-keystore.p12",
12+
"clientAuthenticationRequired": true
13+
}
14+
}
15+
}

‎tika-server/tika-server-core/src/test/resources/configs/tika-config-server-tls-client-auth-valid.json‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"server": {
3+
"port": 9999,
4+
"endpoints": [
5+
"status"
6+
],
7+
"tlsConfig": {
8+
"active": true,
9+
"keyStoreType": "PKCS12",
10+
"keyStorePassword": "tika-secret",
11+
"keyStoreFile": "src/test/resources/ssl-keys/tika-server-keystore.p12",
12+
"trustStoreType": "PKCS12",
13+
"trustStorePassword": "tika-secret",
14+
"trustStoreFile": "src/test/resources/ssl-keys/tika-server-truststore.p12",
15+
"clientAuthenticationRequired": true
16+
}
17+
}
18+
}

‎tika-server/tika-server-core/src/test/resources/configs/tika-config-server-tls-missing-keystore.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"server": {
3+
"port": 9999,
4+
"endpoints": [
5+
"status"
6+
],
7+
"tlsConfig": {
8+
"active": true,
9+
"keyStoreType": "PKCS12",
10+
"keyStorePassword": "tika-secret",
11+
"keyStoreFile": "src/test/resources/ssl-keys/does-not-exist.p12"
12+
}
13+
}
14+
}

0 commit comments

Comments
 (0)