Skip to content

Commit 9b5f6ba

Browse files
authored
Close the keystore file in SslContextAwareAbstractSource (#506)
* Close the keystore file in `SslContextAwareAbstractSource` The keystore was loaded through a `FileInputStream` that was never closed, both when validating the configuration and each time an `SSLContext` was created. The leaked handle keeps the keystore locked on Windows until the stream is garbage collected, which makes tests that place the keystore in a JUnit `@TempDir` fail during cleanup. Assisted-By: Claude Fable 5.1 <noreply@anthropic.com> * Link the changelog entry to the pull request Assisted-By: Claude Fable 5.1 <noreply@anthropic.com> * Add the pull request link to the changelog entry Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 2890a89 commit 9b5f6ba

3 files changed

Lines changed: 138 additions & 3 deletions

File tree

flume-ng-core/src/main/java/org/apache/flume/source/SslContextAwareAbstractSource.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.apache.flume.source;
1818

1919
import java.io.FileInputStream;
20+
import java.io.InputStream;
2021
import java.security.KeyStore;
2122
import java.util.Arrays;
2223
import java.util.LinkedHashSet;
@@ -104,9 +105,9 @@ protected void configureSsl(Context context) {
104105
if (sslEnabled) {
105106
Objects.requireNonNull(keystore, KEYSTORE_KEY + " must be specified when SSL is enabled");
106107
Objects.requireNonNull(keystorePassword, KEYSTORE_PASSWORD_KEY + " must be specified when SSL is enabled");
107-
try {
108+
try (InputStream in = new FileInputStream(keystore)) {
108109
KeyStore ks = KeyStore.getInstance(keystoreType);
109-
ks.load(new FileInputStream(keystore), keystorePassword.toCharArray());
110+
ks.load(in, keystorePassword.toCharArray());
110111
} catch (Exception ex) {
111112
throw new FlumeException("Source " + getName() + " configured with invalid keystore: " + keystore, ex);
112113
}
@@ -117,7 +118,9 @@ private Optional<SSLContext> getSslContext() {
117118
if (sslEnabled) {
118119
try {
119120
KeyStore ks = KeyStore.getInstance(keystoreType);
120-
ks.load(new FileInputStream(keystore), keystorePassword.toCharArray());
121+
try (InputStream in = new FileInputStream(keystore)) {
122+
ks.load(in, keystorePassword.toCharArray());
123+
}
121124

122125
// can be set with "ssl.KeyManagerFactory.algorithm"
123126
String algorithm = KeyManagerFactory.getDefaultAlgorithm();
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
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.flume.source;
18+
19+
import static org.junit.Assert.assertFalse;
20+
import static org.junit.Assert.assertTrue;
21+
import static org.junit.Assume.assumeTrue;
22+
23+
import java.io.IOException;
24+
import java.nio.file.Files;
25+
import java.nio.file.Path;
26+
import java.nio.file.Paths;
27+
import java.util.List;
28+
import java.util.stream.Collectors;
29+
import java.util.stream.Stream;
30+
import org.apache.flume.Context;
31+
import org.apache.flume.FlumeException;
32+
import org.apache.flume.sdk.test.TestKeyStores;
33+
import org.junit.Before;
34+
import org.junit.Rule;
35+
import org.junit.Test;
36+
import org.junit.rules.TemporaryFolder;
37+
38+
public class TestSslContextAwareAbstractSource {
39+
40+
private static final String KEYSTORE_PASSWORD = "password";
41+
42+
@Rule
43+
public final TemporaryFolder tempFolder = new TemporaryFolder();
44+
45+
private Path keystore;
46+
47+
/** Minimal concrete source exposing the SSL configuration of the abstract class. */
48+
private static final class TestSource extends SslContextAwareAbstractSource {
49+
void configure(Context context) {
50+
configureSsl(context);
51+
}
52+
}
53+
54+
@Before
55+
public void writeKeystore() throws Exception {
56+
keystore = TestKeyStores.selfSigned("CN=localhost")
57+
.writeKeyStore(tempFolder.newFile("keystore.jks").toPath(), "JKS", KEYSTORE_PASSWORD);
58+
}
59+
60+
private Context sslContext(String password) {
61+
Context context = new Context();
62+
context.put("ssl", "true");
63+
context.put("keystore", keystore.toString());
64+
context.put("keystore-password", password);
65+
return context;
66+
}
67+
68+
private TestSource configuredSource() {
69+
TestSource source = new TestSource();
70+
source.configure(sslContext(KEYSTORE_PASSWORD));
71+
return source;
72+
}
73+
74+
@Test
75+
public void sslContextIsCreatedFromKeystore() {
76+
TestSource source = configuredSource();
77+
assertTrue(source.isSslEnabled());
78+
assertTrue(source.getSslContextSupplier().get().isPresent());
79+
assertTrue(source.getSslEngineSupplier(false).get().isPresent());
80+
}
81+
82+
@Test
83+
public void noSslContextWhenSslIsDisabled() {
84+
TestSource source = new TestSource();
85+
source.configure(new Context());
86+
assertFalse(source.isSslEnabled());
87+
assertFalse(source.getSslContextSupplier().get().isPresent());
88+
}
89+
90+
@Test(expected = FlumeException.class)
91+
public void wrongKeystorePasswordIsRejected() {
92+
new TestSource().configure(sslContext("wrong"));
93+
}
94+
95+
@Test
96+
public void keystoreIsDeletableAfterUse() {
97+
// The source must not keep the keystore open: on Windows an open file cannot be deleted
98+
assertTrue(configuredSource().getSslContextSupplier().get().isPresent());
99+
assertTrue("The keystore is still open", keystore.toFile().delete());
100+
}
101+
102+
@Test
103+
public void keystoreDescriptorIsReleased() throws IOException {
104+
// On Linux the open file descriptors of the process are listed under `/proc/self/fd`
105+
Path fdDir = Paths.get("/proc/self/fd");
106+
assumeTrue("Not running on Linux", Files.isDirectory(fdDir));
107+
assertTrue(configuredSource().getSslContextSupplier().get().isPresent());
108+
String keystorePath = keystore.toRealPath().toString();
109+
try (Stream<Path> fds = Files.list(fdDir)) {
110+
List<Path> open =
111+
fds.filter(fd -> keystorePath.equals(readLink(fd))).collect(Collectors.toList());
112+
assertTrue("File descriptors still open on the keystore: " + open, open.isEmpty());
113+
}
114+
}
115+
116+
private static String readLink(Path fd) {
117+
try {
118+
return Files.readSymbolicLink(fd).toString();
119+
} catch (IOException e) {
120+
// The descriptor was closed between the listing and the read
121+
return null;
122+
}
123+
}
124+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xmlns="https://logging.apache.org/xml/ns"
4+
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
5+
type="fixed">
6+
<issue id="506" link="https://github.com/apache/logging-flume/pull/506"/>
7+
<description format="asciidoc">Fix keystore file descriptor leak in `SslContextAwareAbstractSource`.</description>
8+
</entry>

0 commit comments

Comments
 (0)