bcgit / bc-java

Bouncy Castle Java Distribution (Mirror)
https://www.bouncycastle.org/java.html
MIT License
2.31k stars 1.14k forks source link

Invalid XMSS/XMSSMT signature after updating and storing privateKey #446

Closed whme closed 5 years ago

whme commented 5 years ago

Hi, There seems to be a problem with XMSS/XMSSMT privateKeys serialization.

Regardless with which digestAlg and height/layers used, one privateKey used like: load -> sign -> update -> save, produces invalid signatures after the second signing. (However it seems to be especially consistent for SHA512withXMSSMT height = 4, layers = 2) Maybe this goes in the same direction as this.

This happens with the 160 version as well as with the 161b20 beta version.

bcgit commented 5 years ago

Can you provide an example for this? Thanks.

whme commented 5 years ago
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.bouncycastle.pqc.jcajce.interfaces.StateAwareSignature;
import org.bouncycastle.pqc.jcajce.provider.BouncyCastlePQCProvider;
import org.bouncycastle.pqc.jcajce.spec.XMSSMTParameterSpec;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.security.*;
import java.security.spec.PKCS8EncodedKeySpec;

/*

Seems to be a serialization problem of the key
as saving the key to a store, a file or simply
cloning it does break it. (saveToFile=true)
Where as keeping it in memory does work fine.

*/

public class IssueExample {
    public static PrivateKey privateKey;
    public static PublicKey publicKey;
    public static StateAwareSignature signer, verifier;
    public static byte[] signature;

    public static String digest, sigAlg, payload, keyPath;
    public static int height, layers;

    public static boolean saveToFile;

    public static void main(String[] args) throws Exception{
        Security.addProvider(new BouncyCastlePQCProvider());
        keyPath = "privateKey.sk";

        height = 4;
        layers = 2;
        digest = "SHA512";
        sigAlg = digest+"withXMSSMT";
        payload = "Hello World";

        saveToFile = false; //set true to see issue

        generateKeyPair();
        for (int i: new int[4])run();
    }

    public static void run() throws Exception{
        sign();
        updateKey();
        verify();
    }

    public static void sign() throws Exception{
        signer = (StateAwareSignature) Signature.getInstance(sigAlg, "BCPQC");
        signer.initSign(privateKey);
        signer.update(payload.getBytes());
        signature = signer.sign();
        print("Signed");
    }

    public static void verify()throws Exception{
        verifier = (StateAwareSignature) Signature.getInstance(sigAlg, "BCPQC");
        verifier.initVerify(publicKey);
        verifier.update(payload.getBytes());
        print("Signature valid: " + verifier.verify(signature));
    }

    public static void updateKey() throws Exception{
        if (saveToFile) {
            saveKeyToFile(signer.getUpdatedPrivateKey());
            privateKey = loadKeyFromFile();
        }else privateKey = signer.getUpdatedPrivateKey();
    }

    public static void generateKeyPair() throws Exception{
        KeyPairGenerator kpg = KeyPairGenerator.getInstance("XMSSMT", "BCPQC");
        kpg.initialize(new XMSSMTParameterSpec(height, layers, digest));
        KeyPair keyPair = kpg.generateKeyPair();
        if (saveToFile) {
            saveKeyToFile(keyPair.getPrivate());
            privateKey = loadKeyFromFile();
        }else privateKey = keyPair.getPrivate();
        publicKey = keyPair.getPublic();
    }

    private static void saveKeyToFile(PrivateKey _key) throws Exception{
//        print("Saving key to file");
        File pk = new File(keyPath);
        if (pk.exists())pk.delete();
        try (FileOutputStream fos = new FileOutputStream(pk)) {
            fos.write(_key.getEncoded());
        }
    }

    public static PrivateKey loadKeyFromFile() throws Exception{
//        print("Reading private key from file");
        File pk = new File(keyPath);
        KeyFactory kf = KeyFactory.getInstance("XMSSMT", "BCPQC");
        try (FileInputStream fis = new FileInputStream(pk)) {
            return kf.generatePrivate(new PKCS8EncodedKeySpec(fis.readAllBytes()));
        }
    }

    public static void print(String msg){
        System.out.println(msg);
    }
}

Saving the Key to a KeyStore or cloning it using SerializationUtils.clone() does result in the same behavior. However the Key might be stored without getting broken after generation and before any usage. Which leads me to the conclusion there must be a problem with the State of the Key during serialization which leads to it signing stuff wrong.

bcgit commented 5 years ago

I think I've found it. It was not fun... Try 161b22, it is now up on https://www.bouncycastle.org/betas Let us know how you go. Thanks.

whme commented 5 years ago

Thank you very much for your effort, it works perfectly fine now! Would it be possible if one could get the keystate, like privateKey.getState() returns the number of remaining signatures?

bcgit commented 5 years ago

And thank you for your help. Glad to hear it's working.

bcgit commented 5 years ago

I'll look into the getState() (getUsagesRemaining()?) method. I think we can add something to do that, have to investigate first though.

whme commented 5 years ago

Perfect thank you very much! Name it whatever you like, the interesting variable actually is : privateKey -> keyparams -> index, but I guess getUsagesRemaining() is way better than getIndex() or getState()!

bcgit commented 5 years ago

Latest beta (161b23) now includes a getRemainingUsages() on the keys and StateAwareSignature.isSigningCapable() now returns false if the key is no longer usable as well.

whme commented 5 years ago

Thank you very much!