Make AuthorizedKeysDecoder thread-safe

AuthorizedKeysDecoder is mutating state of a single instance, which is
counter-intuitive and leads to the last decoded key to be left present
in the instance.

Fix this by making the deconding method static and keeping the instance
an internal thing, closing the leak and making the decoding process
inherently thread-safe.

Change-Id: I000b100773d206aee6b0e4f634f7d46a8b56544e
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/AuthorizedKeysDecoder.java b/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/AuthorizedKeysDecoder.java
index 6828c0ad8c430087ff8f9c9182e783f19b594e25..877992096756e5a12cc2966713080c076454935c 100644 (file)
--- a/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/AuthorizedKeysDecoder.java
+++ b/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/AuthorizedKeysDecoder.java
@@ -35,10 +35,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.netconf.
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/**
- * FIXME: This should be probably located at AAA library.
- */
-public class AuthorizedKeysDecoder {
+final class AuthorizedKeysDecoder {
     private static final Logger LOG = LoggerFactory.getLogger(AuthorizedKeysDecoder.class);
 
     private static final ImmutableMap<String, String> ECDSA_CURVES = ImmutableMap.<String, String>builder()
@@ -73,23 +70,62 @@ public class AuthorizedKeysDecoder {
         }
     }
 
-    private byte[] bytes = new byte[0];
-    private int pos = 0;
+    private final byte[] bytes;
+    private int pos;
 
-    public PublicKey decodePublicKey(final SshPublicKey keyLine) throws GeneralSecurityException {
+    private AuthorizedKeysDecoder(final SshPublicKey keyLine) {
         bytes = keyLine.getValue();
-        pos = 0;
+    }
 
-        final var type = decodeType();
+    static @NonNull PublicKey decodePublicKey(final @NonNull SshPublicKey keyLine) throws GeneralSecurityException {
+        final var instance = new AuthorizedKeysDecoder(keyLine);
+        final var type = instance.decodeType();
         return switch (type) {
-            case KEY_TYPE_RSA -> decodeAsRSA();
-            case KEY_TYPE_DSA -> decodeAsDSA();
-            case KEY_TYPE_ECDSA -> decodeAsEcDSA();
-            default -> throw new IllegalArgumentException("Unknown decode key type " + type + " in " + keyLine);
+            case KEY_TYPE_RSA -> instance.decodeAsRSA();
+            case KEY_TYPE_DSA -> instance.decodeAsDSA();
+            case KEY_TYPE_ECDSA -> instance.decodeAsEcDSA();
+            default -> throw new NoSuchAlgorithmException("Unknown decode key type " + type + " in " + keyLine);
         };
     }
 
-    private PublicKey decodeAsEcDSA() throws GeneralSecurityException {
+    static @NonNull SshPublicKey encodePublicKey(final PublicKey publicKey) throws IOException {
+        final var baos = new ByteArrayOutputStream();
+
+        try (var dout = new DataOutputStream(baos)) {
+            if (publicKey instanceof RSAPublicKey rsa) {
+                dout.writeInt(KEY_TYPE_RSA_BYTES.length);
+                dout.write(KEY_TYPE_RSA_BYTES);
+                encodeBigInt(dout, rsa.getPublicExponent());
+                encodeBigInt(dout, rsa.getModulus());
+            } else if (publicKey instanceof DSAPublicKey dsa) {
+                final var dsaParams = dsa.getParams();
+                dout.writeInt(KEY_TYPE_DSA_BYTES.length);
+                dout.write(KEY_TYPE_DSA_BYTES);
+                encodeBigInt(dout, dsaParams.getP());
+                encodeBigInt(dout, dsaParams.getQ());
+                encodeBigInt(dout, dsaParams.getG());
+                encodeBigInt(dout, dsa.getY());
+            } else if (publicKey instanceof ECPublicKey ec) {
+                dout.writeInt(KEY_TYPE_ECDSA_BYTES.length);
+                dout.write(KEY_TYPE_ECDSA_BYTES);
+                dout.writeInt(ECDSA_SUPPORTED_CURVE_NAME_BYTES.length);
+                dout.write(ECDSA_SUPPORTED_CURVE_NAME_BYTES);
+
+                final var q = ec.getQ();
+                final var coordX = q.getAffineXCoord().getEncoded();
+                final var coordY = q.getAffineYCoord().getEncoded();
+                dout.writeInt(coordX.length + coordY.length + 1);
+                dout.writeByte(0x04);
+                dout.write(coordX);
+                dout.write(coordY);
+            } else {
+                throw new IOException("Unknown public key encoding: " + publicKey);
+            }
+        }
+        return new SshPublicKey(baos.toByteArray());
+    }
+
+    private @NonNull PublicKey decodeAsEcDSA() throws GeneralSecurityException {
         if (EC_FACTORY == null) {
             throw new NoSuchAlgorithmException("ECDSA keys are not supported");
         }
@@ -102,7 +138,7 @@ public class AuthorizedKeysDecoder {
         return EC_FACTORY.generatePublic(new ECPublicKeySpec(point, params256r1));
     }
 
-    private PublicKey decodeAsDSA() throws GeneralSecurityException {
+    private @NonNull PublicKey decodeAsDSA() throws GeneralSecurityException {
         if (DSA_FACTORY == null) {
             throw new NoSuchAlgorithmException("RSA keys are not supported");
         }
@@ -114,7 +150,7 @@ public class AuthorizedKeysDecoder {
         return DSA_FACTORY.generatePublic(new DSAPublicKeySpec(y, p, q, g));
     }
 
-    private PublicKey decodeAsRSA() throws GeneralSecurityException {
+    private @NonNull PublicKey decodeAsRSA() throws GeneralSecurityException {
         if (RSA_FACTORY == null) {
             throw new NoSuchAlgorithmException("RSA keys are not supported");
         }
@@ -149,41 +185,4 @@ public class AuthorizedKeysDecoder {
         out.writeInt(bytes.length);
         out.write(bytes);
     }
-
-    public static @NonNull SshPublicKey encodePublicKey(final PublicKey publicKey) throws IOException {
-        final var baos = new ByteArrayOutputStream();
-
-        try (var dout = new DataOutputStream(baos)) {
-            if (publicKey instanceof RSAPublicKey rsa) {
-                dout.writeInt(KEY_TYPE_RSA_BYTES.length);
-                dout.write(KEY_TYPE_RSA_BYTES);
-                encodeBigInt(dout, rsa.getPublicExponent());
-                encodeBigInt(dout, rsa.getModulus());
-            } else if (publicKey instanceof DSAPublicKey dsa) {
-                final var dsaParams = dsa.getParams();
-                dout.writeInt(KEY_TYPE_DSA_BYTES.length);
-                dout.write(KEY_TYPE_DSA_BYTES);
-                encodeBigInt(dout, dsaParams.getP());
-                encodeBigInt(dout, dsaParams.getQ());
-                encodeBigInt(dout, dsaParams.getG());
-                encodeBigInt(dout, dsa.getY());
-            } else if (publicKey instanceof ECPublicKey ec) {
-                dout.writeInt(KEY_TYPE_ECDSA_BYTES.length);
-                dout.write(KEY_TYPE_ECDSA_BYTES);
-                dout.writeInt(ECDSA_SUPPORTED_CURVE_NAME_BYTES.length);
-                dout.write(ECDSA_SUPPORTED_CURVE_NAME_BYTES);
-
-                final var q = ec.getQ();
-                final var coordX = q.getAffineXCoord().getEncoded();
-                final var coordY = q.getAffineYCoord().getEncoded();
-                dout.writeInt(coordX.length + coordY.length + 1);
-                dout.writeByte(0x04);
-                dout.write(coordX);
-                dout.write(coordY);
-            } else {
-                throw new IOException("Unknown public key encoding: " + publicKey);
-            }
-        }
-        return new SshPublicKey(baos.toByteArray());
-    }
 }

diff --git a/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSshAuthProvider.java b/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSshAuthProvider.java
index 0bd92411cbf81dbde44e56f4db582d4ff459e1c3..d2c0f8bc2e6418fbf7963f3b4b3baf6ff0b570f7 100644 (file)
--- a/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSshAuthProvider.java
+++ b/apps/callhome-provider/src/main/java/org/opendaylight/netconf/callhome/mount/CallHomeMountSshAuthProvider.java
@@ -182,7 +182,6 @@ public final class CallHomeMountSshAuthProvider implements CallHomeSshAuthProvid
 
     private static final class DeviceConfig extends AbstractDeviceListener {
         private final ConcurrentMap<PublicKey, Device> byPublicKey = new ConcurrentHashMap<>();
-        private final AuthorizedKeysDecoder keyDecoder = new AuthorizedKeysDecoder();
 
         Device get(final PublicKey key) {
             return byPublicKey.get(key);
@@ -190,7 +189,7 @@ public final class CallHomeMountSshAuthProvider implements CallHomeSshAuthProvid
 
         @Override
         void addDevice(final SshPublicKey publicKey, final Device device) {
-            final PublicKey key = publicKey(publicKey, device);
+            final var key = publicKey(publicKey, device);
             if (key != null) {
                 byPublicKey.put(key, device);
             }
@@ -198,15 +197,15 @@ public final class CallHomeMountSshAuthProvider implements CallHomeSshAuthProvid
 
         @Override
         void removeDevice(final SshPublicKey publicKey, final Device device) {
-            final PublicKey key = publicKey(publicKey, device);
+            final var key = publicKey(publicKey, device);
             if (key != null) {
                 byPublicKey.remove(key);
             }
         }
 
-        private PublicKey publicKey(final SshPublicKey hostKey, final Device device) {
+        private static PublicKey publicKey(final SshPublicKey hostKey, final Device device) {
             try {
-                return keyDecoder.decodePublicKey(hostKey);
+                return AuthorizedKeysDecoder.decodePublicKey(hostKey);
             } catch (GeneralSecurityException e) {
                 LOG.error("Unable to decode SSH key for {}. Ignoring update for this device", device.getUniqueId(), e);
                 return null;