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.

Sign up for GitHub

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

Jump to bottom

README additions #58

Merged
merged 1 commit into from
Aug 15, 2022
+23 −8
Merged

README additions #58

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,8 @@ class User {
nullable: false,
transformer: new EncryptionTransformer({
key: 'e41c966f21f9e1577802463f8924e6a3fe3e9751f201304213b2f845d8841d61',
algorithm: 'aes-256-cbc',
ivLength: 16,
iv: 'ff5ac19190424b1d88f9419ef949ae56'
algorithm: 'aes-256-gcm',
ivLength: 16
})
})
secret: string;
Expand All @@ -70,9 +69,8 @@ class User {
nullable: false,
transformer: new JSONEncryptionTransformer({
key: 'e41c966f21f9e1577802463f8924e6a3fe3e9751f201304213b2f845d8841d61',
algorithm: 'aes-256-cbc',
ivLength: 16,
iv: 'ff5ac19190424b1d88f9419ef949ae56'
algorithm: 'aes-256-gcm',
ivLength: 16
})
})
secret: object;
Expand Down Expand Up @@ -101,7 +99,7 @@ class User extends BaseEntity {
nullable: false,
encrypt: {
key: "d85117047fd06d3afa79b6e44ee3a52eb426fc24c3a2e3667732e8da0342b4da",
algorithm: "aes-256-cbc",
algorithm: "aes-256-gcm",
ivLength: 16
}
})
Expand Down Expand Up @@ -131,7 +129,7 @@ encryption-config.ts
// it's better to use an environment variable or to use dotenv in order to load the value
export const MyEncryptionTransformerConfig = {
key: process.env.ENCRYPTION_KEY,
algorithm: 'aes-256-cbc',
algorithm: 'aes-256-gcm',
ivLength: 16
};
```
Expand Down Expand Up @@ -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.

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 •
edited
Loading

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.

This means that the encryption keys are are 256 bits (32-bytes) long and that the mode of operation
is GCM ([Galois Counter Mode](https://en.wikipedia.org/wiki/Galois/Counter_Mode)).

GCM provides both secrecy and authenticity and can generally use CPU acceleration where available.

### 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." ?

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.
Comment on lines +189 to +194
Copy link
Owner

@generalpiston generalpiston Aug 2, 2022 •
edited
Loading

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/.



### Error: Invalid IV length

The most likely reasons you're receiving this error:
Expand Down