NIFI-11536 Corrected Keystore and Truststore auto-reloading

- Replaced Jetty KeyStoreScanner and custom TrustStoreScanner with shared StoreScanner
- New StoreScanner uses TLS Configuration to reload SSLContext instead of relying on Jetty SslContextFactory properties

This closes #7446

Signed-off-by: David Handermann <exceptionfactory@apache.org>
This commit is contained in:
Emilio Setiadarma 2023-06-26 18:00:06 -07:00 committed by exceptionfactory
parent d24318cdb8
commit a85ef2c1f4
No known key found for this signature in database
GPG Key ID: 29B6A52D2AAE8DBA
6 changed files with 308 additions and 247 deletions

View File

@ -27,10 +27,9 @@ import org.apache.nifi.security.util.TlsException;
import org.apache.nifi.security.util.TlsPlatform;
import org.apache.nifi.util.FormatUtils;
import org.apache.nifi.util.NiFiProperties;
import org.apache.nifi.web.server.util.TrustStoreScanner;
import org.apache.nifi.web.server.util.StoreScanner;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.util.ssl.KeyStoreScanner;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import javax.net.ssl.SSLContext;
@ -149,12 +148,12 @@ public class FrameworkServerConnectorFactory extends StandardServerConnectorFact
if (storeScanInterval != null) {
sslContextFactory.setKeyStorePath(tlsConfiguration.getKeystorePath());
final KeyStoreScanner keyStoreScanner = new KeyStoreScanner(sslContextFactory);
final StoreScanner keyStoreScanner = new StoreScanner(sslContextFactory, tlsConfiguration, sslContextFactory.getKeyStoreResource());
keyStoreScanner.setScanInterval(storeScanInterval);
getServer().addBean(keyStoreScanner);
sslContextFactory.setTrustStorePath(tlsConfiguration.getTruststorePath());
final TrustStoreScanner trustStoreScanner = new TrustStoreScanner(sslContextFactory);
final StoreScanner trustStoreScanner = new StoreScanner(sslContextFactory, tlsConfiguration, sslContextFactory.getTrustStoreResource());
trustStoreScanner.setScanInterval(storeScanInterval);
getServer().addBean(trustStoreScanner);
}

View File

@ -0,0 +1,158 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.nifi.web.server.util;
import org.apache.nifi.security.util.TlsConfiguration;
import org.apache.nifi.security.util.TlsException;
import org.eclipse.jetty.util.Scanner;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedOperation;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import javax.net.ssl.SSLContext;
import java.io.File;
import java.io.IOException;
import java.util.Collections;
import static org.apache.nifi.security.util.SslContextFactory.createSslContext;
/**
* File Scanner for Keystore or Truststore reloading using provided TLS Configuration
*/
public class StoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener {
private static final Logger LOG = Log.getLogger(StoreScanner.class);
private final SslContextFactory sslContextFactory;
private final TlsConfiguration tlsConfiguration;
private final File file;
private final Scanner scanner;
private final String resourceName;
public StoreScanner(final SslContextFactory sslContextFactory,
final TlsConfiguration tlsConfiguration,
final Resource resource) {
this.sslContextFactory = sslContextFactory;
this.tlsConfiguration = tlsConfiguration;
this.resourceName = resource.getName();
try {
File monitoredFile = resource.getFile();
if (monitoredFile == null || !monitoredFile.exists()) {
throw new IllegalArgumentException(String.format("%s file does not exist", resourceName));
}
if (monitoredFile.isDirectory()) {
throw new IllegalArgumentException(String.format("expected %s file not directory", resourceName));
}
if (resource.getAlias() != null) {
// this resource has an alias, use the alias, as that's what's returned in the Scanner
monitoredFile = new File(resource.getAlias());
}
file = monitoredFile;
if (LOG.isDebugEnabled()) {
LOG.debug("File monitoring started {} [{}]", resourceName, monitoredFile);
}
} catch (final IOException e) {
throw new IllegalArgumentException(String.format("could not obtain %s file", resourceName), e);
}
File parentFile = file.getParentFile();
if (!parentFile.exists() || !parentFile.isDirectory()) {
throw new IllegalArgumentException(String.format("error obtaining %s dir", resourceName));
}
scanner = new Scanner();
scanner.setScanDirs(Collections.singletonList(parentFile));
scanner.setScanInterval(1);
scanner.setReportDirs(false);
scanner.setReportExistingFilesOnStartup(false);
scanner.setScanDepth(1);
scanner.addListener(this);
addBean(scanner);
}
@Override
public void fileAdded(final String filename) {
LOG.debug("Resource [{}] File [{}] added", resourceName, filename);
reloadMatched(filename);
}
@Override
public void fileChanged(final String filename) {
LOG.debug("Resource [{}] File [{}] changed", resourceName, filename);
reloadMatched(filename);
}
@Override
public void fileRemoved(final String filename) {
LOG.debug("Resource [{}] File [{}] removed", resourceName, filename);
reloadMatched(filename);
}
@ManagedOperation(
value = "Scan for changes in the SSL Keystore/Truststore",
impact = "ACTION"
)
public void scan() {
LOG.debug("Resource [{}] scanning started", resourceName);
this.scanner.scan();
this.scanner.scan();
}
@ManagedOperation(
value = "Reload the SSL Keystore/Truststore",
impact = "ACTION"
)
public void reload() {
LOG.debug("File [{}] reload started", file);
try {
this.sslContextFactory.reload(contextFactory -> contextFactory.setSslContext(createContext()));
LOG.info("File [{}] reload completed", file);
} catch (final Throwable t) {
LOG.warn("File [{}] reload failed", file, t);
}
}
@ManagedAttribute("scanning interval to detect changes which need reloaded")
public int getScanInterval() {
return this.scanner.getScanInterval();
}
public void setScanInterval(int scanInterval) {
this.scanner.setScanInterval(scanInterval);
}
private void reloadMatched(final String filename) {
if (file.toPath().toString().equals(filename)) {
reload();
}
}
private SSLContext createContext() {
try {
return createSslContext(tlsConfiguration);
} catch (final TlsException e) {
throw new IllegalArgumentException("Failed to create SSL context with the TLS configuration", e);
}
}
}

View File

@ -1,151 +0,0 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.nifi.web.server.util;
import org.eclipse.jetty.util.Scanner;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedOperation;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import java.io.File;
import java.io.IOException;
import java.util.Collections;
/**
* <p>The {@link TrustStoreScanner} is used to monitor the TrustStore file used by the {@link SslContextFactory}.
* It will reload the {@link SslContextFactory} if it detects that the TrustStore file has been modified.</p>
* <p>
* Though it would have been more ideal to simply extend KeyStoreScanner and override the keystore resource
* with the truststore resource, KeyStoreScanner's constructor was written in a way that doesn't make this possible.
*/
public class TrustStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener {
private static final Logger LOG = Log.getLogger(TrustStoreScanner.class);
private final SslContextFactory sslContextFactory;
private final File truststoreFile;
private final Scanner _scanner;
public TrustStoreScanner(SslContextFactory sslContextFactory) {
this.sslContextFactory = sslContextFactory;
try {
Resource truststoreResource = sslContextFactory.getTrustStoreResource();
File monitoredFile = truststoreResource.getFile();
if (monitoredFile == null || !monitoredFile.exists()) {
throw new IllegalArgumentException("truststore file does not exist");
}
if (monitoredFile.isDirectory()) {
throw new IllegalArgumentException("expected truststore file not directory");
}
if (truststoreResource.getAlias() != null) {
// this resource has an alias, use the alias, as that's what's returned in the Scanner
monitoredFile = new File(truststoreResource.getAlias());
}
truststoreFile = monitoredFile;
if (LOG.isDebugEnabled()) {
LOG.debug("Monitored Truststore File: {}", monitoredFile);
}
} catch (IOException e) {
throw new IllegalArgumentException("could not obtain truststore file", e);
}
File parentFile = truststoreFile.getParentFile();
if (!parentFile.exists() || !parentFile.isDirectory()) {
throw new IllegalArgumentException("error obtaining truststore dir");
}
_scanner = new Scanner();
_scanner.setScanDirs(Collections.singletonList(parentFile));
_scanner.setScanInterval(1);
_scanner.setReportDirs(false);
_scanner.setReportExistingFilesOnStartup(false);
_scanner.setScanDepth(1);
_scanner.addListener(this);
addBean(_scanner);
}
@Override
public void fileAdded(String filename) {
if (LOG.isDebugEnabled()) {
LOG.debug("added {}", filename);
}
if (truststoreFile.toPath().toString().equals(filename)) {
reload();
}
}
@Override
public void fileChanged(String filename) {
if (LOG.isDebugEnabled()) {
LOG.debug("changed {}", filename);
}
if (truststoreFile.toPath().toString().equals(filename)) {
reload();
}
}
@Override
public void fileRemoved(String filename) {
if (LOG.isDebugEnabled()) {
LOG.debug("removed {}", filename);
}
if (truststoreFile.toPath().toString().equals(filename)) {
reload();
}
}
@ManagedOperation(value = "Scan for changes in the SSL Truststore", impact = "ACTION")
public void scan() {
if (LOG.isDebugEnabled()) {
LOG.debug("scanning");
}
_scanner.scan();
_scanner.scan();
}
@ManagedOperation(value = "Reload the SSL Truststore", impact = "ACTION")
public void reload() {
if (LOG.isDebugEnabled()) {
LOG.debug("reloading truststore file {}", truststoreFile);
}
try {
sslContextFactory.reload(scf -> {
});
} catch (Throwable t) {
LOG.warn("Truststore Reload Failed", t);
}
}
@ManagedAttribute("scanning interval to detect changes which need reloaded")
public int getScanInterval() {
return _scanner.getScanInterval();
}
public void setScanInterval(int scanInterval) {
_scanner.setScanInterval(scanInterval);
}
}

View File

@ -20,17 +20,17 @@ import org.apache.nifi.jetty.configuration.connector.alpn.ALPNServerConnectionFa
import org.apache.nifi.security.util.TemporaryKeyStoreBuilder;
import org.apache.nifi.security.util.TlsConfiguration;
import org.apache.nifi.util.NiFiProperties;
import org.apache.nifi.web.server.util.TrustStoreScanner;
import org.apache.nifi.web.server.util.StoreScanner;
import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.SslConnectionFactory;
import org.eclipse.jetty.util.ssl.KeyStoreScanner;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import java.util.Collection;
import java.util.Properties;
import static org.junit.jupiter.api.Assertions.assertEquals;
@ -182,11 +182,8 @@ class FrameworkServerConnectorFactoryTest {
private void assertAutoReloadEnabled(final ServerConnector serverConnector) {
final Server server = serverConnector.getServer();
final KeyStoreScanner keyStoreScanner = server.getBean(KeyStoreScanner.class);
assertNotNull(keyStoreScanner);
final TrustStoreScanner trustStoreScanner = server.getBean(TrustStoreScanner.class);
assertNotNull(trustStoreScanner);
final Collection<StoreScanner> scanners = server.getBeans(StoreScanner.class);
assertEquals(2, scanners.size());
}
private NiFiProperties getProperties(final Properties serverProperties) {

View File

@ -0,0 +1,143 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.nifi.web.server.util;
import org.apache.nifi.security.util.TemporaryKeyStoreBuilder;
import org.apache.nifi.security.util.TlsConfiguration;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import org.mockito.Captor;
import org.mockito.junit.jupiter.MockitoExtension;
import javax.net.ssl.SSLContext;
import java.io.File;
import java.io.IOException;
import java.nio.file.Paths;
import java.util.HashMap;
import java.util.Map;
import java.util.function.Consumer;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
public class StoreScannerTest {
private SslContextFactory sslContextFactory;
private static TlsConfiguration tlsConfiguration;
private static File keyStoreFile;
private static File trustStoreFile;
private static Map<File, StoreScanner> filesToScannerMap;
@Captor
private ArgumentCaptor<Consumer<SslContextFactory>> consumerArgumentCaptor;
@BeforeAll
public static void initClass() {
tlsConfiguration = new TemporaryKeyStoreBuilder().build();
keyStoreFile = Paths.get(tlsConfiguration.getKeystorePath()).toFile();
trustStoreFile = Paths.get(tlsConfiguration.getTruststorePath()).toFile();
}
@BeforeEach
public void init() throws IOException {
sslContextFactory = mock(SslContextFactory.class);
Resource keyStoreResource = mock(Resource.class);
when(keyStoreResource.getFile()).thenReturn(keyStoreFile);
when(sslContextFactory.getKeyStoreResource()).thenReturn(keyStoreResource);
Resource trustStoreResource = mock(Resource.class);
when(trustStoreResource.getFile()).thenReturn(trustStoreFile);
when(sslContextFactory.getTrustStoreResource()).thenReturn(trustStoreResource);
final StoreScanner keyStoreScanner = new StoreScanner(sslContextFactory, tlsConfiguration, sslContextFactory.getKeyStoreResource());
final StoreScanner trustStoreScanner = new StoreScanner(sslContextFactory, tlsConfiguration, sslContextFactory.getTrustStoreResource());
filesToScannerMap = new HashMap<>();
filesToScannerMap.put(keyStoreFile, keyStoreScanner);
filesToScannerMap.put(trustStoreFile, trustStoreScanner);
}
@Test
public void testFileAdded() throws Exception {
for (final Map.Entry<File, StoreScanner> entry : filesToScannerMap.entrySet()) {
final File file = entry.getKey();
final StoreScanner scanner = entry.getValue();
scanner.fileAdded(file.getAbsolutePath());
verify(sslContextFactory).reload(consumerArgumentCaptor.capture());
final Consumer<SslContextFactory> consumer = consumerArgumentCaptor.getValue();
consumer.accept(sslContextFactory);
verify(sslContextFactory).setSslContext(ArgumentMatchers.any(SSLContext.class));
clearInvocations(sslContextFactory);
}
}
@Test
public void testFileChanged() throws Exception {
for (final Map.Entry<File, StoreScanner> entry : filesToScannerMap.entrySet()) {
final File file = entry.getKey();
final StoreScanner scanner = entry.getValue();
scanner.fileChanged(file.getAbsolutePath());
verify(sslContextFactory).reload(consumerArgumentCaptor.capture());
final Consumer<SslContextFactory> consumer = consumerArgumentCaptor.getValue();
consumer.accept(sslContextFactory);
verify(sslContextFactory).setSslContext(ArgumentMatchers.any(SSLContext.class));
clearInvocations(sslContextFactory);
}
}
@Test
public void testFileRemoved() throws Exception {
for (final Map.Entry<File, StoreScanner> entry : filesToScannerMap.entrySet()) {
final File file = entry.getKey();
final StoreScanner scanner = entry.getValue();
scanner.fileRemoved(file.getAbsolutePath());
verify(sslContextFactory).reload(consumerArgumentCaptor.capture());
final Consumer<SslContextFactory> consumer = consumerArgumentCaptor.getValue();
consumer.accept(sslContextFactory);
verify(sslContextFactory).setSslContext(ArgumentMatchers.any(SSLContext.class));
clearInvocations(sslContextFactory);
}
}
@Test
public void testReload() throws Exception {
for (final Map.Entry<File, StoreScanner> entry : filesToScannerMap.entrySet()) {
final StoreScanner scanner = entry.getValue();
scanner.reload();
verify(sslContextFactory).reload(consumerArgumentCaptor.capture());
final Consumer<SslContextFactory> consumer = consumerArgumentCaptor.getValue();
consumer.accept(sslContextFactory);
verify(sslContextFactory).setSslContext(ArgumentMatchers.any(SSLContext.class));
clearInvocations(sslContextFactory);
}
}
}

View File

@ -1,85 +0,0 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.nifi.web.server.util;
import org.apache.nifi.security.util.TemporaryKeyStoreBuilder;
import org.apache.nifi.security.util.TlsConfiguration;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentMatchers;
import org.mockito.Mockito;
import java.io.File;
import java.io.IOException;
import java.nio.file.Paths;
import java.util.function.Consumer;
public class TrustStoreScannerTest {
private TrustStoreScanner scanner;
private SslContextFactory sslContextFactory;
private static File keyStoreFile;
private static File trustStoreFile;
@BeforeAll
public static void initClass() {
TlsConfiguration tlsConfiguration = new TemporaryKeyStoreBuilder().build();
keyStoreFile = Paths.get(tlsConfiguration.getKeystorePath()).toFile();
trustStoreFile = Paths.get(tlsConfiguration.getTruststorePath()).toFile();
}
@BeforeEach
public void init() throws IOException {
sslContextFactory = Mockito.mock(SslContextFactory.class);
Resource trustStoreResource = Mockito.mock(Resource.class);
Mockito.when(trustStoreResource.getFile()).thenReturn(trustStoreFile);
Mockito.when(sslContextFactory.getTrustStoreResource()).thenReturn(trustStoreResource);
scanner = new TrustStoreScanner(sslContextFactory);
}
@Test
public void fileAdded() throws Exception {
scanner.fileAdded(trustStoreFile.getAbsolutePath());
Mockito.verify(sslContextFactory).reload(ArgumentMatchers.any(Consumer.class));
}
@Test
public void fileChanged() throws Exception {
scanner.fileChanged(trustStoreFile.getAbsolutePath());
Mockito.verify(sslContextFactory).reload(ArgumentMatchers.any(Consumer.class));
}
@Test
public void fileRemoved() throws Exception {
scanner.fileRemoved(trustStoreFile.getAbsolutePath());
Mockito.verify(sslContextFactory).reload(ArgumentMatchers.any(Consumer.class));
}
@Test
public void reload() throws Exception {
scanner.reload();
Mockito.verify(sslContextFactory).reload(ArgumentMatchers.any(Consumer.class));
}
}