Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto_sign and crypto_sign_detached throwing invalid mem access (windows) #1

Open
bbyrd74 opened this issue Oct 4, 2017 · 7 comments

Comments

@bbyrd74
Copy link
Contributor

bbyrd74 commented Oct 4, 2017

Not sure why these wont work (the 2 verify ones also fail with same error). I do already have many of your libsodium-jna functions working normally. I have also implemented a few new ones for you and have recently pushed them to you for merge.

But these ones just seem to fail. (using eclipse neon 64 and jre 1.8.0_144

muquit added a commit that referenced this issue Oct 4, 2017
@muquit
Copy link
Owner

muquit commented Oct 4, 2017

I added another unit test testCryptoSecretBoxDetached2() which also passes on 64 bit Mac and Windows with java 1.8. Are you using the correct number bytes in key and nonce?

By the way, I did not get any pull request.

Thanks.

@bbyrd74
Copy link
Contributor Author

bbyrd74 commented Oct 5, 2017

Hi - sorry I must have messed up the merge. But I am not using secretboxdetached (which is authentication). I amd trying public key signatures.

But here are the 4 new functions that I do have that do work fine:

`
// definitions for the upper part of SodiumLibrary

    // Signing/Signed keys
    long crypto_sign_secretkeybytes();
    long crypto_sign_publickeybytes();
    int crypto_sign_keypair(byte[] pk, byte[] sk);
    
    // libsodium's generichash (blake2b), this function will only return outlen number of bytes
    // where you can specify outlen.  the key can be null and keylen can be 0
    int crypto_generichash(byte[] out, int outlen,
    		byte[] in, int inlen,
    		byte[] key, long keylen);



    // 2 actual function implementations that implement the 4 upper defintions, these 2 go into the
    // lower part of SodiumLibrary

 //implementation of Sodium's public key / secret key signing.
// a SodiumKeyPair is created and returned containing the secret signing key and the public key
// that has been signed by the private key. the secret key (64 bytes) is simply
// the 32 byte random seed that Sodium generated + the 32byte public key that was signed by the private key
public static SodiumKeyPair cryptoSignKeyPair() throws SodiumLibraryException
{
    SodiumKeyPair kp = new SodiumKeyPair();
    byte[] publicKey = new byte[(int) sodium().crypto_sign_publickeybytes()];
    byte[] privateKey = new byte[(int) sodium().crypto_sign_secretkeybytes()];
    int rc = sodium().crypto_sign_keypair(publicKey, privateKey);
    if (rc != 0)
    {
        throw new SodiumLibraryException("libsodium crypto_sign_keypair() failed, returned " + rc + ", expected 0");
    }
    kp.setPublicKey(publicKey);
    kp.setPrivateKey(privateKey);
    logger.info("pk len: " + publicKey.length);
    logger.info("sk len: " + privateKey.length);
    return kp;
}

// libsodium's generichash (blake2b), this function will only return length number of bytes of the hash
// this implementation does not expect key/key.length which blake does, but we are setting them to null and 0
public static byte[] cryptoGenerichash(byte[] input, int length) throws SodiumLibraryException
{
    byte[] hash = new byte[length];
    int rc = sodium().crypto_generichash(hash, length, input, input.length, null, 0);
    if (rc != 0)
    {
        throw new SodiumLibraryException("libsodium crypto_generichash failed, returned " + rc + ", expected 0");
    }
    return hash;
}
    `

So these 2 work great, now, on my 64bit windows with latest libsodium, and eclipse64bitneon, and java 64bit 1.8.0_144. Notice that we are actually successful at creating the signing private key and the signed public key, we just cannot implement any signing operations - the issue is for me also trying to add support for function crypto_sign_detached. here is what I am trying to implement

        // actual signing and verification operations of the Signing key
        int crypto_sign_detached(byte[] sig, long siglen_p,
        					byte[]  m, long mlen,
        					byte[] sk);


    public static byte[] cryptoSignDetached(byte[] msg, byte[] sk) throws SodiumLibraryException
    {
        byte[] sig = new byte[64];
        int rc = sodium().crypto_sign_detached(sig, sig.length, msg, (long) msg.length, sk);
        if (rc != 0)
        {
            throw new SodiumLibraryException("libsodium crypto_sign_detached failed, returned " + rc + ", expected 0");
        }
        return sig;
    }

but eclipse errors out with

Exception in thread "main" java.lang.Error: Invalid memory access
at com.sun.jna.Native.invokeInt(Native Method)
at com.sun.jna.Function.invoke(Function.java:390)
at com.sun.jna.Function.invoke(Function.java:323)
at com.sun.jna.Library$Handler.invoke(Library.java:236)
at com.sun.proxy.$Proxy0.crypto_sign_detached(Unknown Source)
at com.muquit.libsodiumjna.SodiumLibrary.cryptoSignDetached(SodiumLibrary.java:286)
at tezzigator.LibsodiumTest.main(LibsodiumTest.java:94)

Id be more than happy to work with you to troubleshoot this. (I really need to get signatures to work, and kalium is such a massive pain with its massive dependencies)

@muquit
Copy link
Owner

muquit commented Oct 5, 2017

It appears you did not generate the key pair, you just created bunch of bytes. SodiumKeypair is just a place holder, it does not generate pair. Generate the pair by calling cryptoBoxKeyPair(), it will work.

Thanks.

@bbyrd74
Copy link
Contributor Author

bbyrd74 commented Oct 5, 2017

cryptoBoxKeyPair is for public key authentication encryption. I am working on public key signatures here - not auth/encryption.

but i figured out the issue; it was something non-intuitive inside libsodium itself for the sign/open pieces whereby instead of passing the actual length of the resulting message you want back, you must just pass a zero instead of the length; so whatever I guess. seems odd. its all working now, so now I have 4 very nice functions for you now - if you'd like to help me out with the proper way to merge? I thought did it the other day, and from all indications it looks like it completed, but ive just never done this before.

btw thanks for this excellent library, its been extremely helpful for what I am tryign to achieve here.

@muquit
Copy link
Owner

muquit commented Oct 6, 2017

That's great! For your new code, you can try again to send a pull request. Otherwise if possible, you can do the following:

  • Document the API in README.md
  • Javadoc comments in the source
  • Add Unit tests for the API
    Then zip the files up and email me, I will merge.

Thanks.

@bbyrd74
Copy link
Contributor Author

bbyrd74 commented Nov 1, 2017 via email

muquit pushed a commit that referenced this issue Nov 11, 2017
Update SodiumLibrary.java for support for signing, blake2b generic ha…
@muquit
Copy link
Owner

muquit commented Nov 12, 2017

You pull request is merged. If possible, please update the README.md about the APIs you added.
Also it would be great if you add unit tests for the API.
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants