Fix endianness issue and key generation in ECDH

Author: filipnavara

crypto/src/bcpg/ECDHPublicBCPGKey.cs

@@ -1,6 +1,7 @@
 using System;
 
 using Org.BouncyCastle.Asn1;
+using Org.BouncyCastle.Math;
 using Org.BouncyCastle.Math.EC;
 
 namespace Org.BouncyCastle.Bcpg
@@ -48,6 +49,21 @@ public ECDHPublicBcpgKey(
             VerifySymmetricKeyAlgorithm();
         }
 
+        public ECDHPublicBcpgKey(
+            DerObjectIdentifier oid,
+            BigInteger encodedPoint,
+            HashAlgorithmTag hashAlgorithm,
+            SymmetricKeyAlgorithmTag symmetricKeyAlgorithm)
+            : base(oid, encodedPoint)
+        {
+            reserved = 1;
+            hashFunctionId = hashAlgorithm;
+            symAlgorithmId = symmetricKeyAlgorithm;
+
+            VerifyHashAlgorithm();
+            VerifySymmetricKeyAlgorithm();
+        }
+
         public virtual byte Reserved
         {
             get { return reserved; }

crypto/src/openpgp/PgpEncryptedDataGenerator.cs

@@ -104,6 +104,8 @@ public override void AddSessionInfo(
 
             private byte[] EncryptSessionInfo(byte[] sessionInfo, SecureRandom random)
             {
+                AsymmetricKeyParameter akp = pubKey.GetKey();
+                
                 if (pubKey.Algorithm != PublicKeyAlgorithmTag.ECDH)
                 {
                     IBufferedCipher c;
@@ -127,7 +129,6 @@ private byte[] EncryptSessionInfo(byte[] sessionInfo, SecureRandom random)
                             throw new PgpException("unknown asymmetric algorithm: " + pubKey.Algorithm);
                     }
 
-                    AsymmetricKeyParameter akp = pubKey.GetKey();
                     c.Init(true, new ParametersWithRandom(akp, random));
                     return c.DoFinal(sessionInfo);
                 }
@@ -136,12 +137,13 @@ private byte[] EncryptSessionInfo(byte[] sessionInfo, SecureRandom random)
                 KeyParameter key;
                 byte[] encodedPublicKey;
 
-                if (ecKey.CurveOid.Id.Equals(MiscObjectIdentifiers.Curve25519.Id))
+                if (akp is X25519PublicKeyParameters)
                 {
+                    X25519PublicKeyParameters pub = (X25519PublicKeyParameters)akp;
                     byte[] privateKey = new byte[X25519.PointSize];
                     X25519.GeneratePrivateKey(random, privateKey);
                     byte[] sharedKey = new byte[32];
-                    X25519.CalculateAgreement(privateKey, 0, BigIntegers.AsUnsignedByteArray(ecKey.EncodedPoint), 1, sharedKey, 0);
+                    X25519.CalculateAgreement(privateKey, 0, pub.GetEncoded(), 0, sharedKey, 0);
                     byte[] publicKey = new byte[X25519.PointSize + 1];
                     publicKey[0] = 0x40; // compressed point
                     X25519.GeneratePublicKey(privateKey, 0, publicKey, 1);
@@ -158,7 +160,7 @@ private byte[] EncryptSessionInfo(byte[] sessionInfo, SecureRandom random)
                     ECPrivateKeyParameters ephPriv = (ECPrivateKeyParameters)ephKp.Private;
                     ECPublicKeyParameters ephPub = (ECPublicKeyParameters)ephKp.Public;
 
-                    ECPublicKeyParameters pub = (ECPublicKeyParameters)pubKey.GetKey();
+                    ECPublicKeyParameters pub = (ECPublicKeyParameters)akp;
                     ECPoint S = pub.Q.Multiply(ephPriv.D).Normalize();
 
                     key = new KeyParameter(Rfc6637Utilities.CreateKey(pubKey.PublicKeyPacket, S));

crypto/src/openpgp/PgpPublicKey.cs

@@ -11,6 +11,7 @@
 using Org.BouncyCastle.Crypto.Parameters;
 using Org.BouncyCastle.Math;
 using Org.BouncyCastle.Math.EC;
+using Org.BouncyCastle.Math.EC.Rfc7748;
 using Org.BouncyCastle.Math.EC.Rfc8032;
 using Org.BouncyCastle.Security;
 using Org.BouncyCastle.Utilities;
@@ -196,6 +197,19 @@ public PgpPublicKey(
                 ecK.Encode(encodedPoint, 1);
                 bcpgKey = new ECDsaPublicBcpgKey(GnuObjectIdentifiers.Ed25519, new BigInteger(1, encodedPoint));
             }
+            else if (pubKey is X25519PublicKeyParameters)
+            {
+                X25519PublicKeyParameters ecK = (X25519PublicKeyParameters)pubKey;
+                byte[] encodedPoint = new byte[X25519.PointSize + 1];
+                encodedPoint[0] = 0x40;
+                ecK.Encode(encodedPoint, 1);
+                Array.Reverse(encodedPoint, 1, X25519.PointSize);
+                bcpgKey = new ECDHPublicBcpgKey(
+                    MiscObjectIdentifiers.Curve25519,
+                    new BigInteger(1, encodedPoint),
+                    HashAlgorithmTag.Sha256,
+                    SymmetricKeyAlgorithmTag.Aes128);
+            }
             else if (pubKey is ElGamalPublicKeyParameters)
             {
                 ElGamalPublicKeyParameters eK = (ElGamalPublicKeyParameters) pubKey;
@@ -510,7 +524,11 @@ public AsymmetricKeyParameter GetKey()
                         return GetECKey("ECDSA");
                     case PublicKeyAlgorithmTag.ECDH:
                         if (((ECPublicBcpgKey)publicPk.Key).CurveOid.Id.Equals(MiscObjectIdentifiers.Curve25519.Id))
-                            return new X25519PublicKeyParameters(((ECPublicBcpgKey)publicPk.Key).EncodedPoint.ToByteArrayUnsigned(), 0);
+                        {
+                            byte[] encodedPoint = ((ECPublicBcpgKey)publicPk.Key).EncodedPoint.ToByteArrayUnsigned();
+                            Array.Reverse(encodedPoint, 1, X25519.PointSize);
+                            return new X25519PublicKeyParameters(encodedPoint, 1);
+                        }
                         else
                             return GetECKey("ECDH");
                     case PublicKeyAlgorithmTag.EdDsa:

crypto/src/openpgp/PgpPublicKeyEncryptedData.cs

@@ -217,11 +217,10 @@ private byte[] RecoverSessionData(PgpPrivateKey privKey)
 
                 if (privKey.Key is X25519PrivateKeyParameters x25519privKeyParams)
                 {
-                    byte[] sharedKey = new byte[32];
-                    byte[] reversedPrivateKey = new byte[32];
-                    x25519privKeyParams.Encode(reversedPrivateKey, 0);
-                    Array.Reverse((Array)reversedPrivateKey);
-                    X25519.ScalarMult(reversedPrivateKey, 0, pEnc, 1, sharedKey, 0);
+                    byte[] sharedKey = new byte[X25519.PointSize];
+                    byte[] privateKey = new byte[X25519.PointSize];
+                    x25519privKeyParams.Encode(privateKey, 0);
+                    X25519.CalculateAgreement(privateKey, 0, pEnc, 1, sharedKey, 0);
                     key = new KeyParameter(Rfc6637Utilities.CreateKey(privKey.PublicKeyPacket, sharedKey));
                 }
                 else

crypto/src/openpgp/PgpSecretKey.cs

@@ -57,9 +57,23 @@ internal PgpSecretKey(
                     secKey = new DsaSecretBcpgKey(dsK.X);
                     break;
                 case PublicKeyAlgorithmTag.ECDH:
+                    if (privKey.Key is X25519PrivateKeyParameters)
+                    {
+                        X25519PrivateKeyParameters x25519K = (X25519PrivateKeyParameters)privKey.Key;
+                        byte[] secretKey = new byte[X25519PrivateKeyParameters.KeySize];
+                        x25519K.Encode(secretKey, 0);
+                        Array.Reverse(secretKey);
+                        secKey = new ECSecretBcpgKey(new BigInteger(1, secretKey));
+                    }
+                    else
+                    {
+                        ECPrivateKeyParameters ecK = (ECPrivateKeyParameters)privKey.Key;
+                        secKey = new ECSecretBcpgKey(ecK.D);
+                    }
+                    break;
                 case PublicKeyAlgorithmTag.ECDsa:
-                    ECPrivateKeyParameters ecK = (ECPrivateKeyParameters)privKey.Key;
-                    secKey = new ECSecretBcpgKey(ecK.D);
+                    ECPrivateKeyParameters ecdsaK = (ECPrivateKeyParameters)privKey.Key;
+                    secKey = new ECSecretBcpgKey(ecdsaK.D);
                     break;
                 case PublicKeyAlgorithmTag.EdDsa:
                     Ed25519PrivateKeyParameters edK = (Ed25519PrivateKeyParameters)privKey.Key;
@@ -683,7 +697,11 @@ internal PgpPrivateKey DoExtractPrivateKey(byte[] rawPassPhrase, bool clearPassP
                     break;
                 case PublicKeyAlgorithmTag.ECDH:
                     if (((ECPublicBcpgKey)secret.PublicKeyPacket.Key).CurveOid.Id.Equals(MiscObjectIdentifiers.Curve25519.Id))
-                        privateKey = new X25519PrivateKeyParameters(new ECSecretBcpgKey(bcpgIn).X.ToByteArrayUnsigned(), 0);
+                    {
+                        var x = new ECSecretBcpgKey(bcpgIn).X.ToByteArrayUnsigned();
+                        Array.Reverse(x);
+                        privateKey = new X25519PrivateKeyParameters(x, 0);
+                    }
                     else
                         privateKey = GetECKey("ECDH", bcpgIn);
                     break;

crypto/test/src/openpgp/test/PgpECDHTest.cs

@@ -4,13 +4,16 @@
 using System.Text;
 
 using NUnit.Framework;
-
+using Org.BouncyCastle.Asn1;
+using Org.BouncyCastle.Asn1.Gnu;
+using Org.BouncyCastle.Asn1.Misc;
 using Org.BouncyCastle.Asn1.Sec;
 using Org.BouncyCastle.Crypto;
 using Org.BouncyCastle.Crypto.Parameters;
 using Org.BouncyCastle.Security;
 using Org.BouncyCastle.Utilities;
 using Org.BouncyCastle.Utilities.Encoders;
+using Org.BouncyCastle.Utilities.IO;
 using Org.BouncyCastle.Utilities.Test;
 
 namespace Org.BouncyCastle.Bcpg.OpenPgp.Tests
@@ -51,6 +54,40 @@ public class PgpECDHTest
                 "6HiuFH7VKWcxPUBjXwf5+Z3uOKEp28tBgNyDrdbr1BbqlgYzIKq/pe9zUbUXfitn" +
                 "vFc6HcGhvmRQreQ+Yw1x3x0HJeoPwg==");
 
+        private static readonly byte[] testX25519PubKey =
+            Base64.Decode(
+                "mDMEX9XwXhYJKwYBBAHaRw8BAQdAR5ZghmMHL8wldNlOkmbaiAOdyF5V5bgZdKq7" +
+                "L+yb4A20HEVDREggPHRlc3QuZWNkaEBleGFtcGxlLmNvbT6IkAQTFggAOBYhBGoy" +
+                "UrxNv7c3S2JjGzewWiN8tfzXBQJf1fBeAhsDBQsJCAcCBhUKCQgLAgQWAgMBAh4B" +
+                "AheAAAoJEDewWiN8tfzX0ZMA/AhEvrIgu+29eMQeuHOwX1ZY/UssU5TdVROQzGTL" +
+                "n5cgAP9hIKtt/mZ112HiAHDuWk2JskdtsuopnrEccz4PSEkSDLg4BF/V8F4SCisG" +
+                "AQQBl1UBBQEBB0DLPhNt/6GHDbb7vZW/iMsbXTZpgJNQiT6QA/4EzgYQLwMBCAeI" +
+                "eAQYFggAIBYhBGoyUrxNv7c3S2JjGzewWiN8tfzXBQJf1fBeAhsMAAoJEDewWiN8" +
+                "tfzXU34BAKJJLDee+qJCmUI20sMy/YoKfWmMnH2RBBHmLV8FAJ7vAP0e2wGixEfs" +
+                "oPqe8fHmvjQGxSByOyQGn7yD+oq9nVzTAA==");
+
+        private static readonly byte[] testX25519PrivKey =
+            Base64.Decode(
+                "lIYEX9XwXhYJKwYBBAHaRw8BAQdAR5ZghmMHL8wldNlOkmbaiAOdyF5V5bgZdKq7" +
+                "L+yb4A3+BwMCMscozrXr93fOFmtxu/BJjEJrwRl20Jrv9lryfM+SF4UHgVMmJUpJ" +
+                "1RuTbSnM2KaqHwOgmdrvf2FJnpg1vMafBk1CmopqkRzzrbJ6xQhiPrQcRUNESCA8" +
+                "dGVzdC5lY2RoQGV4YW1wbGUuY29tPoiQBBMWCAA4FiEEajJSvE2/tzdLYmMbN7Ba" +
+                "I3y1/NcFAl/V8F4CGwMFCwkIBwIGFQoJCAsCBBYCAwECHgECF4AACgkQN7BaI3y1" +
+                "/NfRkwD8CES+siC77b14xB64c7BfVlj9SyxTlN1VE5DMZMuflyAA/2Egq23+ZnXX" +
+                "YeIAcO5aTYmyR22y6imesRxzPg9ISRIMnIsEX9XwXhIKKwYBBAGXVQEFAQEHQMs+" +
+                "E23/oYcNtvu9lb+IyxtdNmmAk1CJPpAD/gTOBhAvAwEIB/4HAwJ7ShSBrUuUAM5r" +
+                "G4I/gJKo+eBmbNC4NM81eALAF1vcovZPsGsiZ8IgXT64XiC1bpeAoINn6vM4vVbi" +
+                "LqNKqu6ll3ZgQ4po6vCW9GkhuEMmiHgEGBYIACAWIQRqMlK8Tb+3N0tiYxs3sFoj" +
+                "fLX81wUCX9XwXgIbDAAKCRA3sFojfLX811N+AQCiSSw3nvqiQplCNtLDMv2KCn1p" +
+                "jJx9kQQR5i1fBQCe7wD9HtsBosRH7KD6nvHx5r40BsUgcjskBp+8g/qKvZ1c0wA=");
+
+        private static readonly byte[] testX25519Message =
+            Base64.Decode(
+                "hF4DbDc2fNL0VcUSAQdAqdV0v1D4X9cuGrT7+oQBpMFnw1wdfAcxH9xdO00s2HUw" +
+                "qB+XkIRETH7yesynLOKajmYftMWZRyTnW2tJUc1w5NFPjPxcbvd2bYmqkY57uAFg" +
+                "0kcBKhFklH2LRbBNThtQr3jn2YEFbNnhiGfOpoHfCn0oFh5RbXDwm+P3Q3tksvpZ" +
+                "wEGe2VkxLLe7BWnv/sRINQ2YpuaYshe8hw==");
+
         private void Generate()
         {
             SecureRandom random = SecureRandom.GetInstance("SHA1PRNG");
@@ -105,6 +142,71 @@ private void Generate()
             PgpPrivateKey pgpPrivKey = secRing.GetSecretKey().ExtractPrivateKey(passPhrase);
         }
 
+        private void Generate25519()
+        {
+            SecureRandom random = SecureRandom.GetInstance("SHA1PRNG");
+
+            //
+            // Generate a master key
+            //
+            IAsymmetricCipherKeyPairGenerator keyGen = GeneratorUtilities.GetKeyPairGenerator("Ed25519");
+            keyGen.Init(new ECKeyGenerationParameters(GnuObjectIdentifiers.Ed25519, random));
+
+            AsymmetricCipherKeyPair kpSign = keyGen.GenerateKeyPair();
+
+            PgpKeyPair ecdsaKeyPair = new PgpKeyPair(PublicKeyAlgorithmTag.EdDsa, kpSign, DateTime.UtcNow);
+
+            //
+            // Generate an encryption key
+            //
+            keyGen = GeneratorUtilities.GetKeyPairGenerator("X25519");
+            keyGen.Init(new ECKeyGenerationParameters(MiscObjectIdentifiers.Curve25519, random));
+
+            AsymmetricCipherKeyPair kpEnc = keyGen.GenerateKeyPair();
+
+            PgpKeyPair ecdhKeyPair = new PgpKeyPair(PublicKeyAlgorithmTag.ECDH, kpEnc, DateTime.UtcNow);
+
+            //
+            // Generate a key ring
+            //
+            char[] passPhrase = "test".ToCharArray();
+            PgpKeyRingGenerator keyRingGen = new PgpKeyRingGenerator(PgpSignature.PositiveCertification, ecdsaKeyPair,
+                "test@bouncycastle.org", SymmetricKeyAlgorithmTag.Aes256, passPhrase, true, null, null, random);
+            keyRingGen.AddSubKey(ecdhKeyPair);
+
+            PgpPublicKeyRing pubRing = keyRingGen.GeneratePublicKeyRing();
+
+            // TODO: add check of KdfParameters
+            DoBasicKeyRingCheck(pubRing);
+
+            PgpSecretKeyRing secRing = keyRingGen.GenerateSecretKeyRing();
+
+            PgpPublicKeyRing pubRingEnc = new PgpPublicKeyRing(pubRing.GetEncoded());
+            if (!Arrays.AreEqual(pubRing.GetEncoded(), pubRingEnc.GetEncoded()))
+            {
+                Fail("public key ring encoding failed");
+            }
+
+            PgpSecretKeyRing secRingEnc = new PgpSecretKeyRing(secRing.GetEncoded());
+            if (!Arrays.AreEqual(secRing.GetEncoded(), secRingEnc.GetEncoded()))
+            {
+                Fail("secret key ring encoding failed");
+            }
+
+            // Extract back the ECDH key and verify the encoded values to ensure correct endianness
+            PgpSecretKey pgpSecretKey = secRing.GetSecretKey(ecdhKeyPair.KeyId);
+            PgpPrivateKey pgpPrivKey = pgpSecretKey.ExtractPrivateKey(passPhrase);
+
+            if (!Arrays.AreEqual(((X25519PrivateKeyParameters)kpEnc.Private).GetEncoded(), ((X25519PrivateKeyParameters)pgpPrivKey.Key).GetEncoded()))
+            {
+                Fail("private key round trip failed");
+            }
+            if (!Arrays.AreEqual(((X25519PublicKeyParameters)kpEnc.Public).GetEncoded(), ((X25519PublicKeyParameters)pgpSecretKey.PublicKey.GetKey()).GetEncoded()))
+            {
+                Fail("private key round trip failed");
+            }
+        }
+
         private void TestDecrypt(PgpSecretKeyRing secretKeyRing)
         {
             PgpObjectFactory pgpF = new PgpObjectFactory(testMessage);
@@ -134,14 +236,14 @@ private void TestDecrypt(PgpSecretKeyRing secretKeyRing)
     //        }
         }
 
-        private void EncryptDecryptTest()
+        private void EncryptDecryptTest(string algorithm, DerObjectIdentifier curve)
         {
             SecureRandom random = SecureRandom.GetInstance("SHA1PRNG");
 
             byte[] text = Encoding.ASCII.GetBytes("hello world!");
 
-            IAsymmetricCipherKeyPairGenerator keyGen = GeneratorUtilities.GetKeyPairGenerator("ECDH");
-            keyGen.Init(new ECKeyGenerationParameters(SecObjectIdentifiers.SecP256r1, random));
+            IAsymmetricCipherKeyPairGenerator keyGen = GeneratorUtilities.GetKeyPairGenerator(algorithm);
+            keyGen.Init(new ECKeyGenerationParameters(curve, random));
 
             AsymmetricCipherKeyPair kpEnc = keyGen.GenerateKeyPair();
 
@@ -197,6 +299,39 @@ private void EncryptDecryptTest()
             }
         }
 
+        private void GnuPGCrossCheck()
+        {
+            PgpSecretKeyRing secretKeyRing = new PgpSecretKeyRing(testX25519PrivKey);
+
+            PgpObjectFactory pgpF = new PgpObjectFactory(testX25519Message);
+
+            PgpEncryptedDataList encList = (PgpEncryptedDataList)pgpF.NextPgpObject();
+
+            PgpPublicKeyEncryptedData encP = (PgpPublicKeyEncryptedData)encList[0];
+
+            PgpSecretKey secretKey = secretKeyRing.GetSecretKey(0x6c37367cd2f455c5);
+
+            PgpPrivateKey pgpPrivKey = secretKey.ExtractPrivateKey("test".ToCharArray());
+
+            Stream clear = encP.GetDataStream(pgpPrivKey);
+
+            pgpF = new PgpObjectFactory(clear);
+
+            PgpCompressedData c1 = (PgpCompressedData)pgpF.NextPgpObject();
+
+            pgpF = new PgpObjectFactory(c1.GetDataStream());
+
+            PgpLiteralData ld = (PgpLiteralData)pgpF.NextPgpObject();
+
+            Stream inLd = ld.GetDataStream();
+            byte[] bytes = Streams.ReadAll(inLd);
+
+            if (!Arrays.AreEqual(bytes, Encoding.ASCII.GetBytes("hello world!")))
+            {
+                Fail("wrong plain text in decrypted packet");
+            }
+        }
+
         public override void PerformTest()
         {
             //
@@ -213,9 +348,15 @@ public override void PerformTest()
 
             TestDecrypt(secretKeyRing);
 
-            EncryptDecryptTest();
+            EncryptDecryptTest("ECDH", SecObjectIdentifiers.SecP256r1);
+
+            EncryptDecryptTest("X25519", MiscObjectIdentifiers.Curve25519);
+
+            GnuPGCrossCheck();
 
             Generate();
+
+            Generate25519();
         }
 
         private void DoBasicKeyRingCheck(PgpPublicKeyRing pubKeyRing)