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

use native API for AES #314

Closed
totaam opened this issue Jul 10, 2024 · 5 comments
Closed

use native API for AES #314

totaam opened this issue Jul 10, 2024 · 5 comments
Labels
enhancement New feature or request network

Comments

@totaam
Copy link
Collaborator

totaam commented Jul 10, 2024

Switch from forge to W3C: SubtleCrypto interface

forge has problems and doesn't handle Uint8Array natively, which makes it slow - also it looks abandoned.
The new API supports BufferSource.

We can remove huge libraries and rely on the browser implemenation!
even Safari supports it: caniuse: SubtleCrypto

Some examples here: aes-cbc

@totaam totaam added enhancement New feature or request network labels Jul 10, 2024
@totaam
Copy link
Collaborator Author

totaam commented Sep 29, 2024

totaam added a commit that referenced this issue Sep 29, 2024
aes still needs doing.. fails with the ever so unhelpful 'OperationError'
totaam added a commit that referenced this issue Oct 1, 2024
totaam added a commit that referenced this issue Oct 1, 2024
and require 'crypto.getRandomValues' rather than using an insecure fallback
totaam added a commit that referenced this issue Oct 1, 2024
totaam added a commit that referenced this issue Oct 1, 2024
totaam added a commit that referenced this issue Oct 2, 2024
totaam added a commit that referenced this issue Oct 2, 2024
@totaam
Copy link
Collaborator Author

totaam commented Oct 2, 2024

The key (!) problem is that the xpra python code wrongly used 258 bits of padding with AES-CBC, which manifested itself half the time! See: Xpra-org/xpra#4372 (comment)

totaam added a commit that referenced this issue Oct 2, 2024
totaam added a commit that referenced this issue Oct 3, 2024
not the current one, which may be different by then.
also some logging tweaks
@totaam
Copy link
Collaborator Author

totaam commented Oct 3, 2024

Now working for AES-CBC (which is the default mode).

The other modes need more work.

@totaam
Copy link
Collaborator Author

totaam commented Oct 5, 2024

As per the Xpra-org/xpra#4372 (comment), the browser's implementation of AES is inadequate for our use case.
What we really want is Web Crypto Streams instead - which is only at the draft stage 😞

So the options to make this work without making it completely unsafe are:

  • send new IV with each new encrypted packet - would make sense to use a counter for some of the bits rather than just random input, to avoid the birthday paradox: 3 common mistakes when implementing encryption : IV generation: It's always better to generate AES-GCM, AES-CTR and ChaCha20 nonces deterministically with a counter rather than randomly, or worse, hardcoded.
  • does this make any sense to use any other modes at all?

totaam added a commit that referenced this issue Oct 7, 2024
tell the server that we can't handle stream mode, so that it will send a new iv with every encryption 'session'
totaam added a commit that referenced this issue Oct 7, 2024
missed from 842fd3c which implies using a new iv for every packet
@totaam
Copy link
Collaborator Author

totaam commented Oct 8, 2024

This will do for now.

It's likely that more updates will be needed for Xpra-org/xpra#4375

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request network
Projects
None yet
Development

No branches or pull requests

1 participant