Merge pull request #646 from Kr0emer/fix/bug-002-dsa-domain-params-validation

fix(dsa): reject invalid domain parameters to prevent universal signature forgery

Author: kjur

src/dsa-2.0.js

@@ -47,7 +47,24 @@ KJUR.crypto.DSA = function() {
         _getVbyList = _ASN1HEX.getVbyList,
         _getVbyListEx = _ASN1HEX.getVbyListEx,
 	_isASN1HEX = _ASN1HEX.isASN1HEX,
-	_BigInteger = BigInteger;
+	_BigInteger = BigInteger,
+	_BI_ONE = BigInteger.ONE;
+
+    var _validatePublicArgs = function(p, q, g, y) {
+	if (p == null || q == null || g == null || y == null)
+	    throw new Error("invalid DSA public key");
+
+	// FIPS 186-4 4.7: domain parameters and public key shall be validated.
+	if (_BI_ONE.compareTo(q) >= 0 || q.compareTo(p) >= 0)
+	    throw new Error("invalid DSA public key");
+	if (_BI_ONE.compareTo(g) >= 0 || g.compareTo(p) >= 0)
+	    throw new Error("invalid DSA public key");
+	if (_BI_ONE.compareTo(y) >= 0 || y.compareTo(p) >= 0)
+	    throw new Error("invalid DSA public key");
+	if (g.modPow(q, p).compareTo(_BI_ONE) != 0)
+	    throw new Error("invalid DSA public key");
+    };
+
     this.p = null;
     this.q = null;
     this.g = null;
@@ -120,6 +137,8 @@ KJUR.crypto.DSA = function() {
      * @since jsrsasign 7.0.0 dsa 2.0.0
      */
     this.setPublic = function(p, q, g, y) {
+	_validatePublicArgs(p, q, g, y);
+
 	this.isPublic = true;
 	this.p = p;
 	this.q = q;

test/qunit-do-dsa.html

@@ -129,6 +129,17 @@
   ok(dsa2.verifyWithMessageHash(sHashHex, hSigVal), "");
 });
 
+test("setPublicHex rejects invalid domain parameter g=1", function() {
+  var dsa = new KJUR.crypto.DSA();
+  var f = false;
+  try {
+    dsa.setPublicHex("17", "0b", "01", "01");
+  } catch (ex) {
+    f = true;
+  }
+  ok(f, "invalid dsa public key rejected");
+});
+
 test("signWithMessageHash retries when s is zero", function() {
   var pSmall = new BigInteger("17", 16);
   var qSmall = new BigInteger("0b", 16);

test/qunit-do-x509.html

@@ -611,6 +611,56 @@
 equal(x.verifySignature(pubkey), false, "false");
 });
 
+test("verifySignature DSA forged cert with g=1 shall be rejected", function() {
+var DERSeq = function(arr) { return new KJUR.asn1.DERSequence({array: arr}); };
+var DERSet = function(arr) { return new KJUR.asn1.DERSet({array: arr}); };
+var DERInt = function(v) {
+    if (typeof v == "string") return new KJUR.asn1.DERInteger({hex: v});
+    return new KJUR.asn1.DERInteger({"int": v});
+};
+var DEROid = function(o) { return new KJUR.asn1.DERObjectIdentifier({oid: o}); };
+var DERUtf8 = function(s) { return new KJUR.asn1.DERUTF8String({str: s}); };
+var DERBits = function(hex) { return new KJUR.asn1.DERBitString({hex: "00" + hex}); };
+var DERUtcT = function(s) { return new KJUR.asn1.DERUTCTime({str: s}); };
+var DERTag = function(t, o) {
+    return new KJUR.asn1.DERTaggedObject({tag: t, explicit: true, obj: o});
+};
+var rdnCN = function(cn) { return DERSeq([DERSet([DERSeq([DEROid("2.5.4.3"), DERUtf8(cn)])])]); };
+
+var pHex = "17";
+var qHex = "0b";
+var gHex = "01";
+var yHex = "01";
+
+var dsaParams = DERSeq([DERInt(pHex), DERInt(qHex), DERInt(gHex)]);
+var spki = DERSeq([DERSeq([DEROid("1.2.840.10040.4.1"), dsaParams]),
+		   DERBits(DERInt(yHex).getEncodedHex())]);
+
+var tbs = DERSeq([
+    DERTag("a0", DERInt(2)),
+    DERInt(1),
+    DERSeq([DEROid("1.2.840.10040.4.3")]),
+    rdnCN("Malicious CA"),
+    DERSeq([DERUtcT("250101000000Z"), DERUtcT("351231235959Z")]),
+    rdnCN("Malicious CA"),
+    spki
+]);
+var forgedSigHex = DERSeq([DERInt(1), DERInt(7)]).getEncodedHex();
+var certHex = DERSeq([tbs, DERSeq([DEROid("1.2.840.10040.4.3")]), DERBits(forgedSigHex)]).getEncodedHex();
+var certPEM = KJUR.asn1.ASN1Util.getPEMStringFromHex(certHex, "CERTIFICATE");
+
+var accepted = false;
+try {
+    var x = new X509();
+    x.readCertPEM(certPEM);
+    var pubkey = x.getPublicKey();
+    accepted = x.verifySignature(pubkey);
+} catch (ex) {
+    accepted = false;
+}
+equal(accepted, false, "forged certificate rejected");
+});
+
 test("verifySignature ECDSA k1.self.sha1.cer with k1.pub.p8 VALID", function() {
 var pubkey = KEYUTIL.getKey(K1PUBP8);
 var x = new X509();
@@ -691,4 +741,3 @@
 
 </body>
 </html>
-