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

README additions #58

Merged
merged 1 commit into from
Aug 15, 2022
Merged

README additions #58

merged 1 commit into from
Aug 15, 2022

Conversation

coderdan
Copy link
Contributor

@coderdan coderdan commented Aug 2, 2022

Updated the README to make sure insecure code doesn't get implemented by copy-pasta.

### What alogorithm should I use?

Unless you need to maintain compatibility with an older system (or you know exactly what you're doing),
you should use "aes-256-gcm" for the mode.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I think GCM or CTR modes require the set of IVs to be unique. There can be no overlap, basically.

The code currently generates IVs randomly if not provided. It doesn't guarantee uniqueness.

For GCM mode, perhaps we should allow options.iv to be a function where library users can provide a function that guarantees uniqueness.

Until then, CBC might be safer.

Copy link
Contributor Author

@coderdan coderdan Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CBC requires unique IVs, too. All modes of operation for AES to (with the possible exception of SIV which isn't available in Node anyway).

If the IVs are 16-bytes long and generated with a Cryptographically Secure Pseudo Random Number Generator (CSPRNG) then that is perfectly fine. The randomBytes function (that is used internally now) meets this requirement.

Providing a function to generate the IV would work but puts a lot of responsibility on the developer and could make mistakes more likely. Note: with SIV, I say possible because that mode is "resistant" to IV reuse but still recommends unique IVs.

Comment on lines +189 to +194
### Should I hardcode the IV?

No. Don't ever do this.
It will break the encryption and is vulnerable to a "repeated nonce" attack.

If you don't provide an IV, the library will randomly generate a secure one for you.
Copy link
Owner

@generalpiston generalpiston Aug 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "In practice, hardcoding an IV is not secure. See repeated nonce attack." And link to https://www.elttam.com/blog/key-recovery-attacks-on-gcm/.


### Should I hardcode the IV?

No. Don't ever do this.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strong words... in a test case I think using the same IV is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strong words indeed but for good reason! How about we say "No. Don't ever do this in production." ?

@@ -179,6 +177,23 @@ class User {

Queries that transform the encrypted column wont work because transformers and subscribers operate outside of the DBMS.

### What alogorithm should I use?

Unless you need to maintain compatibility with an older system (or you know exactly what you're doing),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these algorithms have different properties that have different applications and requirements. We can give advice like "aes-256-gcm works well for encryption at rest" and point people to more information. But I don't think we should give off a tone where all of this stuff seems beyond them.

So how about this:

Which algorithm should I use?

aes-256-gcm works well for encryption at rest.

For a break down of algorithms, see https://stackoverflow.com/questions/1220751/how-to-choose-an-aes-encryption-mode-cbc-ecb-ctr-ocb-cfb.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no problem in pointing people to educational resources. Having said that, its probably worth mentioning that unless you understand this stuff well that GCM is generally the best choice.

CBC does not provide authenticity and so is vulnerable to chosen ciphertext attacks.

@generalpiston generalpiston merged commit 07dd7c4 into generalpiston:master Aug 15, 2022
@generalpiston
Copy link
Owner

Thanks @coderdan for your PR. Great work!

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