BouncyCastle ECDH Key Agreement Fails

I implemented a Elliptic Curve Diffie Hellman cryptography using BouncyCastle API. But, it doesn't seem that the key agreement is working properly. It prints false.

Where did I do wrong? Thank you.

ECParameterSpec ecSpec = ECNamedCurveTable.getParameterSpec("B-571");

    KeyPairGenerator g = KeyPairGenerator.getInstance("ECDH", "BC");

    g.initialize(ecSpec, new SecureRandom());

    KeyPair aKeyPair = g.generateKeyPair();

    KeyAgreement aKeyAgree = KeyAgreement.getInstance("ECDH", "BC");

    aKeyAgree.init(aKeyPair.getPrivate());

     KeyPair bKeyPair = g.generateKeyPair();

    KeyAgreement bKeyAgree = KeyAgreement.getInstance("ECDH", "BC");

    bKeyAgree.init(bKeyPair.getPrivate());

    //
    // agreement
    //
    aKeyAgree.doPhase(bKeyPair.getPublic(), true);
    bKeyAgree.doPhase(aKeyPair.getPublic(), true);

    byte[] aSecret = aKeyAgree.generateSecret();
    byte[] bSecret = bKeyAgree.generateSecret();

    System.out.println(aSecret);
    System.out.println(bSecret);
    if (aSecret.equals(bSecret)){
        return true;
    } else { return false; }

It does work correctly, when I tested with Bouncy Castle 1.49. However, you're comparing using the wrong method.

If you need time-constant comparison, use MessageDigest.isEqual.
If you don't need time-constant comparison, use Arrays.equals.

To print out the contents of the byte arrays, use Arrays.toString:

System.out.println(Arrays.toString(aSecret));
System.out.println(Arrays.toString(bSecret));
return MessageDigest.isEqual(aSecret, bSecret);

Edit: The OP asked me to explain what "time-constant comparison" means, so here goes: a time-constant comparison takes the same amount of time to run, whether or not the two strings match. A non-time-constant comparison will usually take less time to run if the two strings have a mismatch, and the runtime depends on where the mismatch is: the comparison stops when the first mismatch is found.

Whether you need a time-constant comparison or not depends on whether you have a timing oracle. That is, will the length of time the comparison took will give an attacker useful information?

Here's an example of a timing oracle: suppose you're a web server sending a cookie to a browser. You don't want the user to tamper with the cookie, so you attach an HMAC of the cookie's contents:

cookie_to_send = hmac(cookie) + ":" + cookie

Now, when the browser sends the cookie back to you, you recompute the HMAC and see if it matches:

mac, cookie = received_cookie.split(":")
compare(mac, hmac(cookie))

And if the comparison fails (the mac doesn't match the hmac(cookie)), then you reject the request.

In that compare operation above, it is very, very important that this be a constant-time comparison. Otherwise, the attacker can look at how long your server took to reject the request, and use that to deduce what the expected HMAC value is. This is because one component of the comparison (the HMAC value in the cookie, before the first :) is controlled by the attacker, and the attacker can adjust its value byte-by-byte to see how long the rejection takes each time.

In your case, your byte arrays (that you're comparing) are generated from generateSecret(), which suggests to me that it's not attacker-controlled. So, on the surface, it doesn't appear as if a time-constant comparison is necessary. But I'm not a security expert, so I don't know for sure. It's always safe (but can be slow, if the strings being compared are long) to use a time-constant comparison, so if at all unsure, that's what I suggest.