On Tue, Apr 03, 2007 at 09:20:36PM +0200, Sback wrote:
> 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
Shouldn't this be caught in signing? It's not much use catching it on
verification: If it won't verify, it's not a valid signature.
>
> 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.
Add an assertion. And maybe change the 256 to the number of bits
required (based on the group) in the new value.
>
>
> 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].
We don't exactly implement FIPS 2, because we use a 256-bit hash.
>
> 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.
By all means fix 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL:
<https://emu.freenetproject.org/pipermail/devl/attachments/20070407/9619e281/attachment.pgp>
