Hi,
working on a class test for DSA.java, as needed for my Google Summer of
Code ranking, I have found two more bugs [another one is already fixed,
after nextgens gave me the repos access] in the current DSA implementation.
It is unlikely that they compare during the normal usage, but since
there are no raised exceptions, I believe they must be fixed.
The first bug is generated by this code [part of DSATest.java, not yet
committed]:
public void testSign_border() {
BigInteger k = BigInteger.ONE;
BigInteger q = Global.DSAgroupBigA.getQ().add(BigInteger.ONE);
BigInteger p = q;
BigInteger g = p.add(BigInteger.ONE);
DSAGroup aDSAgroup = new DSAGroup(p,q,g);
DSAPrivateKey aDSAPrivKey=new DSAPrivateKey(aDSAgroup,randomSource);
DSAPublicKey aDSAPubKey=new DSAPublicKey(aDSAgroup,aDSAPrivKey);
DSASignature aSignature=
DSA.sign(aDSAgroup,aDSAPrivKey,k,aDSAPrivKey.getX(),randomSource);
assertTrue(DSA.verify(aDSAPubKey,aSignature,aDSAPrivKey.getX(),false));
}
As there are only few controls over parameters when creating a
DSAPrivateKey and a DSAPublicKey,
I've been able to generate a weird key pair that behaves not correctly
over the DSA.sign method.
I decided to use values that will create a R value [of the generated
signature] of 1.
R = (g^k mod p) mod q
[ k = 1, q = aPrimeNumber+1, p = q, g = p+1 ]
In this way the verify of the signature return FALSE, even if parameters
are correct.
It depends on the fact that DSA.verify, instead of raising an exception
over bad parameters,
catches it and return false. What is more, when using BigInteger.ZERO as
hased message,
it returns true correctly; but not with BigInteger.TEN... even more
uncertain
The second bug is generated througth this method [in the same test class]:
public void testSign_border2() {
BigInteger q = BigInteger.ONE;
DSAGroup aDSAgroup = new DSAGroup(
Global.DSAgroupBigA.getP(),q,Global.DSAgroupBigA.getG());
DSAPrivateKey aDSAPrivKey=new DSAPrivateKey(aDSAgroup,randomSource);
//not bug correlated part------------------------------------------
DSAPublicKey aDSAPubKey=new DSAPublicKey(aDSAgroup,aDSAPrivKey);
DSASignature aSignature=
DSA.sign(aDSAgroup,aDSAPrivKey,aDSAPrivKey.getX(),randomSource);
assertTrue(DSA.verify(aDSAPubKey,aSignature,aDSAPrivKey.getX(),false));
//not bug correlated part------------------------------------------
}
In this way the problem is on the creation of the DSAPrivateKey, because
it remains
stuck in one creator method of DSAPrivateKey.java:
public DSAPrivateKey(DSAGroup g, Random r) {
BigInteger tempX;
do {
tempX = new NativeBigInteger(256, r);
} while (tempX.compareTo(g.getQ()) > -1);
this.x = tempX;
}
if the Q value it is little enough [as 1 in my test, but I imagine it
could be a problem also for other small values]
it is almost impossible to generate a smaller X value. And it will
remain in the loop.
Both these two bugs are connected to the fact that there is no
parameters-checking when creating
DSA entities. In my opinion it should be enough to check them on the
standards that freenet follows
[that seems to be a mix between FIPS-186-2[0] and the
not-yet-standarized FIPS-182-3[1]].
Then, always IMHO, it could be even better to check on FIPS-186-2 and
raise only warnings
if they don't respect the not-yet-standardized FIPS-186-3 [because the
most part of method works
well with arguments valid for both the standards].
If you think it could be important to fix them and if you want I could
prepare the parameters-checking for
DSA entities and commit them.
Cheers,
Sback
[0] http://csrc.nist.gov/publications/fips/fips186-2/fips186-2-change1.pdf
[1]
http://csrc.nist.gov/publications/drafts/fips_186-3/Draft-FIPS-186-3%20_March2006.pdf
