From e589f5691e8a1cbec2709ea305aba6d2360c77c4 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 31 Jan 2024 12:17:51 +0100 Subject: [PATCH] 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 --- .../callhome/mount/AuthorizedKeysDecoder.java | 105 +++++++++--------- .../mount/CallHomeMountSshAuthProvider.java | 9 +- 2 files changed, 56 insertions(+), 58 deletions(-) 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 6828c0ad8c..8779920967 100644 --- 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 ECDSA_CURVES = ImmutableMap.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 0bd92411cb..d2c0f8bc2e 100644 --- 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 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; -- 2.36.6