-
Notifications
You must be signed in to change notification settings - Fork 17
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
Remove outdated mcrypt dependency and rely on native node crypto #23
Conversation
MCRYPT_RIJNDAEL_128 seems to be alias name for AES-256-CBC
crypto's createCipheriv / createDecipheriv handle the padding for you
Hwy @b00tsy you should publish a JsCryptor-3 on npm :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I published my own of this PR changes minus the mcrypt dependency
jscryptor-kevkevin@0.0.2
but would be great to see a new version of this lib
"dependencies": { | ||
"mcrypt": "^0.1" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can probably delete this
|
||
var _settings = {}; | ||
|
||
var _configure_settings = function(version) { | ||
var settings = { | ||
algorithm: 'rijndael-128', | ||
algorithm: 'aes-256-cbc', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are rijndael-128
and aes-256-cbc
compatible would these break implementations that need backward compatibility?
As mcrypt is not maintained anymore node versions >= 12 can't use JSCryptor anymore.
Relying solely on node crypto might break backwards compatibility (not sure which versions will work).
TODO:
find out which node versions support native node crypto and if there are versions left behind decide whether they can be ditched