Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Post-Quantum Cryptography (PQC) Algorithms for TLS 1.3 Key Exchange #1560

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ private enum All
ffdhe3072(NamedGroup.ffdhe3072, "DiffieHellman"),
ffdhe4096(NamedGroup.ffdhe4096, "DiffieHellman"),
ffdhe6144(NamedGroup.ffdhe6144, "DiffieHellman"),
ffdhe8192(NamedGroup.ffdhe8192, "DiffieHellman");
ffdhe8192(NamedGroup.ffdhe8192, "DiffieHellman"),

kyber512(NamedGroup.kyber512, "KEM"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are incorrect. We only support ML-KEM (FIPS draft), not round 3 Kyber.

kyber768(NamedGroup.kyber768, "KEM"),
kyber1024(NamedGroup.kyber1024, "KEM");

private final int namedGroup;
private final String name;
Expand Down
36 changes: 34 additions & 2 deletions tls/src/main/java/org/bouncycastle/tls/NamedGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ public class NamedGroup
public static final int arbitrary_explicit_prime_curves = 0xFF01;
public static final int arbitrary_explicit_char2_curves = 0xFF02;

public static final int kyber512 = 0x023A;
Copy link

@hwupathum hwupathum Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in #1578, the NIST round 3 implementation of Kyber is used in liboqs and FIPS implementation is used in BouncyCastle for the oids mentioned in this PR.

| kyber512 | 0x023A | Yes | OQS_CODEPOINT_KYBER512 |

In liboqs, the FIPS implementation uses a separate OID and a separate name

| mlkem512 | 0x0247 | Yes | OQS_CODEPOINT_MLKEM512 |

Therefore to keep interoperability between BC and liboqs, we need to support both NIST3 and FIPS implementations for BC. Specially since both Cloudflare and Google chrome supports X25519Kyber768 which used NIST round 3 implementation of Kyber. It is important to support the older KEMs as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hwupathum Note that that issue is stale. In 0.10.0, liboqs now includes draft ML-KEM in addition to round 3 Kyber.

Copy link

@hwupathum hwupathum Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but as I mentioned the algorithm IDs are different between liboqs and BC. Therefore if I try to do a TLS handshake between a BC and liboqs with kyber768, it fails due to algorithm id mismatch. Also if I try to use mlkem768 in liboqs, the handshake fails due to shared secret mismatch.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hwupathum Yes, I would expect the first to fail, because Round 3 Kyber and draft ML-DSA are different.

The latter I would've expected to succeed. I'd suggest we should fix the compatibility for this case.

I don't think support for Round 3 Kyber will remain once NIST publishes drafts. I'd imagine both Cloudflare and Google will update fairly quickly to the standardized (non-draft) ML-KEM implementation.

public static final int kyber768 = 0x023C;
public static final int kyber1024 = 0x023D;

/* Names of the actual underlying elliptic curves (not necessarily matching the NamedGroup names). */
private static final String[] CURVE_NAMES = new String[] { "sect163k1", "sect163r1", "sect163r2", "sect193r1",
"sect193r2", "sect233k1", "sect233r1", "sect239k1", "sect283k1", "sect283r1", "sect409k1", "sect409r1",
Expand Down Expand Up @@ -130,7 +134,8 @@ public static boolean canBeNegotiated(int namedGroup, ProtocolVersion version)
else
{
if ((namedGroup >= brainpoolP256r1tls13 && namedGroup <= brainpoolP512r1tls13)
|| (namedGroup == curveSM2))
|| (namedGroup == curveSM2)
|| (namedGroup == kyber512 || namedGroup == kyber768 || namedGroup == kyber1024))
{
return false;
}
Expand Down Expand Up @@ -260,6 +265,21 @@ public static String getFiniteFieldName(int namedGroup)
return null;
}

public static String getKEMName(int namedGroup)
{
switch (namedGroup)
{
case kyber512:
return "kyber512";
case kyber768:
return "kyber768";
case kyber1024:
return "kyber1024";
default:
return null;
}
}

public static int getMaximumChar2CurveBits()
{
return 571;
Expand Down Expand Up @@ -344,6 +364,12 @@ public static String getStandardName(int namedGroup)
return finiteFieldName;
}

String kemName = getKEMName(namedGroup);
if (null != kemName)
{
return kemName;
}

return null;
}

Expand Down Expand Up @@ -412,9 +438,15 @@ public static boolean refersToASpecificFiniteField(int namedGroup)
return namedGroup >= ffdhe2048 && namedGroup <= ffdhe8192;
}

public static boolean refersToASpecificKEM(int namedGroup)
{
return namedGroup == kyber512 || namedGroup == kyber768 || namedGroup == kyber1024;
}

public static boolean refersToASpecificGroup(int namedGroup)
{
return refersToASpecificCurve(namedGroup)
|| refersToASpecificFiniteField(namedGroup);
|| refersToASpecificFiniteField(namedGroup)
|| refersToASpecificKEM(namedGroup);
}
}
1 change: 1 addition & 0 deletions tls/src/main/java/org/bouncycastle/tls/NamedGroupRole.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ public class NamedGroupRole
public static final int dh = 1;
public static final int ecdh = 2;
public static final int ecdsa = 3;
public static final int kem = 4;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@

import org.bouncycastle.tls.crypto.TlsAgreement;
import org.bouncycastle.tls.crypto.TlsCrypto;
import org.bouncycastle.tls.crypto.TlsCryptoParameters;
import org.bouncycastle.tls.crypto.TlsDHConfig;
import org.bouncycastle.tls.crypto.TlsECConfig;
import org.bouncycastle.tls.crypto.TlsKEMConfig;
import org.bouncycastle.tls.crypto.TlsSecret;
import org.bouncycastle.util.Arrays;

Expand Down Expand Up @@ -405,16 +407,21 @@ else if (NamedGroup.refersToASpecificFiniteField(namedGroup))
{
agreement = crypto.createDHDomain(new TlsDHConfig(namedGroup, true)).createDH();
}
else if (NamedGroup.refersToASpecificKEM(namedGroup))
{
agreement = crypto.createKEMDomain(new TlsKEMConfig(namedGroup, new TlsCryptoParameters(tlsServerContext))).createKEM();
}
else
{
throw new TlsFatalAlert(AlertDescription.internal_error);
}

agreement.receivePeerValue(clientShare.getKeyExchange());

byte[] key_exchange = agreement.generateEphemeral();
KeyShareEntry serverShare = new KeyShareEntry(namedGroup, key_exchange);
TlsExtensionsUtils.addKeyShareServerHello(serverHelloExtensions, serverShare);

agreement.receivePeerValue(clientShare.getKeyExchange());
sharedSecret = agreement.calculateSecret();
}

Expand Down
16 changes: 13 additions & 3 deletions tls/src/main/java/org/bouncycastle/tls/TlsUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.bouncycastle.tls.crypto.TlsEncryptor;
import org.bouncycastle.tls.crypto.TlsHash;
import org.bouncycastle.tls.crypto.TlsHashOutputStream;
import org.bouncycastle.tls.crypto.TlsKEMConfig;
import org.bouncycastle.tls.crypto.TlsSecret;
import org.bouncycastle.tls.crypto.TlsStreamSigner;
import org.bouncycastle.tls.crypto.TlsStreamVerifier;
Expand Down Expand Up @@ -4022,6 +4023,7 @@ public static Vector getNamedGroupRoles(Vector keyExchangeAlgorithms)
// TODO[tls13] We're conservatively adding both here, though maybe only one is needed
addToSet(result, NamedGroupRole.dh);
addToSet(result, NamedGroupRole.ecdh);
addToSet(result, NamedGroupRole.kem);
break;
}
}
Expand Down Expand Up @@ -5303,7 +5305,7 @@ static Hashtable addKeyShareToClientHello(TlsClientContext clientContext, TlsCli
Hashtable clientAgreements = new Hashtable(3);
Vector clientShares = new Vector(2);

collectKeyShares(clientContext.getCrypto(), supportedGroups, keyShareGroups, clientAgreements, clientShares);
collectKeyShares(clientContext, supportedGroups, keyShareGroups, clientAgreements, clientShares);

// TODO[tls13-psk] When clientShares empty, consider not adding extension if pre_shared_key in use
TlsExtensionsUtils.addKeyShareClientHello(clientExtensions, clientShares);
Expand All @@ -5319,7 +5321,7 @@ static Hashtable addKeyShareToClientHelloRetry(TlsClientContext clientContext, H
Hashtable clientAgreements = new Hashtable(1, 1.0f);
Vector clientShares = new Vector(1);

collectKeyShares(clientContext.getCrypto(), supportedGroups, keyShareGroups, clientAgreements, clientShares);
collectKeyShares(clientContext, supportedGroups, keyShareGroups, clientAgreements, clientShares);

TlsExtensionsUtils.addKeyShareClientHello(clientExtensions, clientShares);

Expand All @@ -5332,9 +5334,10 @@ static Hashtable addKeyShareToClientHelloRetry(TlsClientContext clientContext, H
return clientAgreements;
}

private static void collectKeyShares(TlsCrypto crypto, int[] supportedGroups, Vector keyShareGroups,
private static void collectKeyShares(TlsClientContext clientContext, int[] supportedGroups, Vector keyShareGroups,
Hashtable clientAgreements, Vector clientShares) throws IOException
{
TlsCrypto crypto = clientContext.getCrypto();
if (isNullOrEmpty(supportedGroups))
{
return;
Expand Down Expand Up @@ -5371,6 +5374,13 @@ else if (NamedGroup.refersToASpecificFiniteField(supportedGroup))
agreement = crypto.createDHDomain(new TlsDHConfig(supportedGroup, true)).createDH();
}
}
else if (NamedGroup.refersToASpecificKEM(supportedGroup))
{
if (crypto.hasKEMAgreement())
{
agreement = crypto.createKEMDomain(new TlsKEMConfig(supportedGroup, new TlsCryptoParameters(clientContext))).createKEM();
}
}

if (null != agreement)
{
Expand Down
15 changes: 15 additions & 0 deletions tls/src/main/java/org/bouncycastle/tls/crypto/TlsCrypto.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ public interface TlsCrypto
*/
boolean hasECDHAgreement();

/**
* Return true if this TlsCrypto can support KEM key agreement.
*
* @return true if this instance can support KEM key agreement, false otherwise.
*/
boolean hasKEMAgreement();

/**
* Return true if this TlsCrypto can support the passed in block/stream encryption algorithm.
*
Expand Down Expand Up @@ -213,6 +220,14 @@ TlsCipher createCipher(TlsCryptoParameters cryptoParams, int encryptionAlgorithm
*/
TlsECDomain createECDomain(TlsECConfig ecConfig);

/**
* Create a domain object supporting the domain parameters described in kemConfig.
*
* @param kemConfig the config describing the KEM parameters to use.
* @return a TlsKEMDomain supporting the parameters in kemConfig.
*/
TlsKEMDomain createKEMDomain(TlsKEMConfig kemConfig);

/**
* Adopt the passed in secret, creating a new copy of it.
*
Expand Down
52 changes: 52 additions & 0 deletions tls/src/main/java/org/bouncycastle/tls/crypto/TlsKEMConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package org.bouncycastle.tls.crypto;

public class TlsKEMConfig
{
protected final int namedGroup;
protected final TlsCryptoParameters cryptoParams;
protected final int kemNamedGroup;

public TlsKEMConfig(int namedGroup, TlsCryptoParameters cryptoParams)
{
this.namedGroup = namedGroup;
this.cryptoParams = cryptoParams;
this.kemNamedGroup = getKEMNamedGroup(namedGroup);
}

public int getNamedGroup()
{
return namedGroup;
}

public boolean isServer()
{
return cryptoParams.isServer();
}

public int getKEMNamedGroup()
{
return kemNamedGroup;
}

private int getKEMNamedGroup(int namedGroup)
{
return namedGroup;
// switch (namedGroup)
// {
// case NamedGroup.kyber512:
// case NamedGroup.secp256Kyber512:
// case NamedGroup.x25519Kyber512:
// return NamedGroup.kyber512;
// case NamedGroup.kyber768:
// case NamedGroup.secp384Kyber768:
// case NamedGroup.x25519Kyber768:
// case NamedGroup.x448Kyber768:
// return NamedGroup.kyber768;
// case NamedGroup.kyber1024:
// case NamedGroup.secp521Kyber1024:
// return NamedGroup.kyber1024;
// default:
// return namedGroup;
// }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.bouncycastle.tls.crypto;

public interface TlsKEMDomain
{
TlsAgreement createKEM();
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
import org.bouncycastle.tls.crypto.TlsECDomain;
import org.bouncycastle.tls.crypto.TlsHMAC;
import org.bouncycastle.tls.crypto.TlsHash;
import org.bouncycastle.tls.crypto.TlsKEMConfig;
import org.bouncycastle.tls.crypto.TlsKEMDomain;
import org.bouncycastle.tls.crypto.TlsNonceGenerator;
import org.bouncycastle.tls.crypto.TlsSRP6Client;
import org.bouncycastle.tls.crypto.TlsSRP6Server;
Expand Down Expand Up @@ -211,6 +213,11 @@ public TlsECDomain createECDomain(TlsECConfig ecConfig)
}
}

public TlsKEMDomain createKEMDomain(TlsKEMConfig kemConfig)
{
return new BcTlsKyberDomain(this, kemConfig);
}

public TlsNonceGenerator createNonceGenerator(byte[] additionalSeedMaterial)
{
int cryptoHashAlgorithm = CryptoHashAlgorithm.sha256;
Expand Down Expand Up @@ -304,6 +311,11 @@ public boolean hasECDHAgreement()
return true;
}

public boolean hasKEMAgreement()
{
return true;
}

public boolean hasEncryptionAlgorithm(int encryptionAlgorithm)
{
switch (encryptionAlgorithm)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package org.bouncycastle.tls.crypto.impl.bc;

import java.io.IOException;
import org.bouncycastle.crypto.AsymmetricCipherKeyPair;
import org.bouncycastle.crypto.SecretWithEncapsulation;
import org.bouncycastle.pqc.crypto.crystals.kyber.KyberPrivateKeyParameters;
import org.bouncycastle.pqc.crypto.crystals.kyber.KyberPublicKeyParameters;
import org.bouncycastle.tls.crypto.TlsAgreement;
import org.bouncycastle.tls.crypto.TlsSecret;
import org.bouncycastle.util.Arrays;

public class BcTlsKyber implements TlsAgreement
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rename this as well to BcTlsMlKem perhaps :-)

(Comments apply below as well)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to drop a reference to https://datatracker.ietf.org/doc/draft-connolly-tls-mlkem-key-agreement/ as well.

{
protected final BcTlsKyberDomain domain;

protected AsymmetricCipherKeyPair localKeyPair;
protected KyberPublicKeyParameters peerPublicKey;
protected byte[] ciphertext;
protected byte[] secret;

public BcTlsKyber(BcTlsKyberDomain domain)
{
this.domain = domain;
}

public byte[] generateEphemeral() throws IOException
{
if (domain.getTlsKEMConfig().isServer())
{
return Arrays.clone(ciphertext);
}
else
{
this.localKeyPair = domain.generateKeyPair();
return domain.encodePublicKey((KyberPublicKeyParameters)localKeyPair.getPublic());
}
}

public void receivePeerValue(byte[] peerValue) throws IOException
{
if (domain.getTlsKEMConfig().isServer())
{
this.peerPublicKey = domain.decodePublicKey(peerValue);
SecretWithEncapsulation encap = domain.enCap(peerPublicKey);
ciphertext = encap.getEncapsulation();
secret = encap.getSecret();
}
else
{
this.ciphertext = Arrays.clone(peerValue);
}
}

public TlsSecret calculateSecret() throws IOException
{
if (domain.getTlsKEMConfig().isServer())
{
return domain.adoptLocalSecret(secret);
}
else
{
return domain.adoptLocalSecret(domain.deCap((KyberPrivateKeyParameters)localKeyPair.getPrivate(), ciphertext));
}
}
}
Loading