diff --git a/crypto/src/crypto/agreement/DHAgreement.cs b/crypto/src/crypto/agreement/DHAgreement.cs
index b5af104f9..e988c0d53 100644
--- a/crypto/src/crypto/agreement/DHAgreement.cs
+++ b/crypto/src/crypto/agreement/DHAgreement.cs
@@ -85,7 +85,11 @@ namespace Org.BouncyCastle.Crypto.Agreement
BigInteger p = dhParams.P;
- BigInteger result = pub.Y.ModPow(privateValue, p);
+ BigInteger peerY = pub.Y;
+ if (peerY == null || peerY.CompareTo(BigInteger.One) <= 0 || peerY.CompareTo(p.Subtract(BigInteger.One)) >= 0)
+ throw new ArgumentException("Diffie-Hellman public key is weak");
+
+ BigInteger result = peerY.ModPow(privateValue, p);
if (result.Equals(BigInteger.One))
throw new InvalidOperationException("Shared key can't be 1");
diff --git a/crypto/src/crypto/agreement/DHBasicAgreement.cs b/crypto/src/crypto/agreement/DHBasicAgreement.cs
index d6f017e32..6c3fe6595 100644
--- a/crypto/src/crypto/agreement/DHBasicAgreement.cs
+++ b/crypto/src/crypto/agreement/DHBasicAgreement.cs
@@ -56,7 +56,13 @@ namespace Org.BouncyCastle.Crypto.Agreement
if (!pub.Parameters.Equals(dhParams))
throw new ArgumentException("Diffie-Hellman public key has wrong parameters.");
- BigInteger result = pub.Y.ModPow(key.X, dhParams.P);
+ BigInteger p = dhParams.P;
+
+ BigInteger peerY = pub.Y;
+ if (peerY == null || peerY.CompareTo(BigInteger.One) <= 0 || peerY.CompareTo(p.Subtract(BigInteger.One)) >= 0)
+ throw new ArgumentException("Diffie-Hellman public key is weak");
+
+ BigInteger result = peerY.ModPow(key.X, p);
if (result.Equals(BigInteger.One))
throw new InvalidOperationException("Shared key can't be 1");
diff --git a/crypto/src/crypto/parameters/DHPublicKeyParameters.cs b/crypto/src/crypto/parameters/DHPublicKeyParameters.cs
index 1704c47dc..e7aeeff19 100644
--- a/crypto/src/crypto/parameters/DHPublicKeyParameters.cs
+++ b/crypto/src/crypto/parameters/DHPublicKeyParameters.cs
@@ -46,7 +46,7 @@ namespace Org.BouncyCastle.Crypto.Parameters
this.y = Validate(y, parameters);
}
- public BigInteger Y
+ public virtual BigInteger Y
{
get { return y; }
}
diff --git a/crypto/test/src/test/DHTest.cs b/crypto/test/src/test/DHTest.cs
index 18a0d84d6..fca487008 100644
--- a/crypto/test/src/test/DHTest.cs
+++ b/crypto/test/src/test/DHTest.cs
@@ -633,6 +633,98 @@ namespace Org.BouncyCastle.Tests
}
}
+ internal static readonly string Message = "Hello";
+
+ internal static readonly SecureRandom rand = new SecureRandom();
+
+ public static DHParameters Ike2048()
+ {
+ BigInteger p = new BigInteger(
+ "ffffffffffffffffc90fdaa22168c234c4c6628b80dc1cd129024e088a67cc74"
+ + "020bbea63b139b22514a08798e3404ddef9519b3cd3a431b302b0a6df25f1437"
+ + "4fe1356d6d51c245e485b576625e7ec6f44c42e9a637ed6b0bff5cb6f406b7ed"
+ + "ee386bfb5a899fa5ae9f24117c4b1fe649286651ece45b3dc2007cb8a163bf05"
+ + "98da48361c55d39a69163fa8fd24cf5f83655d23dca3ad961c62f356208552bb"
+ + "9ed529077096966d670c354e4abc9804f1746c08ca18217c32905e462e36ce3b"
+ + "e39e772c180e86039b2783a2ec07a28fb5c55df06f4c52c9de2bcbf695581718"
+ + "3995497cea956ae515d2261898fa051015728e5a8aacaa68ffffffffffffffff", 16);
+ BigInteger g = new BigInteger("2");
+ return new DHParameters(p, g);
+ }
+
+ internal class DHWeakPubKey
+ : DHPublicKeyParameters
+ {
+ private readonly BigInteger weakY;
+
+ public DHWeakPubKey(BigInteger weakY, DHParameters parameters)
+ : base(BigInteger.Two, parameters)
+ {
+ this.weakY = weakY;
+ }
+
+ public override BigInteger Y
+ {
+ get { return weakY; }
+ }
+ }
+
+ /**
+ * Tests whether a provider accepts invalid public keys that result in predictable shared secrets.
+ * This test is based on RFC 2785, Section 4 and NIST SP 800-56A,
+ * If an attacker can modify both public keys in an ephemeral-ephemeral key agreement scheme then
+ * it may be possible to coerce both parties into computing the same predictable shared key.
+ * <p/>
+ * Note: the test is quite whimsical. If the prime p is not a safe prime then the provider itself
+ * cannot prevent all small-subgroup attacks because of the missing parameter q in the
+ * Diffie-Hellman parameters. Implementations must add additional countermeasures such as the ones
+ * proposed in RFC 2785.
+ */
+ [Test]
+ public void TestSubgroupConfinement()
+ {
+ DHParameters parameters = Ike2048();
+ BigInteger p = parameters.P, g = parameters.G;
+
+ IAsymmetricCipherKeyPairGenerator keyGen = GeneratorUtilities.GetKeyPairGenerator("DH");
+
+ //keyGen.initialize(params);
+ keyGen.Init(new DHKeyGenerationParameters(new SecureRandom(), parameters));
+
+ AsymmetricCipherKeyPair kp = keyGen.GenerateKeyPair();
+ AsymmetricKeyParameter priv = kp.Private;
+
+ IBasicAgreement ka = AgreementUtilities.GetBasicAgreement("DH");
+
+ BigInteger[] weakPublicKeys = { BigInteger.Zero, BigInteger.One, p.Subtract(BigInteger.One), p,
+ p.Add(BigInteger.One), BigInteger.One.Negate() };
+
+ foreach (BigInteger weakKey in weakPublicKeys)
+ {
+ try
+ {
+ new DHPublicKeyParameters(weakKey, parameters);
+ Fail("Generated weak public key");
+ }
+ catch (ArgumentException ex)
+ {
+ IsTrue("wrong message (constructor)", Platform.StartsWith(ex.Message, "invalid DH public key"));
+ }
+
+ ka.Init(priv);
+
+ try
+ {
+ ka.CalculateAgreement(new DHWeakPubKey(weakKey, parameters));
+ Fail("Generated secrets with weak public key");
+ }
+ catch (ArgumentException ex)
+ {
+ IsTrue("wrong message (CalculateAgreement)", "Diffie-Hellman public key is weak".Equals(ex.Message));
+ }
+ }
+ }
+
public override void PerformTest()
{
TestEnc();
@@ -640,6 +732,7 @@ namespace Org.BouncyCastle.Tests
TestECDH();
TestECDHC();
TestExceptions();
+ TestSubgroupConfinement();
}
public static void Main(
|