From d4e45728129a83c1fca12184e53c9834c70660e1 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 29 Jan 2024 20:30:25 +0100 Subject: [PATCH] Split out keystore-legacy Tracking down the lifecycle of SslContexts brings us to our handling of key material. This is currently tangled netconf-client-mdsal using our home-grown (and problematic) model. Split out netconf-keystore.yang and the baseline implementation into keystore-legacy. Now that it sits side-by-side with keystore-api, we can compare the two. Since this change requires us to intercept a hidden object, rework the test in terms of JUnit5. JIRA: NETCONF-1237 Change-Id: Id9d410e88ec588e148c5f1dff3aad574b3cc8328 Signed-off-by: Robert Varga --- artifacts/pom.xml | 5 + keystore/keystore-legacy/pom.xml | 38 ++++ .../legacy/AbstractNetconfKeystore.java | 109 ++++++++++ .../keystore/legacy/ConfigListener.java | 80 ++++++++ .../keystore/legacy/SecurityHelper.java | 59 ++++++ .../src/main/yang/netconf-keystore.yang | 0 keystore/pom.xml | 1 + plugins/netconf-client-mdsal/pom.xml | 170 ++++++++-------- .../DefaultSslHandlerFactoryProvider.java | 179 ++-------------- .../DefaultSslHandlerFactoryProviderTest.java | 191 +++++++++--------- 10 files changed, 486 insertions(+), 346 deletions(-) create mode 100644 keystore/keystore-legacy/pom.xml create mode 100644 keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/AbstractNetconfKeystore.java create mode 100644 keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/ConfigListener.java create mode 100644 keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/SecurityHelper.java rename {plugins/netconf-client-mdsal => keystore/keystore-legacy}/src/main/yang/netconf-keystore.yang (100%) diff --git a/artifacts/pom.xml b/artifacts/pom.xml index 74175125b0..8edbc87c1a 100644 --- a/artifacts/pom.xml +++ b/artifacts/pom.xml @@ -246,6 +246,11 @@ keystore-api ${project.version} + + org.opendaylight.netconf + keystore-legacy + ${project.version} + org.opendaylight.netconf keystore-none diff --git a/keystore/keystore-legacy/pom.xml b/keystore/keystore-legacy/pom.xml new file mode 100644 index 0000000000..637f4008fc --- /dev/null +++ b/keystore/keystore-legacy/pom.xml @@ -0,0 +1,38 @@ + + + + 4.0.0 + + + org.opendaylight.netconf + netconf-parent + 7.0.0-SNAPSHOT + ../../parent + + + keystore-legacy + bundle + ${project.artifactId} + Legacy NETCONF keystore + + + + org.opendaylight.mdsal + mdsal-binding-api + + + org.opendaylight.mdsal + mdsal-common-api + + + org.opendaylight.yangtools + concepts + + + diff --git a/keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/AbstractNetconfKeystore.java b/keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/AbstractNetconfKeystore.java new file mode 100644 index 0000000000..0cae9c5998 --- /dev/null +++ b/keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/AbstractNetconfKeystore.java @@ -0,0 +1,109 @@ +/* + * Copyright (c) 2024 PANTHEON.tech, s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.netconf.keystore.legacy; + +import static java.util.Objects.requireNonNull; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; +import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.opendaylight.mdsal.binding.api.DataBroker; +import org.opendaylight.mdsal.binding.api.DataTreeIdentifier; +import org.opendaylight.mdsal.common.api.LogicalDatastoreType; +import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.Keystore; +import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017._private.keys.PrivateKey; +import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.trusted.certificates.TrustedCertificate; +import org.opendaylight.yangtools.concepts.Immutable; +import org.opendaylight.yangtools.concepts.Registration; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; + +/** + * Abstract substrate for implementing security services based on the contents of {@link Keystore}. + */ +public abstract class AbstractNetconfKeystore { + @NonNullByDefault + protected record State( + Map privateKeys, + Map trustedCertificates) implements Immutable { + public static final State EMPTY = new State(Map.of(), Map.of()); + + public State { + privateKeys = Map.copyOf(privateKeys); + trustedCertificates = Map.copyOf(trustedCertificates); + } + } + + @NonNullByDefault + private record ConfigState( + Map privateKeys, + Map trustedCertificates) implements Immutable { + static final ConfigState EMPTY = new ConfigState(Map.of(), Map.of()); + + ConfigState { + privateKeys = Map.copyOf(privateKeys); + trustedCertificates = Map.copyOf(trustedCertificates); + } + } + + @NonNullByDefault + record ConfigStateBuilder( + HashMap privateKeys, + HashMap trustedCertificates) { + ConfigStateBuilder { + requireNonNull(privateKeys); + requireNonNull(trustedCertificates); + } + } + + private final AtomicReference<@NonNull ConfigState> state = new AtomicReference<>(ConfigState.EMPTY); + + private @Nullable Registration configListener; + + protected final void start(final DataBroker dataBroker) { + if (configListener == null) { + configListener = dataBroker.registerTreeChangeListener( + DataTreeIdentifier.of(LogicalDatastoreType.CONFIGURATION, InstanceIdentifier.create(Keystore.class)), + new ConfigListener(this)); + } + } + + protected final void stop() { + final var listener = configListener; + if (listener != null) { + configListener = null; + listener.close(); + state.set(ConfigState.EMPTY); + } + } + + protected abstract void onStateUpdated(@NonNull State newState); + + final void runUpdate(final Consumer<@NonNull ConfigStateBuilder> task) { + final var prevState = state.getAcquire(); + + final var builder = new ConfigStateBuilder(new HashMap<>(prevState.privateKeys), + new HashMap<>(prevState.trustedCertificates)); + task.accept(builder); + final var newState = new ConfigState(builder.privateKeys, builder.trustedCertificates); + + // Careful application -- check if listener is still up and whether the state was not updated. + if (configListener == null || state.compareAndExchangeRelease(prevState, newState) != prevState) { + return; + } + + // FIXME: compile to crypto + + onStateUpdated(new State(newState.privateKeys, newState.trustedCertificates)); + + // FIXME: tickle operational updater (which does not exist yet) + } +} diff --git a/keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/ConfigListener.java b/keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/ConfigListener.java new file mode 100644 index 0000000000..35d3c45939 --- /dev/null +++ b/keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/ConfigListener.java @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2024 PANTHEON.tech, s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.netconf.keystore.legacy; + +import static java.util.Objects.requireNonNull; + +import com.google.common.base.Stopwatch; +import java.util.List; +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.opendaylight.mdsal.binding.api.DataTreeChangeListener; +import org.opendaylight.mdsal.binding.api.DataTreeModification; +import org.opendaylight.netconf.keystore.legacy.AbstractNetconfKeystore.ConfigStateBuilder; +import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.Keystore; +import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017._private.keys.PrivateKey; +import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.trusted.certificates.TrustedCertificate; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@NonNullByDefault +record ConfigListener(AbstractNetconfKeystore keystore) implements DataTreeChangeListener { + private static final Logger LOG = LoggerFactory.getLogger(ConfigListener.class); + + ConfigListener { + requireNonNull(keystore); + } + + @Override + public void onInitialData() { + keystore.runUpdate(builder -> { + builder.privateKeys().clear(); + builder.trustedCertificates().clear(); + }); + } + + @Override + public void onDataTreeChanged(final List> changes) { + LOG.debug("Starting update with {} changes", changes.size()); + final var sw = Stopwatch.createStarted(); + keystore.runUpdate(builder -> onDataTreeChanged(builder, changes)); + LOG.debug("Update finished in {}", sw); + } + + private static void onDataTreeChanged(final ConfigStateBuilder builder, + final List> changes) { + for (var change : changes) { + LOG.debug("Processing change {}", change); + final var rootNode = change.getRootNode(); + + for (var mod : rootNode.getModifiedChildren(PrivateKey.class)) { + switch (mod.modificationType()) { + case SUBTREE_MODIFIED, WRITE -> { + final var privateKey = mod.dataAfter(); + builder.privateKeys().put(privateKey.requireName(), privateKey); + } + case DELETE -> builder.privateKeys().remove(mod.dataBefore().requireName()); + default -> { + // no-op + } + } + } + for (var mod : rootNode.getModifiedChildren(TrustedCertificate.class)) { + switch (mod.modificationType()) { + case SUBTREE_MODIFIED, WRITE -> { + final var trustedCertificate = mod.dataAfter(); + builder.trustedCertificates().put(trustedCertificate.requireName(), trustedCertificate); + } + case DELETE -> builder.trustedCertificates().remove(mod.dataBefore().requireName()); + default -> { + // no-op + } + } + } + } + } +} \ No newline at end of file diff --git a/keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/SecurityHelper.java b/keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/SecurityHelper.java new file mode 100644 index 0000000000..621240bbc2 --- /dev/null +++ b/keystore/keystore-legacy/src/main/java/org/opendaylight/netconf/keystore/legacy/SecurityHelper.java @@ -0,0 +1,59 @@ +/* + * Copyright (c) 2023 PANTHEON.tech, s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.netconf.keystore.legacy; + +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.KeyFactory; +import java.security.PrivateKey; +import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.PKCS8EncodedKeySpec; +import java.util.Base64; +import org.eclipse.jdt.annotation.NonNull; + +public final class SecurityHelper { + private CertificateFactory certFactory; + private KeyFactory dsaFactory; + private KeyFactory rsaFactory; + + public @NonNull PrivateKey getJavaPrivateKey(final String base64PrivateKey) throws GeneralSecurityException { + final var keySpec = new PKCS8EncodedKeySpec(base64Decode(base64PrivateKey)); + + if (rsaFactory == null) { + rsaFactory = KeyFactory.getInstance("RSA"); + } + try { + return rsaFactory.generatePrivate(keySpec); + } catch (InvalidKeySpecException ignore) { + // Ignored + } + + if (dsaFactory == null) { + dsaFactory = KeyFactory.getInstance("DSA"); + } + return dsaFactory.generatePrivate(keySpec); + } + + public @NonNull X509Certificate getCertificate(final String base64Certificate) throws GeneralSecurityException { + // TODO: https://stackoverflow.com/questions/43809909/is-certificatefactory-getinstancex-509-thread-safe + // indicates this is thread-safe in most cases, but can we get a better assurance? + if (certFactory == null) { + certFactory = CertificateFactory.getInstance("X.509"); + } + return (X509Certificate) certFactory.generateCertificate( + new ByteArrayInputStream(base64Decode(base64Certificate))); + } + + private static byte[] base64Decode(final String base64) { + return Base64.getMimeDecoder().decode(base64.getBytes(StandardCharsets.US_ASCII)); + } + +} \ No newline at end of file diff --git a/plugins/netconf-client-mdsal/src/main/yang/netconf-keystore.yang b/keystore/keystore-legacy/src/main/yang/netconf-keystore.yang similarity index 100% rename from plugins/netconf-client-mdsal/src/main/yang/netconf-keystore.yang rename to keystore/keystore-legacy/src/main/yang/netconf-keystore.yang diff --git a/keystore/pom.xml b/keystore/pom.xml index 71212f7bb1..d277fd0d44 100644 --- a/keystore/pom.xml +++ b/keystore/pom.xml @@ -30,6 +30,7 @@ keystore-api + keystore-legacy keystore-none diff --git a/plugins/netconf-client-mdsal/pom.xml b/plugins/netconf-client-mdsal/pom.xml index 7f2e04ca3e..b225cd928e 100644 --- a/plugins/netconf-client-mdsal/pom.xml +++ b/plugins/netconf-client-mdsal/pom.xml @@ -32,6 +32,11 @@ com.google.guava guava + + com.guicedee.services + javax.inject + true + io.netty netty-common @@ -45,167 +50,166 @@ netty-transport - org.checkerframework - checker-qual + jakarta.annotation + jakarta.annotation-api + provided + true - org.opendaylight.yangtools - concepts + org.checkerframework + checker-qual - org.opendaylight.yangtools - rfc8528-model-api + org.opendaylight.aaa + aaa-encrypt-service - org.opendaylight.yangtools - util + org.opendaylight.mdsal + mdsal-binding-api - org.opendaylight.yangtools - yang-common + org.opendaylight.mdsal + mdsal-binding-runtime-spi - org.opendaylight.yangtools - yang-data-api + org.opendaylight.mdsal + mdsal-common-api - org.opendaylight.yangtools - yang-data-codec-gson + org.opendaylight.mdsal + mdsal-dom-api - org.opendaylight.yangtools - yang-data-codec-xml + org.opendaylight.mdsal + mdsal-dom-spi - org.opendaylight.yangtools - yang-data-impl + org.opendaylight.mdsal.binding.model.ietf + rfc6991-ietf-inet-types - org.opendaylight.yangtools - yang-data-spi + org.opendaylight.mdsal.binding.model.ietf + rfc6991-ietf-yang-types - org.opendaylight.yangtools - yang-data-util + org.opendaylight.mdsal.binding.model.ietf + rfc8525 - org.opendaylight.yangtools - yang-model-api + org.opendaylight.netconf + netconf-api - org.opendaylight.yangtools - yang-model-spi + org.opendaylight.netconf + keystore-legacy - org.opendaylight.yangtools - yang-model-util + org.opendaylight.netconf + netconf-client - org.opendaylight.yangtools - yang-parser-api + org.opendaylight.netconf + netconf-common - org.opendaylight.yangtools - yang-parser-impl + org.opendaylight.netconf + netconf-common-mdsal - org.opendaylight.yangtools - yang-parser-rfc7950 + org.opendaylight.netconf + netconf-dom-api - org.opendaylight.yangtools - yang-repo-api + org.opendaylight.netconf.model + rfc5277 - org.opendaylight.yangtools - yang-repo-fs + org.opendaylight.netconf.model + rfc6022 - org.opendaylight.yangtools - yang-repo-spi + org.opendaylight.netconf.model + rfc6241 - org.opendaylight.mdsal - mdsal-binding-api + org.opendaylight.netconf.model + rfc6470 - org.opendaylight.mdsal - mdsal-binding-runtime-spi + org.opendaylight.yangtools + concepts - org.opendaylight.mdsal - mdsal-common-api + org.opendaylight.yangtools + rfc8528-model-api - org.opendaylight.mdsal - mdsal-dom-api + org.opendaylight.yangtools + util - org.opendaylight.mdsal - mdsal-dom-spi + org.opendaylight.yangtools + yang-common - org.opendaylight.mdsal.binding.model.ietf - rfc6991-ietf-inet-types + org.opendaylight.yangtools + yang-data-api - org.opendaylight.mdsal.binding.model.ietf - rfc6991-ietf-yang-types + org.opendaylight.yangtools + yang-data-codec-gson - org.opendaylight.mdsal.binding.model.ietf - rfc8525 + org.opendaylight.yangtools + yang-data-codec-xml - org.opendaylight.aaa - aaa-encrypt-service + org.opendaylight.yangtools + yang-data-impl - org.opendaylight.netconf - netconf-api + org.opendaylight.yangtools + yang-data-spi - org.opendaylight.netconf - netconf-client + org.opendaylight.yangtools + yang-data-util - org.opendaylight.netconf - netconf-common + org.opendaylight.yangtools + yang-model-api - org.opendaylight.netconf - netconf-common-mdsal + org.opendaylight.yangtools + yang-model-spi - org.opendaylight.netconf - netconf-dom-api + org.opendaylight.yangtools + yang-model-util - org.opendaylight.netconf.model - rfc5277 + org.opendaylight.yangtools + yang-parser-api - org.opendaylight.netconf.model - rfc6022 + org.opendaylight.yangtools + yang-parser-impl - org.opendaylight.netconf.model - rfc6241 + org.opendaylight.yangtools + yang-parser-rfc7950 - org.opendaylight.netconf.model - rfc6470 + org.opendaylight.yangtools + yang-repo-api - com.guicedee.services - javax.inject - true + org.opendaylight.yangtools + yang-repo-fs - jakarta.annotation - jakarta.annotation-api - provided - true + org.opendaylight.yangtools + yang-repo-spi org.osgi diff --git a/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/impl/DefaultSslHandlerFactoryProvider.java b/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/impl/DefaultSslHandlerFactoryProvider.java index 165e5ab710..64752f6649 100644 --- a/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/impl/DefaultSslHandlerFactoryProvider.java +++ b/plugins/netconf-client-mdsal/src/main/java/org/opendaylight/netconf/client/mdsal/impl/DefaultSslHandlerFactoryProvider.java @@ -9,142 +9,55 @@ package org.opendaylight.netconf.client.mdsal.impl; import static java.util.Objects.requireNonNull; -import java.io.ByteArrayInputStream; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; -import java.security.KeyFactory; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.cert.Certificate; import java.security.cert.CertificateException; -import java.security.cert.CertificateFactory; -import java.security.cert.X509Certificate; -import java.security.spec.InvalidKeySpecException; -import java.security.spec.PKCS8EncodedKeySpec; -import java.util.Base64; -import java.util.HashMap; -import java.util.List; -import java.util.Map; import java.util.Set; import javax.annotation.PreDestroy; import javax.inject.Inject; import javax.inject.Singleton; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.binding.api.DataBroker; -import org.opendaylight.mdsal.binding.api.DataObjectModification; -import org.opendaylight.mdsal.binding.api.DataTreeChangeListener; -import org.opendaylight.mdsal.binding.api.DataTreeIdentifier; -import org.opendaylight.mdsal.binding.api.DataTreeModification; -import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.netconf.client.SslHandlerFactory; import org.opendaylight.netconf.client.mdsal.api.SslHandlerFactoryProvider; +import org.opendaylight.netconf.keystore.legacy.AbstractNetconfKeystore; +import org.opendaylight.netconf.keystore.legacy.SecurityHelper; import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev240120.connection.parameters.protocol.Specification; import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev240120.connection.parameters.protocol.specification.TlsCase; -import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.Keystore; -import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017._private.keys.PrivateKey; -import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.trusted.certificates.TrustedCertificate; -import org.opendaylight.yangtools.concepts.Registration; -import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Deactivate; import org.osgi.service.component.annotations.Reference; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Singleton @Component(service = SslHandlerFactoryProvider.class) -public final class DefaultSslHandlerFactoryProvider - implements SslHandlerFactoryProvider, DataTreeChangeListener, AutoCloseable { - /** - * Internal state, updated atomically. - */ - private record State( - @NonNull Map privateKeys, - @NonNull Map trustedCertificates) { - - State { - requireNonNull(privateKeys); - requireNonNull(trustedCertificates); - } - - @NonNull StateBuilder newBuilder() { - return new StateBuilder(new HashMap<>(privateKeys), new HashMap<>(trustedCertificates)); - } - } - - /** - * Intermediate builder for State. - */ - private record StateBuilder( - @NonNull HashMap privateKeys, - @NonNull HashMap trustedCertificates) { - - StateBuilder { - requireNonNull(privateKeys); - requireNonNull(trustedCertificates); - } - - @NonNull State build() { - return new State(Map.copyOf(privateKeys), Map.copyOf(trustedCertificates)); - } - } - - private static final class SecurityHelper { - private CertificateFactory certFactory; - private KeyFactory dsaFactory; - private KeyFactory rsaFactory; - - java.security.PrivateKey getJavaPrivateKey(final String base64PrivateKey) throws GeneralSecurityException { - final var keySpec = new PKCS8EncodedKeySpec(base64Decode(base64PrivateKey)); - - if (rsaFactory == null) { - rsaFactory = KeyFactory.getInstance("RSA"); - } - try { - return rsaFactory.generatePrivate(keySpec); - } catch (InvalidKeySpecException ignore) { - // Ignored - } - - if (dsaFactory == null) { - dsaFactory = KeyFactory.getInstance("DSA"); - } - return dsaFactory.generatePrivate(keySpec); - } - - private X509Certificate getCertificate(final String base64Certificate) throws GeneralSecurityException { - // TODO: https://stackoverflow.com/questions/43809909/is-certificatefactory-getinstancex-509-thread-safe - // indicates this is thread-safe in most cases, but can we get a better assurance? - if (certFactory == null) { - certFactory = CertificateFactory.getInstance("X.509"); - } - return (X509Certificate) certFactory.generateCertificate( - new ByteArrayInputStream(base64Decode(base64Certificate))); - } - } - - private static final Logger LOG = LoggerFactory.getLogger(DefaultSslHandlerFactoryProvider.class); +public final class DefaultSslHandlerFactoryProvider extends AbstractNetconfKeystore + implements SslHandlerFactoryProvider, AutoCloseable { private static final char[] EMPTY_CHARS = { }; private final @NonNull SslHandlerFactory nospecFactory = new SslHandlerFactoryImpl(this, Set.of()); - private final @NonNull Registration reg; - private volatile @NonNull State state = new State(Map.of(), Map.of()); + private volatile @NonNull State state = State.EMPTY; @Inject @Activate public DefaultSslHandlerFactoryProvider(@Reference final DataBroker dataBroker) { - reg = dataBroker.registerTreeChangeListener( - DataTreeIdentifier.of(LogicalDatastoreType.CONFIGURATION, InstanceIdentifier.create(Keystore.class)), this); + start(dataBroker); } @Deactivate @PreDestroy @Override public void close() { - reg.close(); + stop(); + } + + @Override + protected void onStateUpdated(final State newState) { + state = newState; } @Override @@ -187,7 +100,7 @@ public final class DefaultSslHandlerFactoryProvider KeyStore getJavaKeyStore(final Set allowedKeys) throws GeneralSecurityException, IOException { requireNonNull(allowedKeys); final var current = state; - if (current.privateKeys.isEmpty()) { + if (current.privateKeys().isEmpty()) { throw new KeyStoreException("No keystore private key found"); } @@ -197,7 +110,7 @@ public final class DefaultSslHandlerFactoryProvider final var helper = new SecurityHelper(); // Private keys first - for (var entry : current.privateKeys.entrySet()) { + for (var entry : current.privateKeys().entrySet()) { final var alias = entry.getKey(); if (!allowedKeys.isEmpty() && !allowedKeys.contains(alias)) { continue; @@ -219,72 +132,10 @@ public final class DefaultSslHandlerFactoryProvider keyStore.setKeyEntry(alias, key, EMPTY_CHARS, chain); } - for (var entry : current.trustedCertificates.entrySet()) { + for (var entry : current.trustedCertificates().entrySet()) { keyStore.setCertificateEntry(entry.getKey(), helper.getCertificate(entry.getValue().getCertificate())); } return keyStore; } - - private static byte[] base64Decode(final String base64) { - return Base64.getMimeDecoder().decode(base64.getBytes(StandardCharsets.US_ASCII)); - } - - @Override - public void onDataTreeChanged(final List> changes) { - LOG.debug("Starting update with {} changes", changes.size()); - final var builder = state.newBuilder(); - onDataTreeChanged(builder, changes); - state = builder.build(); - LOG.debug("Update finished"); - } - - private static void onDataTreeChanged(final StateBuilder builder, - final List> changes) { - for (var change : changes) { - LOG.debug("Processing change {}", change); - final var rootNode = change.getRootNode(); - - for (var changedChild : rootNode.modifiedChildren()) { - if (changedChild.dataType().equals(PrivateKey.class)) { - onPrivateKeyChanged(builder.privateKeys, (DataObjectModification)changedChild); - } else if (changedChild.dataType().equals(TrustedCertificate.class)) { - onTrustedCertificateChanged(builder.trustedCertificates, - (DataObjectModification)changedChild); - } - } - } - } - - private static void onPrivateKeyChanged(final HashMap privateKeys, - final DataObjectModification objectModification) { - switch (objectModification.modificationType()) { - case SUBTREE_MODIFIED: - case WRITE: - final var privateKey = objectModification.dataAfter(); - privateKeys.put(privateKey.getName(), privateKey); - break; - case DELETE: - privateKeys.remove(objectModification.dataBefore().getName()); - break; - default: - break; - } - } - - private static void onTrustedCertificateChanged(final HashMap trustedCertificates, - final DataObjectModification objectModification) { - switch (objectModification.modificationType()) { - case SUBTREE_MODIFIED: - case WRITE: - final var trustedCertificate = objectModification.dataAfter(); - trustedCertificates.put(trustedCertificate.getName(), trustedCertificate); - break; - case DELETE: - trustedCertificates.remove(objectModification.dataBefore().getName()); - break; - default: - break; - } - } } diff --git a/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/impl/DefaultSslHandlerFactoryProviderTest.java b/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/impl/DefaultSslHandlerFactoryProviderTest.java index aabe79a730..c59f62805a 100644 --- a/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/impl/DefaultSslHandlerFactoryProviderTest.java +++ b/plugins/netconf-client-mdsal/src/test/java/org/opendaylight/netconf/client/mdsal/impl/DefaultSslHandlerFactoryProviderTest.java @@ -12,20 +12,20 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; import java.security.KeyStoreException; import java.util.ArrayList; import java.util.List; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.binding.api.DataObjectModification; -import org.opendaylight.mdsal.binding.api.DataTreeIdentifier; +import org.opendaylight.mdsal.binding.api.DataTreeChangeListener; import org.opendaylight.mdsal.binding.api.DataTreeModification; import org.opendaylight.netconf.api.xml.XmlUtil; import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.Keystore; @@ -38,11 +38,9 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017. import org.opendaylight.yangtools.concepts.Registration; import org.w3c.dom.Document; import org.w3c.dom.Element; -import org.w3c.dom.Node; -import org.w3c.dom.NodeList; -@RunWith(MockitoJUnitRunner.StrictStubs.class) -public class DefaultSslHandlerFactoryProviderTest { +@ExtendWith(MockitoExtension.class) +class DefaultSslHandlerFactoryProviderTest { private static final String XML_ELEMENT_PRIVATE_KEY = "private-key"; private static final String XML_ELEMENT_NAME = "name"; private static final String XML_ELEMENT_DATA = "data"; @@ -54,144 +52,139 @@ public class DefaultSslHandlerFactoryProviderTest { private DataBroker dataBroker; @Mock private Registration listenerRegistration; + @Mock + private DataTreeModification dataTreeModification1; + @Mock + private DataTreeModification dataTreeModification2; + @Mock + private DataObjectModification keystoreObjectModification1; + @Mock + private DataObjectModification keystoreObjectModification2; + @Mock + private DataObjectModification privateKeyModification; + @Mock + private DataObjectModification trustedCertificateModification; + + private DataTreeChangeListener listener; - @Before - public void setUp() { - doReturn(listenerRegistration).when(dataBroker) - .registerTreeChangeListener(any(DataTreeIdentifier.class), any(DefaultSslHandlerFactoryProvider.class)); + @BeforeEach + void beforeEach() { + doAnswer(inv -> { + listener = inv.getArgument(1); + return listenerRegistration; + }).when(dataBroker).registerTreeChangeListener(any(), any()); } @Test - public void testKeystoreAdapterInit() throws Exception { - final DefaultSslHandlerFactoryProvider keystoreAdapter = new DefaultSslHandlerFactoryProvider(dataBroker); - final var ex = assertThrows(KeyStoreException.class, keystoreAdapter::getJavaKeyStore); - assertThat(ex.getMessage(), startsWith("No keystore private key found")); + void testKeystoreAdapterInit() throws Exception { + try (var keystoreAdapter = new DefaultSslHandlerFactoryProvider(dataBroker)) { + final var ex = assertThrows(KeyStoreException.class, keystoreAdapter::getJavaKeyStore); + assertThat(ex.getMessage(), startsWith("No keystore private key found")); + } } - @SuppressWarnings("unchecked") @Test - public void testWritePrivateKey() throws Exception { - DataTreeModification dataTreeModification = mock(DataTreeModification.class); - DataObjectModification keystoreObjectModification = mock(DataObjectModification.class); - doReturn(keystoreObjectModification).when(dataTreeModification).getRootNode(); - - DataObjectModification childObjectModification = mock(DataObjectModification.class); - doReturn(List.of(childObjectModification)).when(keystoreObjectModification).modifiedChildren(); - doReturn(PrivateKey.class).when(childObjectModification).dataType(); - - doReturn(DataObjectModification.ModificationType.WRITE).when(childObjectModification).modificationType(); + void testWritePrivateKey() throws Exception { + doReturn(keystoreObjectModification1).when(dataTreeModification1).getRootNode(); + doReturn(List.of(privateKeyModification)).when(keystoreObjectModification1) + .getModifiedChildren(PrivateKey.class); + doReturn(DataObjectModification.ModificationType.WRITE).when(privateKeyModification).modificationType(); final var privateKey = getPrivateKey(); - doReturn(privateKey).when(childObjectModification).dataAfter(); + doReturn(privateKey).when(privateKeyModification).dataAfter(); - final var keystoreAdapter = new DefaultSslHandlerFactoryProvider(dataBroker); - keystoreAdapter.onDataTreeChanged(List.of(dataTreeModification)); + try (var keystoreAdapter = new DefaultSslHandlerFactoryProvider(dataBroker)) { + listener.onDataTreeChanged(List.of(dataTreeModification1)); - final var keyStore = keystoreAdapter.getJavaKeyStore(); - assertTrue(keyStore.containsAlias(privateKey.getName())); + final var keyStore = keystoreAdapter.getJavaKeyStore(); + assertTrue(keyStore.containsAlias(privateKey.getName())); + } } - @SuppressWarnings("unchecked") @Test - public void testWritePrivateKeyAndTrustedCertificate() throws Exception { + void testWritePrivateKeyAndTrustedCertificate() throws Exception { // Prepare PrivateKey configuration - DataTreeModification dataTreeModification1 = mock(DataTreeModification.class); - DataObjectModification keystoreObjectModification1 = mock(DataObjectModification.class); doReturn(keystoreObjectModification1).when(dataTreeModification1).getRootNode(); - DataObjectModification childObjectModification1 = mock(DataObjectModification.class); - doReturn(List.of(childObjectModification1)).when(keystoreObjectModification1).modifiedChildren(); - doReturn(PrivateKey.class).when(childObjectModification1).dataType(); - - doReturn(DataObjectModification.ModificationType.WRITE).when(childObjectModification1).modificationType(); + doReturn(List.of(privateKeyModification)).when(keystoreObjectModification1) + .getModifiedChildren(PrivateKey.class); + doReturn(DataObjectModification.ModificationType.WRITE).when(privateKeyModification).modificationType(); final var privateKey = getPrivateKey(); - doReturn(privateKey).when(childObjectModification1).dataAfter(); + doReturn(privateKey).when(privateKeyModification).dataAfter(); // Prepare TrustedCertificate configuration - DataTreeModification dataTreeModification2 = mock(DataTreeModification.class); - DataObjectModification keystoreObjectModification2 = mock(DataObjectModification.class); doReturn(keystoreObjectModification2).when(dataTreeModification2).getRootNode(); - DataObjectModification childObjectModification2 = mock(DataObjectModification.class); - doReturn(List.of(childObjectModification2)).when(keystoreObjectModification2).modifiedChildren(); - doReturn(TrustedCertificate.class).when(childObjectModification2).dataType(); + doReturn(List.of()).when(keystoreObjectModification2).getModifiedChildren(PrivateKey.class); + doReturn(List.of(trustedCertificateModification)).when(keystoreObjectModification2) + .getModifiedChildren(TrustedCertificate.class); + doReturn(DataObjectModification.ModificationType.WRITE).when(trustedCertificateModification).modificationType(); - doReturn(DataObjectModification.ModificationType.WRITE) - .when(childObjectModification2).modificationType(); + final var trustedCertificate = getTrustedCertificate(); + doReturn(trustedCertificate).when(trustedCertificateModification).dataAfter(); - final var trustedCertificate = geTrustedCertificate(); - doReturn(trustedCertificate).when(childObjectModification2).dataAfter(); + try (var keystoreAdapter = new DefaultSslHandlerFactoryProvider(dataBroker)) { + // Apply configurations + listener.onDataTreeChanged(List.of(dataTreeModification1, dataTreeModification2)); - // Apply configurations - final var keystoreAdapter = new DefaultSslHandlerFactoryProvider(dataBroker); - keystoreAdapter.onDataTreeChanged(List.of(dataTreeModification1, dataTreeModification2)); - - // Check result - final var keyStore = keystoreAdapter.getJavaKeyStore(); - assertTrue(keyStore.containsAlias(privateKey.getName())); - assertTrue(keyStore.containsAlias(trustedCertificate.getName())); + // Check result + final var keyStore = keystoreAdapter.getJavaKeyStore(); + assertTrue(keyStore.containsAlias(privateKey.getName())); + assertTrue(keyStore.containsAlias(trustedCertificate.getName())); + } } - private PrivateKey getPrivateKey() throws Exception { - final List privateKeys = new ArrayList<>(); - final Document document = readKeystoreXML(); - final NodeList nodeList = document.getElementsByTagName(XML_ELEMENT_PRIVATE_KEY); + private static PrivateKey getPrivateKey() throws Exception { + final var privateKeys = new ArrayList(); + final var document = readKeystoreXML(); + final var nodeList = document.getElementsByTagName(XML_ELEMENT_PRIVATE_KEY); for (int i = 0; i < nodeList.getLength(); i++) { - final Node node = nodeList.item(i); - if (node.getNodeType() != Node.ELEMENT_NODE) { - continue; - } - final Element element = (Element)node; - final String keyName = element.getElementsByTagName(XML_ELEMENT_NAME).item(0).getTextContent(); - final String keyData = element.getElementsByTagName(XML_ELEMENT_DATA).item(0).getTextContent(); - final NodeList certNodes = element.getElementsByTagName(XML_ELEMENT_CERT_CHAIN); - final List certChain = new ArrayList<>(); - for (int j = 0; j < certNodes.getLength(); j++) { - final Node certNode = certNodes.item(j); - if (certNode.getNodeType() != Node.ELEMENT_NODE) { - continue; + if (nodeList.item(i) instanceof Element element) { + final var keyName = element.getElementsByTagName(XML_ELEMENT_NAME).item(0).getTextContent(); + final var keyData = element.getElementsByTagName(XML_ELEMENT_DATA).item(0).getTextContent(); + final var certNodes = element.getElementsByTagName(XML_ELEMENT_CERT_CHAIN); + final var certChain = new ArrayList(); + for (int j = 0; j < certNodes.getLength(); j++) { + if (certNodes.item(j) instanceof Element certNode) { + certChain.add(certNode.getTextContent()); + } } - certChain.add(certNode.getTextContent()); - } - final PrivateKey privateKey = new PrivateKeyBuilder() + privateKeys.add(new PrivateKeyBuilder() .withKey(new PrivateKeyKey(keyName)) .setName(keyName) .setData(keyData) .setCertificateChain(certChain) - .build(); - privateKeys.add(privateKey); + .build()); + } } return privateKeys.get(0); } - private TrustedCertificate geTrustedCertificate() throws Exception { - final List trustedCertificates = new ArrayList<>(); - final Document document = readKeystoreXML(); - final NodeList nodeList = document.getElementsByTagName(XML_ELEMENT_TRUSTED_CERT); + private static TrustedCertificate getTrustedCertificate() throws Exception { + final var trustedCertificates = new ArrayList(); + final var document = readKeystoreXML(); + final var nodeList = document.getElementsByTagName(XML_ELEMENT_TRUSTED_CERT); for (int i = 0; i < nodeList.getLength(); i++) { - final Node node = nodeList.item(i); - if (node.getNodeType() != Node.ELEMENT_NODE) { - continue; - } - final Element element = (Element)node; - final String certName = element.getElementsByTagName(XML_ELEMENT_NAME).item(0).getTextContent(); - final String certData = element.getElementsByTagName(XML_ELEMENT_CERT).item(0).getTextContent(); + if (nodeList.item(i) instanceof Element element) { + final var certName = element.getElementsByTagName(XML_ELEMENT_NAME).item(0).getTextContent(); + final var certData = element.getElementsByTagName(XML_ELEMENT_CERT).item(0).getTextContent(); - final TrustedCertificate certificate = new TrustedCertificateBuilder() + trustedCertificates.add(new TrustedCertificateBuilder() .withKey(new TrustedCertificateKey(certName)) .setName(certName) .setCertificate(certData) - .build(); - trustedCertificates.add(certificate); + .build()); + } } return trustedCertificates.get(0); } - private Document readKeystoreXML() throws Exception { - return XmlUtil.readXmlToDocument(getClass().getResourceAsStream("/netconf-keystore.xml")); + private static Document readKeystoreXML() throws Exception { + return XmlUtil.readXmlToDocument( + DefaultSslHandlerFactoryProviderTest.class.getResourceAsStream("/netconf-keystore.xml")); } } -- 2.36.6