Catch correct exceptions when decrypting password 15/110515/10
authorSamuel Schneider <samuel.schneider@pantheon.tech>
Thu, 7 Mar 2024 09:11:13 +0000 (10:11 +0100)
committerIvan Hrasko <ivan.hrasko@pantheon.tech>
Fri, 17 May 2024 12:45:17 +0000 (12:45 +0000)
Catch correct exceptions when we fail to decrypt password.
It has been changed from IAE to ISE by: 15f04f0 which adopts
improvement made in AAA-266.

This adaptation makes our workaround to work again.

JIRA: NETCONF-1114
Change-Id: I86741e51855cfee687e56b5991ae39a8b08cd9f7
Signed-off-by: Samuel Schneider <samuel.schneider@pantheon.tech>
Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
apps/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java
apps/netconf-topology/src/test/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopologyTest.java

index f250c54a7b821cd137e9775d5a7158b9d61e094f..e8da8cef8bce32f32a53b8140bcc064ef0bb4065 100644 (file)
@@ -123,9 +123,8 @@ public abstract class AbstractNetconfTopology {
             nodeHandler = new NetconfNodeHandler(clientFactory, timer, baseSchemaProvider, schemaManager,
                 schemaAssembler, builderFactory, deviceActionFactory, deviceSalFacade, deviceId, nodeId, netconfNode,
                 nodeOptional);
-        } catch (IllegalArgumentException e) {
-            // This is a workaround for NETCONF-1114 where the encrypted password's lexical structure is not enforced
-            // in the datastore and it ends up surfacing when we decrypt the password.
+        } catch (IllegalStateException e) {
+            // This is a workaround for NETCONF-1114 when we fail to decrypt the password
             LOG.warn("RemoteDevice{{}} failed to connect, removing from operational datastore", nodeId, e);
             deviceSalFacade.close();
             return;
index 55b5831db1f95445937a9f0ec53683e7756ef8f5..302a1e3fda205dd9101bf7ea195e846c325200fb 100644 (file)
@@ -10,24 +10,78 @@ package org.opendaylight.netconf.topology.spi;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.verify;
 
+import java.nio.charset.StandardCharsets;
+import java.security.GeneralSecurityException;
+import java.util.concurrent.TimeUnit;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.opendaylight.aaa.encrypt.AAAEncryptionService;
+import org.opendaylight.mdsal.binding.api.DataBroker;
+import org.opendaylight.mdsal.binding.api.WriteTransaction;
+import org.opendaylight.mdsal.common.api.CommitInfo;
+import org.opendaylight.mdsal.dom.api.DOMMountPointService;
+import org.opendaylight.netconf.client.NetconfClientFactory;
+import org.opendaylight.netconf.client.mdsal.api.BaseNetconfSchemaProvider;
+import org.opendaylight.netconf.client.mdsal.api.CredentialProvider;
+import org.opendaylight.netconf.client.mdsal.api.DeviceActionFactory;
+import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceHandler;
+import org.opendaylight.netconf.client.mdsal.api.RemoteDeviceId;
+import org.opendaylight.netconf.client.mdsal.api.SchemaResourceManager;
+import org.opendaylight.netconf.client.mdsal.api.SslContextFactoryProvider;
+import org.opendaylight.netconf.client.mdsal.impl.DefaultBaseNetconfSchemaProvider;
+import org.opendaylight.netconf.common.NetconfTimer;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Host;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Address;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.PortNumber;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev240120.connection.parameters.Protocol.Name;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev240120.connection.parameters.ProtocolBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev240120.credentials.credentials.LoginPwBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev240120.credentials.credentials.LoginPwUnencryptedBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev240120.credentials.credentials.login.pw.LoginPasswordBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.device.rev240120.credentials.credentials.login.pw.unencrypted.LoginPasswordUnencryptedBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.node.topology.rev231121.NetconfNodeBuilder;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.NodeId;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node;
 import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.NodeBuilder;
+import org.opendaylight.yangtools.yang.common.Decimal64;
 import org.opendaylight.yangtools.yang.common.Uint16;
 import org.opendaylight.yangtools.yang.common.Uint32;
+import org.opendaylight.yangtools.yang.parser.impl.DefaultYangParserFactory;
 
+@ExtendWith(MockitoExtension.class)
 class AbstractNetconfTopologyTest {
+    @Mock
+    private NetconfTimer timer;
+    @Mock
+    private SchemaResourceManager schemaManager;
+    @Mock
+    private DeviceActionFactory deviceActionFactory;
+    @Mock
+    private SslContextFactoryProvider sslContextFactoryProvider;
+    @Mock
+    private AAAEncryptionService encryptionService;
+    @Mock
+    private CredentialProvider credentialProvider;
+    @Mock
+    private NetconfClientFactory clientFactory;
+    @Mock
+    private DataBroker dataBroker;
+    @Mock
+    private DOMMountPointService mountPointService;
+    @Mock
+    private WriteTransaction wtx;
+    @Mock
+    private RemoteDeviceHandler delegate;
+
     @Test
     void hideCredentials() {
         final String userName = "admin";
@@ -72,4 +126,78 @@ class AbstractNetconfTopologyTest {
             .build();
         assertNotNull(AbstractNetconfTopology.hideCredentials(node));
     }
+
+    @Test
+    void testFailToDecryptPassword() throws Exception {
+        doReturn(wtx).when(dataBroker).newWriteOnlyTransaction();
+        doReturn(CommitInfo.emptyFluentFuture()).when(wtx).commit();
+        doNothing().when(wtx).merge(any(), any(), any());
+
+        final var schemaAssembler = new NetconfTopologySchemaAssembler(1, 1, 0, TimeUnit.SECONDS);
+        final var topology = new TestingNetconfTopologyImpl("id", clientFactory, timer, schemaAssembler,
+            schemaManager, dataBroker, mountPointService,
+            new NetconfClientConfigurationBuilderFactoryImpl(encryptionService, credentialProvider,
+                sslContextFactoryProvider), deviceActionFactory,
+            new DefaultBaseNetconfSchemaProvider(new DefaultYangParserFactory()));
+
+        final var netconfNode = new NetconfNodeBuilder()
+                .setHost(new Host(new IpAddress(new Ipv4Address("127.0.0.1"))))
+                .setPort(new PortNumber(Uint16.valueOf(9999)))
+                .setReconnectOnChangedSchema(true)
+                .setSchemaless(true)
+                .setTcpOnly(false)
+                .setProtocol(null)
+                .setLockDatastore(true)
+                .setBackoffMultiplier(Decimal64.valueOf("1.5"))
+                .setConcurrentRpcLimit(Uint16.ONE)
+                // One reconnection attempt
+                .setMaxConnectionAttempts(Uint32.ONE)
+                .setDefaultRequestTimeoutMillis(Uint32.valueOf(1000))
+                .setMinBackoffMillis(Uint16.valueOf(100))
+                .setKeepaliveDelay(Uint32.valueOf(1000))
+                .setConnectionTimeoutMillis(Uint32.valueOf(1000))
+                .setMaxBackoffMillis(Uint32.valueOf(1000))
+                .setBackoffJitter(Decimal64.valueOf("0.0"))
+                .setCredentials(new LoginPwBuilder()
+                        .setLoginPassword(new LoginPasswordBuilder()
+                                .setUsername("testuser")
+                                .setPassword("bGlnaHQgd29y".getBytes(StandardCharsets.UTF_8)).build())
+                        .build())
+                .build();
+
+        final var testNode = new NodeBuilder()
+            .setNodeId(new NodeId("nodeId"))
+            .addAugmentation(netconfNode)
+            .build();
+
+        doNothing().when(delegate).close();
+        // throw exception when try to decrypt
+        doThrow(new GeneralSecurityException()).when(encryptionService).decrypt(any());
+
+        topology.onDataTreeChanged(testNode);
+        verify(delegate).close();
+        verify(encryptionService).decrypt(any());
+    }
+
+    private class TestingNetconfTopologyImpl extends AbstractNetconfTopology {
+
+        protected TestingNetconfTopologyImpl(final String topologyId, final NetconfClientFactory clientFactory,
+            final NetconfTimer timer, final NetconfTopologySchemaAssembler schemaAssembler,
+            final SchemaResourceManager schemaManager, final DataBroker dataBroker,
+            final DOMMountPointService mountPointService, final NetconfClientConfigurationBuilderFactory builderFactory,
+            final DeviceActionFactory deviceActionFactory,final BaseNetconfSchemaProvider baseSchemaProvider) {
+            super(topologyId, clientFactory, timer, schemaAssembler, schemaManager, dataBroker, mountPointService,
+                    builderFactory, deviceActionFactory, baseSchemaProvider);
+        }
+
+        // Want to simulate on data tree change
+        public void onDataTreeChanged(final Node node) {
+            ensureNode(node);
+        }
+
+        @Override
+        protected RemoteDeviceHandler createSalFacade(final RemoteDeviceId deviceId, final boolean lockDatastore) {
+            return delegate;
+        }
+    }
 }