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.privateDecrypt - padding issue #9588

Closed
rensrongen opened this issue Nov 12, 2016 · 10 comments
Closed

crypto.privateDecrypt - padding issue #9588

rensrongen opened this issue Nov 12, 2016 · 10 comments
Labels
crypto Issues and PRs related to the crypto subsystem.

Comments

@rensrongen
Copy link

rensrongen commented Nov 12, 2016

  • Version: v7+
  • Platform: Win10x64
  • Subsystem: crypto

crypto.privateDecrypt does not accept RSA_PKCS1_OAEP_PADDING and RSA_PKCS1_PADDING option.

Using either of them produces the following error:

Error: error:0408F090:rsa routines:PKEY_RSA_CTRL:illegal or unsupported padding mode

It does accept RSA_NO_PADDING, but when I decrypt an sha256 signature, it will produce a 256 character string, the final 64 characters contained the actual hash.

For example, I have to do the following to successfully decrypt and read the sha256 signature:

var signature = Crypto.publicDecrypt(
            {
                'key': SessionManager.PublicKey, //buffer
                'padding': Constants.RSA_NO_PADDING
            },
            Buffer.from(ciphertext, 'hex')
        ).toString().substr(192);

Everything works as expected when using privateDecrypt.

@mscdex mscdex added the crypto Issues and PRs related to the crypto subsystem. label Nov 12, 2016
@mscdex
Copy link
Contributor

mscdex commented Nov 12, 2016

Can you provide a complete, simple, standalone example that reproduces the issue?

@shigeki
Copy link
Contributor

shigeki commented Nov 13, 2016

I also would like you to confirm if you really set RSA_PKCS1_PADDING notRSA_PKCS1_PSS_PADDING in case of the error of illegal or unsupported padding mode.
There is not a code path to show the unsupported padding mode error in using RSA_PKCS1_PADDING as https://github.com/nodejs/node/blob/master/deps/openssl/openssl/crypto/rsa/rsa_pmeth.c#L499-L508 .
And the same error would be shown in the case when RSA_PKCS1_OAEP_PADDING is used in sign()/verity(). We can clarify the issue after you provide a complete example.

@rensrongen
Copy link
Author

Alright, I'll look into this later today when I have time.

@rensrongen
Copy link
Author

Example:

var crypto = require('crypto');
var path = require('path');
var constants = require('constants');
var fs = require('fs');

var privateEncryptString = function(str, key) {
    var absolutePath = path.resolve(key);
    var privateKey = fs.readFileSync(absolutePath, 'utf8');
    var encrypted = crypto.privateEncrypt(
    {
        key: privateKey,
        padding: constants.RSA_NO_PADDING 
    }, new Buffer(str));
    return encrypted.toString('base64');
};

var hash = '122e4e90223ff56eb3490debebc64a17502e9025deb0ab56dcd2e4c67ef583f2';
var encypted = privateEncryptString(hash, './privatekey');
console.log(encypted);

Running this produces the following error:

Error: error:0406B07A:rsa routines:RSA_padding_add_none:data too small for key size

Providing RSA_PKCS1_PADDING works as expected, RSA_PKCS1_OAEP_PADDING doesn't work either. (PKEY_RSA_CTRL:illegal or unsupported padding mode)

@mscdex
Copy link
Contributor

mscdex commented Nov 14, 2016

A couple of things to note here:

  • If you're using no padding, then you need to manually pad the input yourself so that it matches the key size. For example, if your key is 1024 bits, you need to make sure the input is 128 bytes. In your example, you could do this by simply adding .repeat(2) to the end of your hash declaration, since it is 64 bytes long already. Generally you should avoid using RSA_NO_PADDING unless you really know what you're doing.
    • I noticed that your hash string seems to be hex. If you meant to use the actual bytes represented by the hex characters, you will need to pass 'hex' as the second argument to new Buffer(). Keep in mind though that this will halve your input size, so you will need to double the size of your input.
  • You should avoid deprecated APIs:
    • Buffer.from() should be used instead of new Buffer() in this case
    • crypto.constants should be used instead of just constants, as the latter is deprecated in favor of per-module constants
  • RSA_PKCS1_OAEP_PADDING can only be used for encryption with public keys, not private keys.

So the crypto documentation needs to be updated to remove RSA_PKCS1_OAEP_PADDING from the list of possible paddings for privateEncrypt().

@rensrongen
Copy link
Author

Thanks for the info.

I don't actually intend to use RSA_NO_PADDING though.
The example, unlike my initial comment, is based off older code.

Shouldn't RSA_PKCS1_OAEP_PADDING still work in the example I provided?

@mscdex
Copy link
Contributor

mscdex commented Nov 14, 2016

@rensrongen Not with a private key. If you use a public key and crypto.publicEncrypt() it should work (OAEP padding is actually the default for that function).

@rensrongen
Copy link
Author

Never mind, it's clarified! I was about to comment on the docs but I missed the last line of your previous comment.

@mscdex
Copy link
Contributor

mscdex commented Nov 14, 2016

I've opened a separate issue for the docs: #9609

@mscdex mscdex closed this as completed Nov 15, 2016
@mv2050arshad
Copy link

Dear friends, i have same problem but when i using RSA_PKCS1_PADDING, just the message can be decrypted(public-key) that they are encrypted(private-key) in 256 byte. some time encrypted message is 257 or 258 or 59 byte, then it failed to decryption. can you tell me whats my wrong and problem ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants