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: State not recovered after an exception is thrown #4879

Closed
jiripospisil opened this issue Jan 26, 2016 · 9 comments
Closed

crypto: State not recovered after an exception is thrown #4879

jiripospisil opened this issue Jan 26, 2016 · 9 comments
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.

Comments

@jiripospisil
Copy link

While upgrading one of our applications to Node.js 4.2.6 (from 4.2.3) I bumped into an issue with the crypto module. In the tests, we supply an invalid key into the createCipheriv function and it barks as expected.

However, when running the same code again later in the tests, this time with a correct key, it still doesn't work and an exception is thrown. It weird thing is that the error comes from a different crypto function (createHash). I assume some internal state is not being cleared.

The code works on Node.js 4.2.3 and fails on 4.2.4-6. It's still entirely possible that we're doing something wrong. Any suggestions are welcomed.

https://gist.github.com/jiripospisil/70450ba715a004a11623

@evanlucas
Copy link
Contributor

@jiripospisil this should have been fixed in v4.2.5. Mind giving that one a try to confirm?

@evanlucas
Copy link
Contributor

ah, nevermind. Didn't see the -6 :[.

/cc @indutny ?

@bnoordhuis
Copy link
Member

761cf2b from #4731 fixes this. I tagged it lts-watch-v4.x the other day, it should be in the next LTS release. I'll close the issue but let me know if you have questions.

@bnoordhuis bnoordhuis added confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem. labels Jan 26, 2016
@MylesBorins
Copy link
Contributor

I'll make sure this lands

@MylesBorins
Copy link
Contributor

I've gone ahead and landed that commit on v4.x-staging.

@jiripospisil
Copy link
Author

Thank you guys for looking into it 👍

@jiripospisil
Copy link
Author

@thealphanerd There's a new LTS release but the issue doesn't seem to be resolved. Was the fix included in the 4.3.0 version?

https://gist.github.com/jiripospisil/5d52e1404e870203c794

@jiripospisil
Copy link
Author

/ping @evanlucas @bnoordhuis

@bnoordhuis
Copy link
Member

@jiripospisil The last LTS release was a security release. The fix for this issue is scheduled for tomorrow's bug fix release.

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

No branches or pull requests

4 participants