diff options
11 files changed, 70 insertions, 30 deletions
diff --git a/security-utils/src/main/java/com/yahoo/security/KeyUtils.java b/security-utils/src/main/java/com/yahoo/security/KeyUtils.java index cef0dd9a62e..47055a65618 100644 --- a/security-utils/src/main/java/com/yahoo/security/KeyUtils.java +++ b/security-utils/src/main/java/com/yahoo/security/KeyUtils.java @@ -280,6 +280,25 @@ public class KeyUtils { return Base64.getUrlEncoder().withoutPadding().encodeToString(toRawX25519PublicKeyBytes(publicKey)); } + // This sanity check is to avoid any DoS potential caused by passing in a very large key + // to a quadratic Base58 decoding routing. We don't do this for the encoding since we + // always control the input size for that case. + private static void verifyB58InputSmallEnoughToBeX25519Key(String key) { + if (key.length() > 64) { // a very wide margin... + throw new IllegalArgumentException("Input Base58 is too large to represent an X25519 key"); + } + } + + public static XECPublicKey fromBase58EncodedX25519PublicKey(String base58pk) { + verifyB58InputSmallEnoughToBeX25519Key(base58pk); + byte[] rawKeyBytes = Base58.codec().decode(base58pk); + return fromRawX25519PublicKey(rawKeyBytes); + } + + public static String toBase58EncodedX25519PublicKey(XECPublicKey publicKey) { + return Base58.codec().encode(toRawX25519PublicKeyBytes(publicKey)); + } + public static XECPrivateKey fromRawX25519PrivateKey(byte[] rawScalarBytes) { try { NamedParameterSpec paramSpec = new NamedParameterSpec("X25519"); @@ -309,6 +328,16 @@ public class KeyUtils { return Base64.getUrlEncoder().withoutPadding().encodeToString(toRawX25519PrivateKeyBytes(privateKey)); } + public static XECPrivateKey fromBase58EncodedX25519PrivateKey(String base58pk) { + verifyB58InputSmallEnoughToBeX25519Key(base58pk); + byte[] rawKeyBytes = Base58.codec().decode(base58pk); + return fromRawX25519PrivateKey(rawKeyBytes); + } + + public static String toBase58EncodedX25519PrivateKey(XECPrivateKey privateKey) { + return Base58.codec().encode(toRawX25519PrivateKeyBytes(privateKey)); + } + // TODO unify with generateKeypair()? public static KeyPair generateX25519KeyPair() { try { diff --git a/security-utils/src/main/java/com/yahoo/security/SealedSharedKey.java b/security-utils/src/main/java/com/yahoo/security/SealedSharedKey.java index a921b3baf87..65f149579f4 100644 --- a/security-utils/src/main/java/com/yahoo/security/SealedSharedKey.java +++ b/security-utils/src/main/java/com/yahoo/security/SealedSharedKey.java @@ -2,11 +2,6 @@ package com.yahoo.security; import java.nio.ByteBuffer; -import java.util.Arrays; -import java.util.Base64; - -import static com.yahoo.security.ArrayUtils.fromUtf8Bytes; -import static com.yahoo.security.ArrayUtils.toUtf8Bytes; /** * A SealedSharedKey represents the public part of a secure one-way ephemeral key exchange. @@ -51,7 +46,7 @@ public record SealedSharedKey(KeyId keyId, byte[] enc, byte[] ciphertext) { byte[] encBytes = new byte[encoded.remaining()]; encoded.get(encBytes); - return Base64.getUrlEncoder().withoutPadding().encodeToString(encBytes); + return Base62.codec().encode(encBytes); } /** @@ -59,7 +54,8 @@ public record SealedSharedKey(KeyId keyId, byte[] enc, byte[] ciphertext) { * created by a call to toTokenString(). */ public static SealedSharedKey fromTokenString(String tokenString) { - byte[] rawTokenBytes = Base64.getUrlDecoder().decode(tokenString); + verifyInputTokenStringNotTooLarge(tokenString); + byte[] rawTokenBytes = Base62.codec().decode(tokenString); if (rawTokenBytes.length < 1) { throw new IllegalArgumentException("Decoded token too small to contain a version"); } @@ -84,4 +80,12 @@ public record SealedSharedKey(KeyId keyId, byte[] enc, byte[] ciphertext) { public int tokenVersion() { return CURRENT_TOKEN_VERSION; } + private static void verifyInputTokenStringNotTooLarge(String tokenString) { + // Expected max decoded size for v1 is 3 + 255 + 32 + 32 = 322. For simplicity, round this + // up to 512 to effectively not have to care about the overhead of any reasonably chosen encoding. + if (tokenString.length() > 512) { + throw new IllegalArgumentException("Token string is too long to possibly be a valid token"); + } + } + } diff --git a/security-utils/src/test/java/com/yahoo/security/KeyUtilsTest.java b/security-utils/src/test/java/com/yahoo/security/KeyUtilsTest.java index dc6078c58b7..f44eadc59d4 100644 --- a/security-utils/src/test/java/com/yahoo/security/KeyUtilsTest.java +++ b/security-utils/src/test/java/com/yahoo/security/KeyUtilsTest.java @@ -155,9 +155,15 @@ public class KeyUtilsTest { var priv = xecPrivFromHex(privHex); assertEquals(privHex, xecHexFromPriv(priv)); + // Base 64 var privB64 = KeyUtils.toBase64EncodedX25519PrivateKey(priv); var priv2 = KeyUtils.fromBase64EncodedX25519PrivateKey(privB64); assertEquals(privHex, xecHexFromPriv(priv2)); + + // Base 58 + var privB58 = KeyUtils.toBase58EncodedX25519PrivateKey(priv); + var priv3 = KeyUtils.fromBase58EncodedX25519PrivateKey(privB58); + assertEquals(privHex, xecHexFromPriv(priv3)); } @Test @@ -166,9 +172,15 @@ public class KeyUtilsTest { var pub = xecPubFromHex(pubHex); assertEquals(pubHex, xecHexFromPub(pub)); + // Base 64 var pubB64 = KeyUtils.toBase64EncodedX25519PublicKey(pub); var pub2 = KeyUtils.fromBase64EncodedX25519PublicKey(pubB64); assertEquals(pubHex, xecHexFromPub(pub2)); + + // Base 58 + var pubB58 = KeyUtils.toBase58EncodedX25519PublicKey(pub); + var pub3 = KeyUtils.fromBase58EncodedX25519PublicKey(pubB58); + assertEquals(pubHex, xecHexFromPub(pub3)); } @Test diff --git a/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java b/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java index 74b4ca0854b..4e64bc3e9aa 100644 --- a/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java +++ b/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java @@ -14,7 +14,6 @@ import java.util.Arrays; import java.util.Base64; import static com.yahoo.security.ArrayUtils.hex; -import static com.yahoo.security.ArrayUtils.toUtf8Bytes; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -47,12 +46,12 @@ public class SharedKeyTest { @Test void token_v1_representation_is_stable() { - var receiverPrivate = KeyUtils.fromBase64EncodedX25519PrivateKey("4qGcntygFn_a3uqeBa1PbDlygQ-cpOuNznTPIz9ftWE"); - var receiverPublic = KeyUtils.fromBase64EncodedX25519PublicKey( "ROAH_S862tNMpbJ49lu1dPXFCPHFIXZK30pSrMZEmEg"); + var receiverPrivate = KeyUtils.fromBase58EncodedX25519PrivateKey("GFg54SaGNCmcSGufZCx68SKLGuAFrASoDeMk3t5AjU6L"); + var receiverPublic = KeyUtils.fromBase58EncodedX25519PublicKey( "5drrkakYLjYSBpr5Haknh13EiCYL36ndMzK4gTJo6pwh"); var keyId = KeyId.ofString("my key ID"); // Token generated for the above receiver public key, with the below expected shared secret (in hex) - var publicToken = "AQlteSBrZXkgSUQgAtTxJJdmv3eUoW5Z3NJSdZ3poKPEkW0SJOGQXP6CaC5XfyAVoUlK_NyYIMsJKyNYKU6WmagZpVG2zQGFJoqiFA"; + var publicToken = "OntP9gRVAjXeZIr4zkYqRJFcnA993v7ZEE7VbcNs1NcR3HdE7Mpwlwi3r3anF1kVa5fn7O1CyeHQpBWpdayUTKkrtyFepG6WJrZdE"; var expectedSharedSecret = "1b33b4dcd6a94e5a4a1ee6d208197d01"; var theirSealed = SealedSharedKey.fromTokenString(publicToken); diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/DecryptTool.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/DecryptTool.java index fc485eb92f2..f1c166ba934 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/DecryptTool.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/DecryptTool.java @@ -14,12 +14,9 @@ import org.apache.commons.cli.Option; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; -import java.util.Arrays; import java.util.List; import java.util.Optional; -import static com.yahoo.security.ArrayUtils.toUtf8Bytes; - /** * Tooling for decrypting a file using a private key that corresponds to the public key used * to originally encrypt the file. @@ -47,7 +44,7 @@ public class DecryptTool implements Tool { .longOpt(RECIPIENT_PRIVATE_KEY_FILE_OPTION) .hasArg(true) .required(false) - .desc("Recipient private key file") + .desc("Recipient private key file in Base58 encoded format") .build(), Option.builder("i") .longOpt(KEY_ID_OPTION) @@ -103,7 +100,7 @@ public class DecryptTool implements Tool { "used when generating the supplied token"); } } - var privateKey = KeyUtils.fromBase64EncodedX25519PrivateKey(Files.readString(privKeyPath).strip()); + var privateKey = KeyUtils.fromBase58EncodedX25519PrivateKey(Files.readString(privKeyPath).strip()); var secretShared = SharedKeyGenerator.fromSealedKey(sealedSharedKey, privateKey); var cipher = SharedKeyGenerator.makeAesGcmDecryptionCipher(secretShared); diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/EncryptTool.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/EncryptTool.java index 737bade400f..886433f00f8 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/EncryptTool.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/EncryptTool.java @@ -15,8 +15,6 @@ import java.nio.file.Files; import java.nio.file.Paths; import java.util.List; -import static com.yahoo.security.ArrayUtils.toUtf8Bytes; - /** * Tooling to encrypt a file using a public key, emitting a non-secret token that can be * passed on to a recipient holding the corresponding private key. @@ -42,7 +40,7 @@ public class EncryptTool implements Tool { .longOpt(RECIPIENT_PUBLIC_KEY_OPTION) .hasArg(true) .required(false) - .desc("Recipient X25519 public key in Base64 encoded format") + .desc("Recipient X25519 public key in Base58 encoded format") .build(), Option.builder("i") .longOpt(KEY_ID_OPTION) @@ -79,7 +77,7 @@ public class EncryptTool implements Tool { var inputArg = leftoverArgs[0]; var outputPath = Paths.get(CliUtils.optionOrThrow(arguments, OUTPUT_FILE_OPTION)); - var recipientPubKey = KeyUtils.fromBase64EncodedX25519PublicKey(CliUtils.optionOrThrow(arguments, RECIPIENT_PUBLIC_KEY_OPTION).strip()); + var recipientPubKey = KeyUtils.fromBase58EncodedX25519PublicKey(CliUtils.optionOrThrow(arguments, RECIPIENT_PUBLIC_KEY_OPTION).strip()); var keyId = KeyId.ofString(CliUtils.optionOrThrow(arguments, KEY_ID_OPTION)); var shared = SharedKeyGenerator.generateForReceiverPublicKey(recipientPubKey, keyId); var cipher = SharedKeyGenerator.makeAesGcmEncryptionCipher(shared); diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/KeygenTool.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/KeygenTool.java index d7885dc6455..3d5accde98f 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/KeygenTool.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/KeygenTool.java @@ -59,7 +59,7 @@ public class KeygenTool implements Tool { return new ToolDescription( "<options>", "Generates an X25519 key pair and stores its private/public parts in " + - "separate files in Base64 encoded form.", + "separate files in Base58 encoded form.", "Note: this is a BETA tool version; its interface may be changed at any time", OPTIONS); } @@ -101,8 +101,8 @@ public class KeygenTool implements Tool { var privFilePerms = PosixFilePermissions.fromString("rw-------"); Files.createFile( privOutPath, PosixFilePermissions.asFileAttribute(privFilePerms)); - Files.writeString(privOutPath, KeyUtils.toBase64EncodedX25519PrivateKey(privKey) + "\n"); - Files.writeString(pubOutPath, KeyUtils.toBase64EncodedX25519PublicKey(pubKey) + "\n"); + Files.writeString(privOutPath, KeyUtils.toBase58EncodedX25519PrivateKey(privKey) + "\n"); + Files.writeString(pubOutPath, KeyUtils.toBase58EncodedX25519PublicKey(pubKey) + "\n"); } catch (IOException e) { throw new RuntimeException(e); diff --git a/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java b/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java index f529ed828ea..d4992e89802 100644 --- a/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java +++ b/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java @@ -168,11 +168,11 @@ public class CryptoToolsTest { assertEquals(expectedPerms, privKeyPerms); } - private static final String TEST_PRIV_KEY = "4qGcntygFn_a3uqeBa1PbDlygQ-cpOuNznTPIz9ftWE"; - private static final String TEST_PUB_KEY = "ROAH_S862tNMpbJ49lu1dPXFCPHFIXZK30pSrMZEmEg"; + private static final String TEST_PRIV_KEY = "GFg54SaGNCmcSGufZCx68SKLGuAFrASoDeMk3t5AjU6L"; + private static final String TEST_PUB_KEY = "5drrkakYLjYSBpr5Haknh13EiCYL36ndMzK4gTJo6pwh"; // Token created for the above public key (matching the above private key), using key id "my key ID" - private static final String TEST_TOKEN = "AQlteSBrZXkgSUQgAtTxJJdmv3eUoW5Z3NJSdZ3poKPEkW0SJOG" + - "QXP6CaC5XfyAVoUlK_NyYIMsJKyNYKU6WmagZpVG2zQGFJoqiFA"; + private static final String TEST_TOKEN = "OntP9gRVAjXeZIr4zkYqRJFcnA993v7ZEE7VbcNs1NcR3HdE7Mp" + + "wlwi3r3anF1kVa5fn7O1CyeHQpBWpdayUTKkrtyFepG6WJrZdE"; private static final String TEST_TOKEN_KEY_ID = "my key ID"; @Test diff --git a/vespaclient-java/src/test/resources/expected-decrypt-help-output.txt b/vespaclient-java/src/test/resources/expected-decrypt-help-output.txt index ef59741cd30..ddf91c779e2 100644 --- a/vespaclient-java/src/test/resources/expected-decrypt-help-output.txt +++ b/vespaclient-java/src/test/resources/expected-decrypt-help-output.txt @@ -10,7 +10,8 @@ the quotes). this is not provided, the key ID stored as part of the token is not verified. - -k,--recipient-private-key-file <arg> Recipient private key file + -k,--recipient-private-key-file <arg> Recipient private key file in + Base58 encoded format -o,--output-file <arg> Output file for decrypted plaintext. Specify '-' (without the quotes) to write plaintext to diff --git a/vespaclient-java/src/test/resources/expected-encrypt-help-output.txt b/vespaclient-java/src/test/resources/expected-encrypt-help-output.txt index 5e1da32cbe7..beddc69855b 100644 --- a/vespaclient-java/src/test/resources/expected-encrypt-help-output.txt +++ b/vespaclient-java/src/test/resources/expected-encrypt-help-output.txt @@ -10,7 +10,7 @@ the quotes). -i,--key-id <arg> Numeric ID of recipient key -o,--output-file <arg> Output file (will be truncated if it already exists) - -r,--recipient-public-key <arg> Recipient X25519 public key in Base64 + -r,--recipient-public-key <arg> Recipient X25519 public key in Base58 encoded format Note: this is a BETA tool version; its interface may be changed at any time diff --git a/vespaclient-java/src/test/resources/expected-keygen-help-output.txt b/vespaclient-java/src/test/resources/expected-keygen-help-output.txt index 60629c4291f..f386f6d2e3a 100644 --- a/vespaclient-java/src/test/resources/expected-keygen-help-output.txt +++ b/vespaclient-java/src/test/resources/expected-keygen-help-output.txt @@ -1,6 +1,6 @@ usage: vespa-security keygen <options> Generates an X25519 key pair and stores its private/public parts in -separate files in Base64 encoded form. +separate files in Base58 encoded form. -h,--help Show help -k,--private-out-file <arg> Output file for private (secret) key. Will be created with restrictive file |