search

kaikramer / keystore-explorer

KeyStore Explorer is a free GUI replacement for the Java command-line utilities keytool and jarsigner.
https://keystore-explorer.org/
GNU General Public License v3.0
1.7k stars 275 forks source link

Import CA Reply always fails for EC keys on KSE 5.5.0 #331

Closed izstas closed 2 years ago

izstas commented 2 years ago

Describe the bug Import CA Reply always fails for EC keys on KSE 5.5.0.

Below are the simplest steps to reproduce I could find, of course, the bug also occurs when you're importing an actual new certificate instead of the one you just exported.

To Reproduce

  1. Open KSE.
  2. Create a new PKCS12 keystore.
  3. Generate an EC prime256v1 key pair (with a self-signed certificate) with any name, alias, etc.
  4. Right click the created entry and choose Export -> Export Certificate Chain, export as X.509 PEM.
  5. Right click the created entry and choose Import CA Reply -> From File and choose the file created in step 4.

Actual behavior The import fails with "The public key of the CA Reply does not match the public key of the key pair entry".

Expected behavior The import succeeds.

Environment

jgrateron commented 2 years ago

It worked using this method, will it be correct? @kaikramer

image

kaikramer commented 2 years ago

@jgrateron Yes, that fixes the issue.

The actual problem is that one PublicKey object was created by BouncyCastle and is of type BCECPublicKey and the other one was created by the Java runtime and is of type ECPublicKeyImpl. And the equals method of BCECPublicKey is not correctly implemented:

public boolean equals(Object o)
{
    if (!(o instanceof BCECPublicKey))
    {
        return false;
    }

    BCECPublicKey other = (BCECPublicKey)o;

    return ecPublicKey.getQ().equals(other.ecPublicKey.getQ()) && (engineGetSpec().equals(other.engineGetSpec()));
}

It should check for instanceof ECPublicKey instead. They have done it right in BCRSAPublicKey, that is why this error only happens with EC keys.

I'll report this bug to the Bouncy Castle project.

kaikramer commented 2 years ago

And BTW just changing the order would fix the problem as well: if (!exitingEntryCerts[0].getPublicKey().equals(certs[0].getPublicKey())) { -> if (!certs[0].getPublicKey().equals(exitingEntryCerts[0].getPublicKey())) {

Because then the equals method of ECPublicKeyImpl would be used, which is implemented correctly.

jgrateron commented 2 years ago

Perfect much better, you have to do it equally in ImportCaReplyFromClipboardAction

kaikramer commented 2 years ago

Yep, same issue in ImportCaReplyFromClipboardAction and there might be other code locations with similar problems. I would really prefer this to be fixed in BC.

I have created a ticket: https://github.com/bcgit/bc-java/issues/1083