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

Update tutorial and comments for A6 to match Node.js best practices #104

Merged
merged 5 commits into from
Sep 29, 2017

Conversation

jboyer2012
Copy link
Contributor

@jboyer2012 jboyer2012 commented Sep 26, 2017

According to Node's Crypto API docs, the default initialization vector used by the createCipher function is not random enough to protect against brute-force attacks. This code change updates the tutorial and comments for A6 to provide guidance on how to create a properly random IV for use in the AES256 algorithm.

@ckarande
Copy link
Member

@jboyer2012 Thanks for the PR. A great suggestion.

A few minor items regarding conventions:

  1. The project style guide needs using double quotes and semicolon at end of statements. Can you please update the PR to fix this error:

    app/data/profile-dao.js
    26 | return crypto.pbkdf2Sync(config.cryptoKey, salt, 100000, 512, 'sha512');
    ^ Strings must use doublequote.
    27 | }
    ^ Missing semicolon.

  2. I just noticed an existing issue (not from your PR) with a comment on line 15 which says:
    // Fix for A7 - Sensitive Data Exposure
    Can you please also change A7 to A6

Other changes look great and I will merge the PR as soon as you update the PR. Thanks 👍

@jboyer2012
Copy link
Contributor Author

@ckarande I have updated the code per your instructions. You're good to go to merge. Thanks for the help!

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

Successfully merging this pull request may close these issues.

2 participants