From bea54d9e469082225cd81ad28d55a0bce75b1402 Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Sun, 6 Jun 2021 20:08:57 +0700 Subject: Improve EdDSA/XDH key validation --- .../crypto/parameters/Ed25519PrivateKeyParameters.cs | 13 +++++++++++++ .../crypto/parameters/Ed25519PublicKeyParameters.cs | 13 +++++++++++++ .../crypto/parameters/Ed448PrivateKeyParameters.cs | 13 +++++++++++++ .../crypto/parameters/Ed448PublicKeyParameters.cs | 13 +++++++++++++ .../crypto/parameters/X25519PrivateKeyParameters.cs | 13 +++++++++++++ .../crypto/parameters/X25519PublicKeyParameters.cs | 13 +++++++++++++ .../crypto/parameters/X448PrivateKeyParameters.cs | 13 +++++++++++++ .../src/crypto/parameters/X448PublicKeyParameters.cs | 13 +++++++++++++ crypto/src/security/PrivateKeyFactory.cs | 20 ++++++++------------ crypto/src/security/PublicKeyFactory.cs | 16 ++++++---------- crypto/test/src/crypto/test/Ed25519Test.cs | 4 ++-- crypto/test/src/crypto/test/Ed448Test.cs | 4 ++-- 12 files changed, 122 insertions(+), 26 deletions(-) diff --git a/crypto/src/crypto/parameters/Ed25519PrivateKeyParameters.cs b/crypto/src/crypto/parameters/Ed25519PrivateKeyParameters.cs index 531bca0e9..4e61a0f8d 100644 --- a/crypto/src/crypto/parameters/Ed25519PrivateKeyParameters.cs +++ b/crypto/src/crypto/parameters/Ed25519PrivateKeyParameters.cs @@ -24,6 +24,11 @@ namespace Org.BouncyCastle.Crypto.Parameters Ed25519.GeneratePrivateKey(random, data); } + public Ed25519PrivateKeyParameters(byte[] buf) + : this(Validate(buf), 0) + { + } + public Ed25519PrivateKeyParameters(byte[] buf, int off) : base(true) { @@ -106,5 +111,13 @@ namespace Org.BouncyCastle.Crypto.Parameters } } } + + private static byte[] Validate(byte[] buf) + { + if (buf.Length != KeySize) + throw new ArgumentException("must have length " + KeySize, "buf"); + + return buf; + } } } diff --git a/crypto/src/crypto/parameters/Ed25519PublicKeyParameters.cs b/crypto/src/crypto/parameters/Ed25519PublicKeyParameters.cs index 96e9ec21f..8a9139e8d 100644 --- a/crypto/src/crypto/parameters/Ed25519PublicKeyParameters.cs +++ b/crypto/src/crypto/parameters/Ed25519PublicKeyParameters.cs @@ -14,6 +14,11 @@ namespace Org.BouncyCastle.Crypto.Parameters private readonly byte[] data = new byte[KeySize]; + public Ed25519PublicKeyParameters(byte[] buf) + : this(Validate(buf), 0) + { + } + public Ed25519PublicKeyParameters(byte[] buf, int off) : base(false) { @@ -36,5 +41,13 @@ namespace Org.BouncyCastle.Crypto.Parameters { return Arrays.Clone(data); } + + private static byte[] Validate(byte[] buf) + { + if (buf.Length != KeySize) + throw new ArgumentException("must have length " + KeySize, "buf"); + + return buf; + } } } diff --git a/crypto/src/crypto/parameters/Ed448PrivateKeyParameters.cs b/crypto/src/crypto/parameters/Ed448PrivateKeyParameters.cs index 1b38143fa..705ad8c53 100644 --- a/crypto/src/crypto/parameters/Ed448PrivateKeyParameters.cs +++ b/crypto/src/crypto/parameters/Ed448PrivateKeyParameters.cs @@ -24,6 +24,11 @@ namespace Org.BouncyCastle.Crypto.Parameters Ed448.GeneratePrivateKey(random, data); } + public Ed448PrivateKeyParameters(byte[] buf) + : this(Validate(buf), 0) + { + } + public Ed448PrivateKeyParameters(byte[] buf, int off) : base(true) { @@ -98,5 +103,13 @@ namespace Org.BouncyCastle.Crypto.Parameters } } } + + private static byte[] Validate(byte[] buf) + { + if (buf.Length != KeySize) + throw new ArgumentException("must have length " + KeySize, "buf"); + + return buf; + } } } diff --git a/crypto/src/crypto/parameters/Ed448PublicKeyParameters.cs b/crypto/src/crypto/parameters/Ed448PublicKeyParameters.cs index d7faac246..8a89be08f 100644 --- a/crypto/src/crypto/parameters/Ed448PublicKeyParameters.cs +++ b/crypto/src/crypto/parameters/Ed448PublicKeyParameters.cs @@ -14,6 +14,11 @@ namespace Org.BouncyCastle.Crypto.Parameters private readonly byte[] data = new byte[KeySize]; + public Ed448PublicKeyParameters(byte[] buf) + : this(Validate(buf), 0) + { + } + public Ed448PublicKeyParameters(byte[] buf, int off) : base(false) { @@ -36,5 +41,13 @@ namespace Org.BouncyCastle.Crypto.Parameters { return Arrays.Clone(data); } + + private static byte[] Validate(byte[] buf) + { + if (buf.Length != KeySize) + throw new ArgumentException("must have length " + KeySize, "buf"); + + return buf; + } } } diff --git a/crypto/src/crypto/parameters/X25519PrivateKeyParameters.cs b/crypto/src/crypto/parameters/X25519PrivateKeyParameters.cs index c9aaad949..63e72d3a7 100644 --- a/crypto/src/crypto/parameters/X25519PrivateKeyParameters.cs +++ b/crypto/src/crypto/parameters/X25519PrivateKeyParameters.cs @@ -22,6 +22,11 @@ namespace Org.BouncyCastle.Crypto.Parameters X25519.GeneratePrivateKey(random, data); } + public X25519PrivateKeyParameters(byte[] buf) + : this(Validate(buf), 0) + { + } + public X25519PrivateKeyParameters(byte[] buf, int off) : base(true) { @@ -59,5 +64,13 @@ namespace Org.BouncyCastle.Crypto.Parameters if (!X25519.CalculateAgreement(data, 0, encoded, 0, buf, off)) throw new InvalidOperationException("X25519 agreement failed"); } + + private static byte[] Validate(byte[] buf) + { + if (buf.Length != KeySize) + throw new ArgumentException("must have length " + KeySize, "buf"); + + return buf; + } } } diff --git a/crypto/src/crypto/parameters/X25519PublicKeyParameters.cs b/crypto/src/crypto/parameters/X25519PublicKeyParameters.cs index 7df5f624d..c28e4de16 100644 --- a/crypto/src/crypto/parameters/X25519PublicKeyParameters.cs +++ b/crypto/src/crypto/parameters/X25519PublicKeyParameters.cs @@ -14,6 +14,11 @@ namespace Org.BouncyCastle.Crypto.Parameters private readonly byte[] data = new byte[KeySize]; + public X25519PublicKeyParameters(byte[] buf) + : this(Validate(buf), 0) + { + } + public X25519PublicKeyParameters(byte[] buf, int off) : base(false) { @@ -36,5 +41,13 @@ namespace Org.BouncyCastle.Crypto.Parameters { return Arrays.Clone(data); } + + private static byte[] Validate(byte[] buf) + { + if (buf.Length != KeySize) + throw new ArgumentException("must have length " + KeySize, "buf"); + + return buf; + } } } diff --git a/crypto/src/crypto/parameters/X448PrivateKeyParameters.cs b/crypto/src/crypto/parameters/X448PrivateKeyParameters.cs index 35eb1c4f7..d10a9035f 100644 --- a/crypto/src/crypto/parameters/X448PrivateKeyParameters.cs +++ b/crypto/src/crypto/parameters/X448PrivateKeyParameters.cs @@ -22,6 +22,11 @@ namespace Org.BouncyCastle.Crypto.Parameters X448.GeneratePrivateKey(random, data); } + public X448PrivateKeyParameters(byte[] buf) + : this(Validate(buf), 0) + { + } + public X448PrivateKeyParameters(byte[] buf, int off) : base(true) { @@ -59,5 +64,13 @@ namespace Org.BouncyCastle.Crypto.Parameters if (!X448.CalculateAgreement(data, 0, encoded, 0, buf, off)) throw new InvalidOperationException("X448 agreement failed"); } + + private static byte[] Validate(byte[] buf) + { + if (buf.Length != KeySize) + throw new ArgumentException("must have length " + KeySize, "buf"); + + return buf; + } } } diff --git a/crypto/src/crypto/parameters/X448PublicKeyParameters.cs b/crypto/src/crypto/parameters/X448PublicKeyParameters.cs index 6c566ddf2..704d2f7c1 100644 --- a/crypto/src/crypto/parameters/X448PublicKeyParameters.cs +++ b/crypto/src/crypto/parameters/X448PublicKeyParameters.cs @@ -14,6 +14,11 @@ namespace Org.BouncyCastle.Crypto.Parameters private readonly byte[] data = new byte[KeySize]; + public X448PublicKeyParameters(byte[] buf) + : this(Validate(buf), 0) + { + } + public X448PublicKeyParameters(byte[] buf, int off) : base(false) { @@ -36,5 +41,13 @@ namespace Org.BouncyCastle.Crypto.Parameters { return Arrays.Clone(data); } + + private static byte[] Validate(byte[] buf) + { + if (buf.Length != KeySize) + throw new ArgumentException("must have length " + KeySize, "buf"); + + return buf; + } } } diff --git a/crypto/src/security/PrivateKeyFactory.cs b/crypto/src/security/PrivateKeyFactory.cs index dfc73c2cd..408c8b6a0 100644 --- a/crypto/src/security/PrivateKeyFactory.cs +++ b/crypto/src/security/PrivateKeyFactory.cs @@ -174,26 +174,26 @@ namespace Org.BouncyCastle.Security } else if (algOid.Equals(EdECObjectIdentifiers.id_X25519)) { - return new X25519PrivateKeyParameters(GetRawKey(keyInfo, X25519PrivateKeyParameters.KeySize), 0); + return new X25519PrivateKeyParameters(GetRawKey(keyInfo)); } else if (algOid.Equals(EdECObjectIdentifiers.id_X448)) { - return new X448PrivateKeyParameters(GetRawKey(keyInfo, X448PrivateKeyParameters.KeySize), 0); + return new X448PrivateKeyParameters(GetRawKey(keyInfo)); } else if (algOid.Equals(EdECObjectIdentifiers.id_Ed25519)) { - return new Ed25519PrivateKeyParameters(GetRawKey(keyInfo, Ed25519PrivateKeyParameters.KeySize), 0); + return new Ed25519PrivateKeyParameters(GetRawKey(keyInfo)); } else if (algOid.Equals(EdECObjectIdentifiers.id_Ed448)) { - return new Ed448PrivateKeyParameters(GetRawKey(keyInfo, Ed448PrivateKeyParameters.KeySize), 0); + return new Ed448PrivateKeyParameters(GetRawKey(keyInfo)); } else if (algOid.Equals(RosstandartObjectIdentifiers.id_tc26_gost_3410_12_512) || algOid.Equals(RosstandartObjectIdentifiers.id_tc26_gost_3410_12_256)) { Gost3410PublicKeyAlgParameters gostParams = Gost3410PublicKeyAlgParameters.GetInstance(keyInfo.PrivateKeyAlgorithm.Parameters); - ECGost3410Parameters ecSpec = null; - BigInteger d = null; + ECGost3410Parameters ecSpec; + BigInteger d; Asn1Object p = keyInfo.PrivateKeyAlgorithm.Parameters.ToAsn1Object(); if (p is Asn1Sequence && (Asn1Sequence.GetInstance(p).Count == 2 || Asn1Sequence.GetInstance(p).Count == 3)) { @@ -280,13 +280,9 @@ namespace Org.BouncyCastle.Security } } - private static byte[] GetRawKey(PrivateKeyInfo keyInfo, int expectedSize) + private static byte[] GetRawKey(PrivateKeyInfo keyInfo) { - byte[] result = Asn1OctetString.GetInstance(keyInfo.ParsePrivateKey()).GetOctets(); - if (expectedSize != result.Length) - throw new SecurityUtilityException("private key encoding has incorrect length"); - - return result; + return Asn1OctetString.GetInstance(keyInfo.ParsePrivateKey()).GetOctets(); } public static AsymmetricKeyParameter DecryptKey( diff --git a/crypto/src/security/PublicKeyFactory.cs b/crypto/src/security/PublicKeyFactory.cs index 15af90f91..10b2aacdc 100644 --- a/crypto/src/security/PublicKeyFactory.cs +++ b/crypto/src/security/PublicKeyFactory.cs @@ -217,19 +217,19 @@ namespace Org.BouncyCastle.Security } else if (algOid.Equals(EdECObjectIdentifiers.id_X25519)) { - return new X25519PublicKeyParameters(GetRawKey(keyInfo, X25519PublicKeyParameters.KeySize), 0); + return new X25519PublicKeyParameters(GetRawKey(keyInfo)); } else if (algOid.Equals(EdECObjectIdentifiers.id_X448)) { - return new X448PublicKeyParameters(GetRawKey(keyInfo, X448PublicKeyParameters.KeySize), 0); + return new X448PublicKeyParameters(GetRawKey(keyInfo)); } else if (algOid.Equals(EdECObjectIdentifiers.id_Ed25519)) { - return new Ed25519PublicKeyParameters(GetRawKey(keyInfo, Ed25519PublicKeyParameters.KeySize), 0); + return new Ed25519PublicKeyParameters(GetRawKey(keyInfo)); } else if (algOid.Equals(EdECObjectIdentifiers.id_Ed448)) { - return new Ed448PublicKeyParameters(GetRawKey(keyInfo, Ed448PublicKeyParameters.KeySize), 0); + return new Ed448PublicKeyParameters(GetRawKey(keyInfo)); } else if (algOid.Equals(RosstandartObjectIdentifiers.id_tc26_gost_3410_12_256) || algOid.Equals(RosstandartObjectIdentifiers.id_tc26_gost_3410_12_512)) @@ -282,17 +282,13 @@ namespace Org.BouncyCastle.Security } } - private static byte[] GetRawKey(SubjectPublicKeyInfo keyInfo, int expectedSize) + private static byte[] GetRawKey(SubjectPublicKeyInfo keyInfo) { /* * TODO[RFC 8422] * - Require keyInfo.Algorithm.Parameters == null? */ - byte[] result = keyInfo.PublicKeyData.GetOctets(); - if (expectedSize != result.Length) - throw new SecurityUtilityException("public key encoding has incorrect length"); - - return result; + return keyInfo.PublicKeyData.GetOctets(); } private static bool IsPkcsDHParam(Asn1Sequence seq) diff --git a/crypto/test/src/crypto/test/Ed25519Test.cs b/crypto/test/src/crypto/test/Ed25519Test.cs index 516574bc3..d771a8c01 100644 --- a/crypto/test/src/crypto/test/Ed25519Test.cs +++ b/crypto/test/src/crypto/test/Ed25519Test.cs @@ -54,9 +54,9 @@ namespace Org.BouncyCastle.Crypto.Tests private void BasicSigTest() { Ed25519PrivateKeyParameters privateKey = new Ed25519PrivateKeyParameters( - Hex.DecodeStrict("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60"), 0); + Hex.DecodeStrict("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60")); Ed25519PublicKeyParameters publicKey = new Ed25519PublicKeyParameters( - Hex.DecodeStrict("d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a"), 0); + Hex.DecodeStrict("d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a")); byte[] sig = Hex.Decode("e5564300c360ac729086e2cc806e828a84877f1eb8e5d974d873e065224901555fb8821590a33bacc61e39701cf9b46bd25bf5f0595bbe24655141438e7a100b"); diff --git a/crypto/test/src/crypto/test/Ed448Test.cs b/crypto/test/src/crypto/test/Ed448Test.cs index 114a31714..bfd9d09f9 100644 --- a/crypto/test/src/crypto/test/Ed448Test.cs +++ b/crypto/test/src/crypto/test/Ed448Test.cs @@ -56,12 +56,12 @@ namespace Org.BouncyCastle.Crypto.Tests "6c82a562cb808d10d632be89c8513ebf" + "6c929f34ddfa8c9f63c9960ef6e348a3" + "528c8a3fcc2f044e39a3fc5b94492f8f" + - "032e7549a20098f95b"), 0); + "032e7549a20098f95b")); Ed448PublicKeyParameters publicKey = new Ed448PublicKeyParameters( Hex.DecodeStrict("5fd7449b59b461fd2ce787ec616ad46a" + "1da1342485a70e1f8a0ea75d80e96778" + "edf124769b46c7061bd6783df1e50f6c" + - "d1fa1abeafe8256180"), 0); + "d1fa1abeafe8256180")); byte[] sig = Hex.DecodeStrict("533a37f6bbe457251f023c0d88f976ae" + "2dfb504a843e34d2074fd823d41a591f" + -- cgit 1.4.1