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

Is this library/package actively maintained or is it unsupported? #1431

Open
purplepenguin42 opened this issue Jul 26, 2024 · 6 comments
Open

Comments

@purplepenguin42
Copy link

Security issue notifications

If you discover a potential security issue in the AWS Encryption SDK we ask that you notify AWS Security via our vulnerability reporting page. Please do not create a public GitHub issue.

Problem:

The docs for this library/package are lacking quite severely in basic working examples. It looks like it was either never really finished/ready for prime time, or has simply been dropped all together in terms of being actively maintained.

For example, the only examples page that seemingly exists doesn't actually explain where the KmsKeyringNode class comes from, or how to get the encrypt and decrypt functions. You have to look at the raw example code in GitHub to even have a chance at understanding how to get working code.

Solution:

Provide a clear copy/paste example in the docs versus omitting key things.

Include the actual import statement needed for KmsKeyringNode:

import { KmsKeyringNode } from '@aws-crypto/client-node';

Include the actual import statement needed for the encrypt and decrypt functionality:

import { buildClient } from '@aws-crypto/client-node';

Include the actual function call to generate the two functions:

const { encrypt, decrypt } = buildClient();

At this point, I still have no idea if this is even the correct way to do it. There aren't any docs whatsoever about the buildClient function among many others.

Out of scope:

Is there anything the solution will intentionally NOT address?

@purplepenguin42 purplepenguin42 changed the title Is this library/package actively maintained or is it unsupported> Is this library/package actively maintained or is it unsupported? Jul 26, 2024
@codyfrisch
Copy link

From all I can tell, and the lack of attention to breaking issues, the lack of response etc. that this project has been abandoned. Which is surprising since it is part of an official AWS project.

@texastony
Copy link
Contributor

@purplepenguin42

The Developer Guide you are quoting from does not detail every line of code;
but it has links to complete examples.

I think you would fine the example-node package particularly helpful.

The first 20 lines of kms_simple.ts detail exactly what you suggest above:

import {
KmsKeyringNode,
buildClient,
CommitmentPolicy,
} from '@aws-crypto/client-node'
/* This builds the client with the REQUIRE_ENCRYPT_REQUIRE_DECRYPT commitment policy,
* which enforces that this client only encrypts using committing algorithm suites
* and enforces that this client
* will only decrypt encrypted messages
* that were created with a committing algorithm suite.
* This is the default commitment policy
* if you build the client with `buildClient()`.
*/
const { encrypt, decrypt } = buildClient(
CommitmentPolicy.REQUIRE_ENCRYPT_REQUIRE_DECRYPT
)

With respect to your other questions:
Yes, this library is complete, maintained, and has not been abandoned;
though we acknowledge that it has not had a release in a long time.

If we linked included links to the example packages in the README,
would that have helped you discover them?

Leaving open, pending @purplepenguin42 response.

@codyfrisch
Copy link

codyfrisch commented Jul 29, 2024

It clearly is not maintained or monitored as the duplexify issue has not been addressed which makes the library useless on node 20 without manually overriding it. There has been a pull request for a fix sitting there waiting for review by someone for months.

No this is library not maintained as it doesn't even work with the current runtime for AWS Lambda without developers having to manually override a dependency to get it to work.

If it was actually being maintained you would know this.

@purplepenguin42
Copy link
Author

purplepenguin42 commented Jul 29, 2024

@texastony

The Developer Guide you are quoting from does not detail every line of code;
but it has links to complete examples.

The reason the examples are confusing is precisely the opposite, they literally have every single line of code, EXCEPT for the two starting lines. I can understand an example that only has a few lines here or there, but the example has all of the other lines of code except the import statement and build client call. It'd be a lot easier to simply add the 2 lines to https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/js-examples.html page.

Regarding the buildClient call, apart from the single example, there aren't any actual docs for what it is or what options it accepts. Compare this to the actual SDK packages, that have complete docs, descriptions of every function, inputs, outputs, etc.

Are there other ways to build the client? Does it take options other than a commitment policy? I have no idea.

Then we get to things like manually needing to verify the context matches after decryption, which is weird considering the actual base KMS client encrypt and decrypt functions automatically do that for you and throws an error respectively.

Again, I mean no offense, but this package looks like someone's weekend project when compared to the actual main AWS SDK packages; it's like night and day difference in terms of documentation and explanations. It seems weird this is the official best practice way of encrypting data using envelope encryption with KMS and node, and it largely feels like an afterthought on AWS' part.

Appreciate the answers nonetheless.

EDIT: The fact the package hasn't received a single update in over 3 years also makes it hard to believe everything is maintained since there are clearly open issues and none of them are being touched or actioned. Put another way, if this package is actively maintained, what does that mean?

@josecorella
Copy link
Contributor

@purplepenguin42

Developer Guide Changes

I have requested changes to our Developer Guide to include buildClient.
I think that is a very good idea.

buildClient

Regarding the options for buildClient, which is, as you have said, entirely undocumented.

The two options today are:

  • Commitment Policy
  • Max Encrypted Data Keys

Here is an example using buildClient with a Commitment Policy and Max Encrypted Data Keys:

const { encrypt } = buildClient({ commitmentPolicy, maxEncryptedDataKeys: 3 })
const { decrypt } = buildClient({
commitmentPolicy,
maxEncryptedDataKeys: false,
})

Encryption Context

I think there is some very reasonable confusion about how Encryption Context works in the AWS Encryption SDK (ESDK) vs KMS.

Both KMS and the ESDK treat Encryption Context as Additional Authenticated Data (AAD),
which is validated at Decrypt.

The KMS decrypt operation takes Encryption Context as an explicit parameter;
the ESDK's Encryption Context is, by default, always serialized in the message,
as is detailed in the Specification's Encrypt.

This is a usability feature; otherwise for every message the customer must always be able to reproduce the Encryption Context of the message, or else fail to decrypt the ciphertext.

Thus, the ESDK's original Decrypt operation does not take Encryption Context as an argument.

This is not optimal;
while it is good that the ESDK validates Encryption Context on Decrypt,
it SHOULD also validate that the Encryption Context aligns with the Requesters expectation.

AWS Crypto Tools is working on changing this in all of our implementations;
we are adding an optional argument of Encryption Context to the Decrypt operation.

We have implemented and deployed this change to the Java & .NET implementations.

The Python, JavaScript, & C implementations are still pending this change.

On Decrypt, manually validating that the Encryption Context is optional,
but it ensures that the message's Encryption Context matches the requesters expectation,
which can mitigate threats in an application's threat model,
though not every threat model will have such a threat.

Duplexify Issue

We have addressed the issue. You can pick up the fix in version 4.0.1 of the ESDK.

Maintenance and Support

Our support policy is detailed here.

The 3.x release will enter End of Support soon;
4.x is Generally Available.

I can clarify what we mean by maintained:

  • Any Security issue is rapidly addressed
  • Usability issues are triaged and prioritized by customer impact
  • GitHub issues and updates to them are always triaged as they come in
  • We may not publicly respond to GitHub issue when it is triaged

Finally, the library's latest releases were published on 2022/03/15, 2023/07/17, and 2024/07/30.
It is incorrect to say that the library has not been updated in 3 years.

@purplepenguin42
Copy link
Author

@josecorella

Appreciate the detailed response and am definitely a fan of the proposed improvements. Regarding the 3 years comment, I was mainly basing that off the changelog for @aws-crypto/client-node package, which appears to only be version bumps with the last actual "real" change in the v2.0.0 release in (2020-09-25): https://github.com/aws/aws-encryption-sdk-javascript/blob/master/modules/client-node/CHANGELOG.md#200-2020-09-25

I understand some of the submodules might have been updated in the meantime with other releases, etc. Either way, don't intend to harp on that, it just seemed overall this repo didn't get much love, but hopefully it will get a bit more moving forward.

Thanks :)

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

No branches or pull requests

4 participants