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

Documentation for most AES-GCM members #100

Merged
merged 2 commits into from
May 2, 2024
Merged

Conversation

jonasfj
Copy link
Member

@jonasfj jonasfj commented May 1, 2024

No description provided.

@jonasfj jonasfj requested review from szakarias and sigurdm May 1, 2024 13:37
@sigurdm
Copy link
Collaborator

sigurdm commented May 2, 2024

Does this documentation come from somewhere, or you wrote it all from scratch?

///
/// **Example**
/// ```dart
/// import 'package:webcrypto/webcrypto.dart';
Copy link
Collaborator

Choose a reason for hiding this comment

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

It nags me that the example is both top-level code, and statement-level code...
Ideally it would somehow be actual runnable code by some convention.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, maybe it's messy and we should add a main, or omit imports.

Feel free to file an issue on the repository.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok: #101

/// plaintext prior to complete verification of the tag breaks the
/// authenticity assurances. Specifically, until the entire message is
/// decrypted it is not possible to know if it is authentic, which would
/// defeat the purpose of _authenticated encryption_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Theoretically there could be a streaming API for computational reasons (ie. you don't have to keep everything in memory at once), ending up with the result being thrown away at the end.

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically correct, but BoringSSL doesn't offer a streaming interface, I suspect I read in a bug somewhere that it doesn't because the authors thought it would be a bad idea.

So that is the opinion we're carrying into package:webcrypto.

Also Web Cryptography specification doesn't offer streaming APIs at all, so I only added them because BoringSSL does. And on web we always buffer up the entire stream before we do the operation 🤣

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd either remove the explanation or link to the discussion. I don't think we need to explain this.

I kind-of agree that a streaming interface would be hard to use correctly. But so is basically all crypto primitives. You really have to know what you are doing.

@jonasfj
Copy link
Member Author

jonasfj commented May 2, 2024

Does this documentation come from somewhere, or you wrote it all from scratch?

I deciphered most of it from NIST SP 800-38D, which is also why I leave a lot of references to it.

Co-authored-by: Sigurd Meldgaard <[email protected]>
@jonasfj
Copy link
Member Author

jonasfj commented May 2, 2024

Filed #102 to follow up with more recommendations, once we find credible sources we can reference.

@jonasfj jonasfj merged commit a8fc471 into google:master May 2, 2024
6 checks passed
@jonasfj jonasfj deleted the docs-gcm branch May 2, 2024 11:13
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