From: Robert Varga Date: Thu, 31 Mar 2022 07:41:19 +0000 (+0200) Subject: Cleanup switch certificate chain handling X-Git-Tag: release/sulfur~14 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=ed4d25aecaabfe181dbb114d4666f3a05f3531fb;p=openflowplugin.git Cleanup switch certificate chain handling SpotBugs is identifying a leak into static state. This is caused by a rather thorough layering violation. Refactor ConnectionAdapter to always report switch certificate -- cleaning up the code in process, making it properly defensive. JIRA: OPNFLWPLUG-1094 Change-Id: If48b57175347dab056424b8f1b169bd552ea029f Signed-off-by: Robert Varga --- diff --git a/openflowjava/openflow-protocol-api/src/main/java/org/opendaylight/openflowjava/protocol/api/connection/ConnectionAdapter.java b/openflowjava/openflow-protocol-api/src/main/java/org/opendaylight/openflowjava/protocol/api/connection/ConnectionAdapter.java index 4de37851db..c60311af64 100644 --- a/openflowjava/openflow-protocol-api/src/main/java/org/opendaylight/openflowjava/protocol/api/connection/ConnectionAdapter.java +++ b/openflowjava/openflow-protocol-api/src/main/java/org/opendaylight/openflowjava/protocol/api/connection/ConnectionAdapter.java @@ -11,8 +11,10 @@ import com.google.common.annotations.Beta; import java.math.BigInteger; import java.net.InetSocketAddress; import java.security.cert.X509Certificate; +import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.openflowjava.protocol.api.extensibility.AlienMessageListener; import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.OpenflowProtocolListener; import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.OpenflowProtocolService; @@ -80,9 +82,10 @@ public interface ConnectionAdapter extends OpenflowProtocolService { /** * Notify listener about switch certificate information. - * @param switchcertificate X509Certificate of switch + * + * @param certificateChain X509 certificate chain presented by the switch */ - void onSwitchCertificateIdentified(X509Certificate switchcertificate); + void onSwitchCertificateIdentified(@Nullable List certificateChain); /** * Set listener for connection became ready-to-use event. diff --git a/openflowjava/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/SslContextFactory.java b/openflowjava/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/SslContextFactory.java index 4ed7618cfa..35ba6f9a58 100644 --- a/openflowjava/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/SslContextFactory.java +++ b/openflowjava/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/SslContextFactory.java @@ -8,6 +8,8 @@ package org.opendaylight.openflowjava.protocol.impl.core; +import static java.util.Objects.requireNonNull; + import java.io.IOException; import java.net.Socket; import java.security.KeyManagementException; @@ -18,11 +20,15 @@ import java.security.Security; import java.security.UnrecoverableKeyException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.util.List; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; +import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509ExtendedTrustManager; +import javax.net.ssl.X509TrustManager; +import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.openflowjava.protocol.api.connection.TlsConfiguration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,16 +39,15 @@ import org.slf4j.LoggerFactory; * @author michal.polkorab */ public class SslContextFactory { + private static final Logger LOG = LoggerFactory.getLogger(SslContextFactory.class); // "TLS" - supports some version of TLS // Use "TLSv1", "TLSv1.1", "TLSv1.2" for specific TLS version private static final String PROTOCOL = "TLS"; + private final TlsConfiguration tlsConfig; - private static X509Certificate switchCertificate = null; - private static boolean isCustomTrustManagerEnabled; - private static final Logger LOG = LoggerFactory - .getLogger(SslContextFactory.class); + private volatile List switchCertificateChain; /** * Sets the TlsConfiguration. @@ -51,29 +56,20 @@ public class SslContextFactory { * TLS configuration object, contains keystore locations + * keystore types */ - public SslContextFactory(TlsConfiguration tlsConfig) { - this.tlsConfig = tlsConfig; - } - - public X509Certificate getSwitchCertificate() { - return switchCertificate; + public SslContextFactory(final TlsConfiguration tlsConfig) { + this.tlsConfig = requireNonNull(tlsConfig); } - public boolean isCustomTrustManagerEnabled() { - return isCustomTrustManagerEnabled; + @Nullable List getSwitchCertificateChain() { + return switchCertificateChain; } - public static void setSwitchCertificate(X509Certificate certificate) { - switchCertificate = certificate; - } - - public static void setIsCustomTrustManagerEnabled(boolean customTrustManagerEnabled) { - isCustomTrustManagerEnabled = customTrustManagerEnabled; + void setSwitchCertificateChain(final X509Certificate[] chain) { + switchCertificateChain = List.of(chain); } public SSLContext getServerContext() { - String algorithm = Security - .getProperty("ssl.KeyManagerFactory.algorithm"); + String algorithm = Security.getProperty("ssl.KeyManagerFactory.algorithm"); if (algorithm == null) { algorithm = "SunX509"; } @@ -92,16 +88,27 @@ public class SslContextFactory { tmf.init(ts); serverContext = SSLContext.getInstance(PROTOCOL); - if (isCustomTrustManagerEnabled) { - CustomTrustManager[] customTrustManager = new CustomTrustManager[tmf.getTrustManagers().length]; - for (int i = 0; i < tmf.getTrustManagers().length; i++) { - customTrustManager[i] = new CustomTrustManager((X509ExtendedTrustManager) - tmf.getTrustManagers()[i]); + + // A bit ugly: intercept trust checks to establish switch certificate + final TrustManager[] delegates = tmf.getTrustManagers(); + final TrustManager[] proxies; + if (delegates != null) { + proxies = new TrustManager[delegates.length]; + for (int i = 0; i < delegates.length; i++) { + final TrustManager delegate = delegates[i]; + if (delegate instanceof X509ExtendedTrustManager) { + proxies[i] = new ProxyExtendedTrustManager((X509ExtendedTrustManager) delegate); + } else if (delegate instanceof X509TrustManager) { + proxies[i] = new ProxyTrustManager((X509TrustManager) delegate); + } else { + LOG.debug("Cannot handle trust manager {}, passing through", delegate); + proxies[i] = delegate; + } } - serverContext.init(kmf.getKeyManagers(), customTrustManager, null); } else { - serverContext.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null); + proxies = null; } + serverContext.init(kmf.getKeyManagers(), proxies, null); } catch (IOException e) { LOG.warn("IOException - Failed to load keystore / truststore." + " Failed to initialize the server-side SSLContext", e); @@ -117,58 +124,81 @@ public class SslContextFactory { return serverContext; } + private final class ProxyTrustManager implements X509TrustManager { + private final X509TrustManager delegate; + + ProxyTrustManager(final X509TrustManager delegate) { + this.delegate = requireNonNull(delegate); + } + + @Override + public void checkClientTrusted(final X509Certificate[] chain, final String authType) + throws CertificateException { + setSwitchCertificateChain(chain); + delegate.checkClientTrusted(chain, authType); + } + + @Override + public void checkServerTrusted(final X509Certificate[] chain, final String authType) + throws CertificateException { + delegate.checkServerTrusted(chain, authType); + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return delegate.getAcceptedIssuers(); + } + } - private static class CustomTrustManager extends X509ExtendedTrustManager { - private final X509ExtendedTrustManager trustManager; + private final class ProxyExtendedTrustManager extends X509ExtendedTrustManager { + private final X509ExtendedTrustManager delegate; - CustomTrustManager(final X509ExtendedTrustManager trustManager) { - this.trustManager = trustManager; + ProxyExtendedTrustManager(final X509ExtendedTrustManager trustManager) { + delegate = requireNonNull(trustManager); } @Override - public void checkClientTrusted(final X509Certificate[] x509Certificates, final String authType, - final Socket socket) throws CertificateException { - SslContextFactory.setSwitchCertificate(x509Certificates[0]); - trustManager.checkClientTrusted(x509Certificates, authType, socket); + public void checkClientTrusted(final X509Certificate[] chain, final String authType, final Socket socket) + throws CertificateException { + setSwitchCertificateChain(chain); + delegate.checkClientTrusted(chain, authType, socket); } @Override - public void checkClientTrusted(final X509Certificate[] x509Certificates, final String authType) + public void checkClientTrusted(final X509Certificate[] chain, final String authType) throws CertificateException { - SslContextFactory.setSwitchCertificate(x509Certificates[0]); - trustManager.checkClientTrusted(x509Certificates, authType); + setSwitchCertificateChain(chain); + delegate.checkClientTrusted(chain, authType); } @Override - public void checkClientTrusted(final X509Certificate[] x509Certificates, final String authType, - final SSLEngine sslEngine) throws CertificateException { - SslContextFactory.setSwitchCertificate(x509Certificates[0]); - trustManager.checkClientTrusted(x509Certificates, authType, sslEngine); + public void checkClientTrusted(final X509Certificate[] chain, final String authType, final SSLEngine sslEngine) + throws CertificateException { + setSwitchCertificateChain(chain); + delegate.checkClientTrusted(chain, authType, sslEngine); } @Override - public void checkServerTrusted(final X509Certificate[] x509Certificates, final String authType, - final SSLEngine sslEngine) throws CertificateException { - trustManager.checkServerTrusted(x509Certificates, authType, sslEngine); + public void checkServerTrusted(final X509Certificate[] chain, final String authType, final SSLEngine sslEngine) + throws CertificateException { + delegate.checkServerTrusted(chain, authType, sslEngine); } @Override - public void checkServerTrusted(final X509Certificate[] x509Certificates, final String authType) + public void checkServerTrusted(final X509Certificate[] chain, final String authType) throws CertificateException { - trustManager.checkServerTrusted(x509Certificates, authType); + delegate.checkServerTrusted(chain, authType); } @Override - public void checkServerTrusted(final X509Certificate[] x509Certificates, final String authType, - final Socket socket) throws CertificateException { - trustManager.checkServerTrusted(x509Certificates, authType, socket); + public void checkServerTrusted(final X509Certificate[] chain, final String authType, final Socket socket) + throws CertificateException { + delegate.checkServerTrusted(chain, authType, socket); } @Override public X509Certificate[] getAcceptedIssuers() { - return trustManager.getAcceptedIssuers(); + return delegate.getAcceptedIssuers(); } - } - } diff --git a/openflowjava/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/TcpChannelInitializer.java b/openflowjava/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/TcpChannelInitializer.java index 4eb27184c8..c06803c47b 100644 --- a/openflowjava/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/TcpChannelInitializer.java +++ b/openflowjava/openflow-protocol-impl/src/main/java/org/opendaylight/openflowjava/protocol/impl/core/TcpChannelInitializer.java @@ -19,6 +19,7 @@ import java.util.Iterator; import java.util.List; import java.util.concurrent.TimeUnit; import javax.net.ssl.SSLEngine; +import org.opendaylight.openflowjava.protocol.api.connection.TlsConfiguration; import org.opendaylight.openflowjava.protocol.impl.core.connection.ConnectionAdapterFactory; import org.opendaylight.openflowjava.protocol.impl.core.connection.ConnectionAdapterFactoryImpl; import org.opendaylight.openflowjava.protocol.impl.core.connection.ConnectionFacade; @@ -76,12 +77,11 @@ public class TcpChannelInitializer extends ProtocolChannelInitializer handshakeFuture = ssl.handshakeFuture(); final ConnectionFacade finalConnectionFacade = connectionFacade; - if (sslFactory.isCustomTrustManagerEnabled()) { - handshakeFuture.addListener(future -> finalConnectionFacade - .onSwitchCertificateIdentified(sslFactory.getSwitchCertificate())); - } + handshakeFuture.addListener(future -> finalConnectionFacade.onSwitchCertificateIdentified( + sslFactory.getSwitchCertificateChain())); handshakeFuture.addListener(future -> finalConnectionFacade.fireConnectionReadyNotification()); ch.pipeline().addLast(PipelineHandlers.SSL_HANDLER.name(), ssl); } ch.pipeline().addLast(PipelineHandlers.OF_FRAME_DECODER.name(), - new OFFrameDecoder(connectionFacade, tlsPresent)); + new OFFrameDecoder(connectionFacade, tlsConfig != null)); ch.pipeline().addLast(PipelineHandlers.OF_VERSION_DETECTOR.name(), new OFVersionDetector()); final OFDecoder ofDecoder = new OFDecoder(); ofDecoder.setDeserializationFactory(getDeserializationFactory()); @@ -117,7 +115,7 @@ public class TcpChannelInitializer extends ProtocolChannelInitializer certificateChain) { + if (certificateChain != null && !certificateChain.isEmpty()) { + switchCertificate = certificateChain.get(0); + } } @Override @@ -263,60 +265,59 @@ public class ConnectionAdapterImpl extends AbstractConnectionAdapterStatistics i } private SwitchCertificate buildSwitchCertificate() { - if (switchCertificate != null) { - SwitchCertificateBuilder switchCertificateBuilder = new SwitchCertificateBuilder(); - SimpleDateFormat formatter = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'-00:00'"); - formatter.setTimeZone(TimeZone.getTimeZone("UTC")); - try { - Map subjectMap = new ConcurrentHashMap<>(); - Map issuerMap = new ConcurrentHashMap<>(); - subjectMap.putAll(new LdapName(switchCertificate.getSubjectX500Principal().getName()) - .getRdns().stream() - .collect(Collectors.toMap(rdn -> rdn.getType(), rdn -> rdn.getValue().toString()))); - issuerMap.putAll(new LdapName(switchCertificate.getIssuerX500Principal().getName()) - .getRdns().stream() - .collect(Collectors.toMap(rdn -> rdn.getType(), rdn -> rdn.getValue().toString()))); - SubjectBuilder subjectBuilder = new SubjectBuilder(); - subjectBuilder.setCommonName(subjectMap.get("CN")); - subjectBuilder.setCountry(subjectMap.get("C")); - subjectBuilder.setLocality(subjectMap.get("L")); - subjectBuilder.setOrganization(subjectMap.get("O")); - subjectBuilder.setOrganizationUnit(subjectMap.get("OU")); - subjectBuilder.setState(subjectMap.get("ST")); - IssuerBuilder issuerBuilder = new IssuerBuilder(); - issuerBuilder.setCommonName(issuerMap.get("CN")); - issuerBuilder.setCountry(issuerMap.get("C")); - issuerBuilder.setLocality(issuerMap.get("L")); - issuerBuilder.setOrganization(issuerMap.get("O")); - issuerBuilder.setOrganizationUnit(issuerMap.get("OU")); - issuerBuilder.setState(issuerMap.get("ST")); - switchCertificateBuilder.setSubject(subjectBuilder.build()); - switchCertificateBuilder.setIssuer(issuerBuilder.build()); - } catch (InvalidNameException e) { - LOG.error("Exception ", e); - } - switchCertificateBuilder.setSerialNumber(switchCertificate.getSerialNumber().toString()); - try { - if (switchCertificate.getSubjectAlternativeNames() != null) { - List subjectAlternateNames = new ArrayList<>(); - switchCertificate.getSubjectAlternativeNames().forEach(generalName -> { - final Object value = generalName.get(1); - if (value instanceof String) { - subjectAlternateNames.add((String) value); - } - }); - switchCertificateBuilder.setSubjectAlternateNames(subjectAlternateNames); - } else { - switchCertificateBuilder.setSubjectAlternateNames(null); - } - } catch (CertificateParsingException e) { - LOG.error("Error encountered while parsing certificate ", e); - } - switchCertificateBuilder.setValidFrom(new DateAndTime(formatter.format(switchCertificate.getNotBefore()))); - switchCertificateBuilder.setValidTo(new DateAndTime(formatter.format(switchCertificate.getNotAfter()))); - return switchCertificateBuilder.build(); + if (switchCertificate == null) { + return null; + } + + final var builder = new SwitchCertificateBuilder(); + final var subjectMap = indexRds(switchCertificate.getSubjectX500Principal()); + if (subjectMap != null) { + builder.setSubject(new SubjectBuilder() + .setCommonName(subjectMap.get("CN")) + .setCountry(subjectMap.get("C")) + .setLocality(subjectMap.get("L")) + .setOrganization(subjectMap.get("O")) + .setOrganizationUnit(subjectMap.get("OU")) + .setState(subjectMap.get("ST")) + .build()); } - return null; + + final var issuerMap = indexRds(switchCertificate.getIssuerX500Principal()); + if (issuerMap != null) { + builder.setIssuer(new IssuerBuilder() + .setCommonName(issuerMap.get("CN")) + .setCountry(issuerMap.get("C")) + .setLocality(issuerMap.get("L")) + .setOrganization(issuerMap.get("O")) + .setOrganizationUnit(issuerMap.get("OU")) + .setState(issuerMap.get("ST")) + .build()); + } + + Collection> altNames = null; + try { + altNames = switchCertificate.getSubjectAlternativeNames(); + } catch (CertificateParsingException e) { + LOG.error("Cannot parse certificate alternate names", e); + } + if (altNames != null) { + builder.setSubjectAlternateNames(altNames.stream() + .filter(list -> list.size() > 1) + .map(list -> list.get(1)) + .filter(String.class::isInstance) + .map(String.class::cast) + .collect(Collectors.toUnmodifiableList())); + } + + // FIXME: do not use SimpleDateFormat + SimpleDateFormat formatter = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'-00:00'"); + formatter.setTimeZone(TimeZone.getTimeZone("UTC")); + + return builder + .setSerialNumber(switchCertificate.getSerialNumber().toString()) + .setValidFrom(new DateAndTime(formatter.format(switchCertificate.getNotBefore()))) + .setValidTo(new DateAndTime(formatter.format(switchCertificate.getNotAfter()))) + .build(); } @Override @@ -328,4 +329,15 @@ public class ConnectionAdapterImpl extends AbstractConnectionAdapterStatistics i public void setExecutorService(final ExecutorService executorService) { this.executorService = executorService; } + + private static Map indexRds(final X500Principal principal) { + final LdapName name; + try { + name = new LdapName(principal.getName()); + } catch (InvalidNameException e) { + LOG.error("Cannot parse principal {}", principal, e); + return null; + } + return name.getRdns().stream().collect(Collectors.toMap(Rdn::getType, rdn -> rdn.getValue().toString())); + } } diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/configuration/ConfigurationProperty.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/configuration/ConfigurationProperty.java index 2b39f24ab7..4a5a58a563 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/configuration/ConfigurationProperty.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/configuration/ConfigurationProperty.java @@ -123,12 +123,7 @@ public enum ConfigurationProperty { /** * Delay for Device removal from Operational DataStore. */ - DEVICE_DATASTORE_REMOVAL_DELAY, - /** - * Enable or disable customtrustmanager which adds switch certificate attributes to TLS - * failure notification property type. - */ - ENABLE_CUSTOM_TRUST_MANAGER; + DEVICE_DATASTORE_REMOVAL_DELAY; private static final Map KEY_VALUE_MAP; diff --git a/openflowplugin-api/src/main/yang/openflow-provider-config.yang b/openflowplugin-api/src/main/yang/openflow-provider-config.yang index 775dece99c..66645a52f7 100644 --- a/openflowplugin-api/src/main/yang/openflow-provider-config.yang +++ b/openflowplugin-api/src/main/yang/openflow-provider-config.yang @@ -221,12 +221,5 @@ module openflow-provider-config { type non-zero-uint32-type; default 500; } - - leaf enable-custom-trust-manager { - description "When true would use customtrustmanager to get switch certificate for TLS - authentication failure notification. "; - type boolean; - default "false"; - } } } diff --git a/openflowplugin-blueprint-config/src/main/resources/initial/openflowplugin.cfg b/openflowplugin-blueprint-config/src/main/resources/initial/openflowplugin.cfg index ef35148f3f..9306919e72 100644 --- a/openflowplugin-blueprint-config/src/main/resources/initial/openflowplugin.cfg +++ b/openflowplugin-blueprint-config/src/main/resources/initial/openflowplugin.cfg @@ -145,12 +145,6 @@ # # device-datastore-removal-delay=500 -# -# When true, uses customtrustmanager to add switch certificate attributes to TLS authentification failure -# notification -# -# enable-custom-trust-manager=false - ############################################################################# # # # Forwarding Rule Manager Application Configuration # diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/configuration/OpenFlowProviderConfigImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/configuration/OpenFlowProviderConfigImpl.java index 10159837ce..2c676c60cd 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/configuration/OpenFlowProviderConfigImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/configuration/OpenFlowProviderConfigImpl.java @@ -165,10 +165,4 @@ public class OpenFlowProviderConfigImpl extends AbstractAugmentablegetProperty( ConfigurationProperty.DEVICE_DATASTORE_REMOVAL_DELAY.toString(), Uint32::valueOf)); } - - @Override - public Boolean getEnableCustomTrustManager() { - return service.getProperty(ConfigurationProperty.ENABLE_CUSTOM_TRUST_MANAGER.toString(), - Boolean::valueOf); - } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/ConnectionManagerImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/ConnectionManagerImpl.java index 0f54f01a94..be08c91e78 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/ConnectionManagerImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/ConnectionManagerImpl.java @@ -29,7 +29,6 @@ import org.opendaylight.mdsal.binding.api.NotificationPublishService; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.openflowjava.protocol.api.connection.ConnectionAdapter; import org.opendaylight.openflowjava.protocol.api.connection.ConnectionReadyListener; -import org.opendaylight.openflowjava.protocol.impl.core.SslContextFactory; import org.opendaylight.openflowplugin.api.OFConstants; import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext; import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionManager; @@ -81,9 +80,9 @@ public class ConnectionManagerImpl implements ConnectionManager { @NonNull final NotificationPublishService notificationPublishService) { this.config = config; this.executorService = executorService; - this.deviceConnectionRateLimiter = new DeviceConnectionRateLimiter(config); + deviceConnectionRateLimiter = new DeviceConnectionRateLimiter(config); this.dataBroker = dataBroker; - this.deviceConnectionHoldTime = config.getDeviceConnectionHoldTimeInSeconds().toJava(); + deviceConnectionHoldTime = config.getDeviceConnectionHoldTimeInSeconds().toJava(); deviceConnectionStatusProvider = new DeviceConnectionStatusProviderImpl(); deviceConnectionStatusProvider.init(); this.notificationPublishService = notificationPublishService; @@ -96,7 +95,7 @@ public class ConnectionManagerImpl implements ConnectionManager { LOG.trace("prepare connection context"); final ConnectionContext connectionContext = new ConnectionContextImpl(connectionAdapter, deviceConnectionStatusProvider); - connectionContext.setDeviceDisconnectedHandler(this.deviceDisconnectedHandler); + connectionContext.setDeviceDisconnectedHandler(deviceDisconnectedHandler); HandshakeListener handshakeListener = new HandshakeListenerImpl(connectionContext, deviceConnectedHandler); final HandshakeManager handshakeManager = createHandshakeManager(connectionAdapter, handshakeListener); @@ -118,7 +117,6 @@ public class ConnectionManagerImpl implements ConnectionManager { final SystemNotificationsListener systemListener = new SystemNotificationsListenerImpl(connectionContext, config.getEchoReplyTimeout().getValue().toJava(), executorService, notificationPublishService); connectionAdapter.setSystemListener(systemListener); - SslContextFactory.setIsCustomTrustManagerEnabled(config.getEnableCustomTrustManager()); LOG.trace("connection ballet finished"); } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/SystemNotificationsListenerImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/SystemNotificationsListenerImpl.java index 5bbfc273b4..2c1798d983 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/SystemNotificationsListenerImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/SystemNotificationsListenerImpl.java @@ -142,23 +142,24 @@ public class SystemNotificationsListenerImpl implements SystemNotificationsListe @Override public void onSslConnectionError(final SslConnectionError notification) { - IpAddress ip = null; + final IpAddress ip; if (connectionContext.getConnectionAdapter() != null && connectionContext.getConnectionAdapter().getRemoteAddress() != null && connectionContext.getConnectionAdapter().getRemoteAddress().getAddress() != null) { ip = IpAddressBuilder.getDefaultInstance( connectionContext.getConnectionAdapter().getRemoteAddress().getAddress().getHostAddress()); + } else { + ip = null; } - SwitchCertificateBuilder switchCertificateBuilder = new SwitchCertificateBuilder(); - if (notification.getSwitchCertificate() != null) { - switchCertificateBuilder = new SwitchCertificateBuilder(notification.getSwitchCertificate()); - } + + final var switchCert = notification.getSwitchCertificate(); + notificationPublishService.offerNotification(new SslErrorBuilder() .setType(SslErrorType.SslConFailed) .setCode(Uint16.valueOf(SslErrorType.SslConFailed.getIntValue())) .setNodeIpAddress(ip) .setData(notification.getInfo()) - .setSwitchCertificate(notification.getSwitchCertificate() != null ? switchCertificateBuilder.build() : null) + .setSwitchCertificate(switchCert == null ? null : new SwitchCertificateBuilder(switchCert).build()) .build()); } } diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/connection/ConnectionManagerImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/connection/ConnectionManagerImplTest.java index 4299e033a9..5f972eced4 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/connection/ConnectionManagerImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/connection/ConnectionManagerImplTest.java @@ -76,7 +76,6 @@ public class ConnectionManagerImplTest { private static final Uint32 ECHO_REPLY_TIMEOUT = Uint32.valueOf(500); private static final Uint16 DEVICE_CONNECTION_RATE_LIMIT_PER_MIN = Uint16.ZERO; private static final Uint16 DEVICE_CONNECTION_HOLD_TIME_IN_SECONDS = Uint16.valueOf(60); - private static final boolean ENABLE_CUSTOM_TRUST_MANAGER = false; @Before public void setUp() { @@ -88,7 +87,6 @@ public class ConnectionManagerImplTest { .setEchoReplyTimeout(new NonZeroUint32Type(ECHO_REPLY_TIMEOUT)) .setDeviceConnectionRateLimitPerMin(DEVICE_CONNECTION_RATE_LIMIT_PER_MIN) .setDeviceConnectionHoldTimeInSeconds(DEVICE_CONNECTION_HOLD_TIME_IN_SECONDS) - .setEnableCustomTrustManager(ENABLE_CUSTOM_TRUST_MANAGER) .build(), threadPool, dataBroker, notificationPublishService); connectionManagerImpl.setDeviceConnectedHandler(deviceConnectedHandler);