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: privateEncrypt & publicDecrypt Implementation #477

Closed
fuson opened this issue Jan 17, 2015 · 13 comments
Closed

crypto: privateEncrypt & publicDecrypt Implementation #477

fuson opened this issue Jan 17, 2015 · 13 comments
Assignees
Labels
crypto Issues and PRs related to the crypto subsystem.

Comments

@fuson
Copy link

fuson commented Jan 17, 2015

Without this functionality i can't migrate from ursa module.

@calvinmetcalf
Copy link
Contributor

I'm not sure the question, they are in iojs

@bnoordhuis bnoordhuis added unconfirmed crypto Issues and PRs related to the crypto subsystem. labels Jan 17, 2015
@Fishrock123
Copy link
Contributor

@calvinmetcalf Those are the opposite of what they are asking.

publicEncrypt & privateDecrypt vs (their) privateEncrypt & publicDecrypt, assuming they are wanting the correct things.

@fuson
Copy link
Author

fuson commented Jan 19, 2015

Fishrock123, correct.

@calvinmetcalf
Copy link
Contributor

ah missred, if you turn off padding and use privateDecrypt on plain text you will get ciphertext that can be decrypted with publicEncrypt

@bnoordhuis
Copy link
Member

@calvinmetcalf Is that process completely symmetric? My initial hunch was that the result from crypto.privateDecrypt() would get bumped to cipher block size even with RSA_NO_PADDING, and a quick test seems to confirm that.

@calvinmetcalf
Copy link
Contributor

what do you mean by cipher block size in this context modulus size?

@bnoordhuis
Copy link
Member

@calvinmetcalf This, basically:

var options = { key: key, padding: constants.RSA_NO_PADDING };
var encrypted = crypto.privateDecrypt(options, 'plaintext');  // encrypted.length == 128
var decrypted = crypto.publicEncrypt(options, encrypted);  // decrypted.length == 128
// decrypted consists of 119 zero bytes followed by the original plaintext

@calvinmetcalf
Copy link
Contributor

yes it's left padded up to the modulus length, it likely does it when you do

var options = { key: key, padding: constants.RSA_NO_PADDING };
var encrypted = crypto.publicEncrypt(options, 'plaintext');  // encrypted.length == 128
var decrypted = crypto.privateDecrypt(options, encrypted); 

as well (can't check at the moment). It's enough functionality to write your own padding/unpadding on top of it

@calvinmetcalf
Copy link
Contributor

to follow up , @bnoordhuis you are correct that it is not symmetric,

var encrypted = crypto.publicEncrypt(options, new Buffer('plaintext'));

throws an error unless you left pad it with zeros yourself, privateDecrypt left pads it for you because the output of publicEncrypt will omit a leading zeros (for instance with this key var encrypted = crypto.privateDecrypt(options, new Buffer('plaintext')); creates an output of 127 bytes.

@indutny
Copy link
Member

indutny commented Jan 27, 2015

It can't be symmetric, there is a prefix byte that does a thing.

@indutny
Copy link
Member

indutny commented Jan 27, 2015

I'm going to work on it.

@indutny
Copy link
Member

indutny commented Jan 27, 2015

See #625

@indutny
Copy link
Member

indutny commented Jan 27, 2015

Shall be fixed in 87e62bd

briansmith pushed a commit to briansmith/ring that referenced this issue Jan 28, 2016
node.js is, effectively, another bindings library. However, it's better
written than most and, with these changes, only a couple of tiny fixes
are needed in node.js. Some of these changes are a little depressing
however so we'll need to push node.js to use APIs where possible.

Changes:
  ∙ Support verify_recover. This is very obscure and the motivation
    appears to be nodejs/node#477 – where it's
    not clear that anyone understands what it means :(
  ∙ Add a few, no-op #defines
  ∙ Add some members to |SSL_CTX| and |SSL| – node.js needs to not
    reach into these structs in the future.
  ∙ Add EC_get_builtin_curves.
  ∙ Add EVP_[CIPHER|MD]_do_all_sorted – these functions are limited to
    decrepit.

Change-Id: I9a3566054260d6c4db9d430beb7c46cc970a9d46
Reviewed-on: https://boringssl-review.googlesource.com/6952
Reviewed-by: Adam Langley <[email protected]>
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

5 participants