Skip to content

Commit

Permalink
BuiltInCiphers: fix ChaCha20-Poly1305 key size
Browse files Browse the repository at this point in the history 
64 bytes is 512 bits, not 256. Also remove the "kdfSize" parameter from
the constructor. It's pointless and just a source of hard to spot
consistency errors if one has to give the key size twice, first in
bytes, then in bits.

Just give the key size in bits, and compute the number of bytes.
  • Loading branch information
@tomaswolf
tomaswolf committed Sep 5, 2024
1 parent ff9fc91 commit f039b68
Showing 1 changed file with 21 additions and 23 deletions.
44 changes: 21 additions & 23 deletions sshd-common/src/main/java/org/apache/sshd/common/cipher/BuiltinCiphers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
* @author <a href="mailto:[email protected]">Apache MINA SSHD Project</a>
*/
public enum BuiltinCiphers implements CipherFactory {
none(Constants.NONE, 0, 0, 0, "None", 0, "None", 8) {
none(Constants.NONE, 0, 0, "None", 0, "None", 8) {
@Override
public Cipher create() {
return new CipherNone();
Expand All @@ -59,58 +59,58 @@ public boolean isSupported() {
return !SecurityUtils.isFipsMode();
}
},
aes128cbc(Constants.AES128_CBC, 16, 0, 16, "AES", 128, "AES/CBC/NoPadding", 16) {
aes128cbc(Constants.AES128_CBC, 16, 0, "AES", 128, "AES/CBC/NoPadding", 16) {
@Override
public Cipher create() {
return new BaseCBCCipher(getIVSize(), getAuthenticationTagSize(), getKdfSize(), getAlgorithm(), getKeySize(),
getTransformation(), getCipherBlockSize());
}
},
aes128ctr(Constants.AES128_CTR, 16, 0, 16, "AES", 128, "AES/CTR/NoPadding", 16) {
aes128ctr(Constants.AES128_CTR, 16, 0, "AES", 128, "AES/CTR/NoPadding", 16) {
@Override
public Cipher create() {
return new BaseCTRCipher(getIVSize(), getAuthenticationTagSize(), getKdfSize(), getAlgorithm(), getKeySize(),
getTransformation(), getCipherBlockSize());
}
},
aes128gcm(Constants.AES128_GCM, 12, 16, 16, "AES", 128, "AES/GCM/NoPadding", 16) {
aes128gcm(Constants.AES128_GCM, 12, 16, "AES", 128, "AES/GCM/NoPadding", 16) {
@Override
public Cipher create() {
return new BaseGCMCipher(
getIVSize(), getAuthenticationTagSize(), getKdfSize(), getAlgorithm(),
getKeySize(), getTransformation(), getCipherBlockSize());
}
},
aes256gcm(Constants.AES256_GCM, 12, 16, 32, "AES", 256, "AES/GCM/NoPadding", 16) {
aes256gcm(Constants.AES256_GCM, 12, 16, "AES", 256, "AES/GCM/NoPadding", 16) {
@Override
public Cipher create() {
return new BaseGCMCipher(
getIVSize(), getAuthenticationTagSize(), getKdfSize(), getAlgorithm(),
getKeySize(), getTransformation(), getCipherBlockSize());
}
},
aes192cbc(Constants.AES192_CBC, 16, 0, 24, "AES", 192, "AES/CBC/NoPadding", 16) {
aes192cbc(Constants.AES192_CBC, 16, 0, "AES", 192, "AES/CBC/NoPadding", 16) {
@Override
public Cipher create() {
return new BaseCBCCipher(getIVSize(), getAuthenticationTagSize(), getKdfSize(), getAlgorithm(), getKeySize(),
getTransformation(), getCipherBlockSize());
}
},
aes192ctr(Constants.AES192_CTR, 16, 0, 24, "AES", 192, "AES/CTR/NoPadding", 16) {
aes192ctr(Constants.AES192_CTR, 16, 0, "AES", 192, "AES/CTR/NoPadding", 16) {
@Override
public Cipher create() {
return new BaseCTRCipher(getIVSize(), getAuthenticationTagSize(), getKdfSize(), getAlgorithm(), getKeySize(),
getTransformation(), getCipherBlockSize());
}
},
aes256cbc(Constants.AES256_CBC, 16, 0, 32, "AES", 256, "AES/CBC/NoPadding", 16) {
aes256cbc(Constants.AES256_CBC, 16, 0, "AES", 256, "AES/CBC/NoPadding", 16) {
@Override
public Cipher create() {
return new BaseCBCCipher(getIVSize(), getAuthenticationTagSize(), getKdfSize(), getAlgorithm(), getKeySize(),
getTransformation(), getCipherBlockSize());
}
},
aes256ctr(Constants.AES256_CTR, 16, 0, 32, "AES", 256, "AES/CTR/NoPadding", 16) {
aes256ctr(Constants.AES256_CTR, 16, 0, "AES", 256, "AES/CTR/NoPadding", 16) {
@Override
public Cipher create() {
return new BaseCTRCipher(getIVSize(), getAuthenticationTagSize(), getKdfSize(), getAlgorithm(), getKeySize(),
Expand All @@ -122,7 +122,7 @@ public Cipher create() {
* @see <A HREF="https://issues.apache.org/jira/browse/SSHD-1004">SSHD-1004</A>
*/
@Deprecated
arcfour128(Constants.ARCFOUR128, 8, 0, 16, "ARCFOUR", 128, "RC4", 8) {
arcfour128(Constants.ARCFOUR128, 8, 0, "ARCFOUR", 128, "RC4", 8) {
@Override
public Cipher create() {
return new BaseRC4Cipher(getIVSize(), getKdfSize(), getKeySize(), getCipherBlockSize());
Expand All @@ -133,7 +133,7 @@ public Cipher create() {
* @see <A HREF="https://issues.apache.org/jira/browse/SSHD-1004">SSHD-1004</A>
*/
@Deprecated
arcfour256(Constants.ARCFOUR256, 8, 0, 32, "ARCFOUR", 256, "RC4", 8) {
arcfour256(Constants.ARCFOUR256, 8, 0, "ARCFOUR", 256, "RC4", 8) {
@Override
public Cipher create() {
return new BaseRC4Cipher(getIVSize(), getKdfSize(), getKeySize(), getCipherBlockSize());
Expand All @@ -144,14 +144,14 @@ public Cipher create() {
* @see <A HREF="https://issues.apache.org/jira/browse/SSHD-1004">SSHD-1004</A>
*/
@Deprecated
blowfishcbc(Constants.BLOWFISH_CBC, 8, 0, 16, "Blowfish", 128, "Blowfish/CBC/NoPadding", 8) {
blowfishcbc(Constants.BLOWFISH_CBC, 8, 0, "Blowfish", 128, "Blowfish/CBC/NoPadding", 8) {
@Override
public Cipher create() {
return new BaseCBCCipher(getIVSize(), getAuthenticationTagSize(), getKdfSize(), getAlgorithm(), getKeySize(),
getTransformation(), getCipherBlockSize());
}
},
cc20p1305_openssh(Constants.CC20P1305_OPENSSH, 8, 16, 64, "ChaCha", 256, "ChaCha", 8) {
cc20p1305_openssh(Constants.CC20P1305_OPENSSH, 8, 16, "ChaCha", 512, "ChaCha", 8) {
@Override
public Cipher create() {
return new ChaCha20Cipher();
Expand All @@ -167,7 +167,7 @@ public boolean isSupported() {
* @see <A HREF="https://issues.apache.org/jira/browse/SSHD-1004">SSHD-1004</A>
*/
@Deprecated
tripledescbc(Constants.TRIPLE_DES_CBC, 8, 0, 24, "DESede", 192, "DESede/CBC/NoPadding", 8) {
tripledescbc(Constants.TRIPLE_DES_CBC, 8, 0, "DESede", 192, "DESede/CBC/NoPadding", 8) {
@Override
public Cipher create() {
return new BaseCBCCipher(getIVSize(), getAuthenticationTagSize(), getKdfSize(), getAlgorithm(), getKeySize(),
Expand All @@ -182,21 +182,19 @@ public Cipher create() {
private final String factoryName;
private final int ivsize;
private final int authSize;
private final int kdfSize;
private final int keysize;
private final int keySize;
private final int blkSize;
private final String algorithm;
private final String transformation;
private final AtomicReference<Boolean> supported = new AtomicReference<>();

BuiltinCiphers(
String factoryName, int ivsize, int authSize, int kdfSize,
String algorithm, int keySize, String transformation, int blkSize) {
String factoryName, int ivsize, int authSize, String algorithm, int keySizeInBits, String transformation,
int blkSize) {
this.factoryName = factoryName;
this.ivsize = ivsize;
this.authSize = authSize;
this.kdfSize = kdfSize;
this.keysize = keySize;
this.keySize = keySizeInBits;
this.algorithm = algorithm;
this.transformation = transformation;
this.blkSize = blkSize;
Expand All @@ -220,7 +218,7 @@ public final String toString() {
public boolean isSupported() {
Boolean value = supported.get();
if (value == null) {
value = Cipher.checkSupported(this.transformation, this.keysize);
value = Cipher.checkSupported(this.transformation, this.keySize);
if (!supported.compareAndSet(null, value)) {
value = supported.get();
}
Expand All @@ -230,7 +228,7 @@ public boolean isSupported() {

@Override
public int getKeySize() {
return keysize;
return keySize;
}

@Override
Expand All @@ -245,7 +243,7 @@ public int getAuthenticationTagSize() {

@Override
public int getKdfSize() {
return kdfSize;
return keySize / Byte.SIZE;
}

@Override
Expand Down

0 comments on commit f039b68

Please sign in to comment.