Skip to content

WIP Cleanups based on review feedback #27

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

Closed
rfk wants to merge 1 commit into from
Closed

WIP Cleanups based on review feedback #27

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
87 changes: 77 additions & 10 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,86 @@
[Latest Version]: https://img.shields.io/crates/v/ece.svg
[crates.io]: https://crates.io/crates/ece

*This crate has not been security reviewed yet, use at your own risk ([tracking issue](https://github.com/mozilla/rust-ece/issues/18))*.
*This crate has not been security reviewed yet, use at your own risk
([tracking issue](https://github.com/mozilla/rust-ece/issues/18))*.

[ece](https://crates.io/crates/ece) is a Rust implementation of the HTTP Encrypted Content-Encoding standard (RFC 8188). It is a port of the [ecec](https://github.com/web-push-libs/ecec) C library.
This crate is destined to be used by higher-level Web Push libraries, both on the server and the client side.
The [ece](https://crates.io/crates/ece) crate is a Rust implementation of Message Encryption for Web Push
([RFC8291](https://tools.ietf.org/html/rfc8291)) and the HTTP Encrypted Content-Encoding scheme
([RFC8188](https://tools.ietf.org/html/rfc8188)) on which it is based.

[Documentation](https://docs.rs/ece/)
It provides low-level cryptographic "plumbing" and is destined to be used by higher-level Web Push libraries, both on
the server and the client side. It is a port of the [ecec](https://github.com/web-push-libs/ecec) C library.

## Cryptographic backends

This crate is designed to be used with different crypto backends. At the moment only [openssl](https://github.com/sfackler/rust-openssl) is supported.
[Full Documentation](https://docs.rs/ece/)

## Implemented schemes

Currently, two HTTP ece schemes are available to consumers of the crate:
- The newer [RFC8188](https://tools.ietf.org/html/rfc8188) `aes128gcm` standard.
- The legacy [draft-03](https://tools.ietf.org/html/draft-ietf-httpbis-encryption-encoding-03) `aesgcm` scheme.
This crate implements both the published Web Push Encryption scheme, and a legacy scheme from earlier drafts
that is still widely used in the wild:

* `aes128gcm`: the scheme described in [RFC8291](https://tools.ietf.org/html/rfc8291) and
[RFC8188](https://tools.ietf.org/html/rfc8188)
* `aesgcm`: the draft scheme described in
[draft-ietf-webpush-encryption-04](https://tools.ietf.org/html/draft-ietf-webpush-encryption-04) and
[draft-ietf-httpbis-encryption-encoding-03](https://tools.ietf.org/html/draft-ietf-httpbis-encryption-encoding-03_)
Copy link
Copy Markdown
Contributor Author

@rfk rfk May 23, 2019

Choose a reason for hiding this comment

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

I tried to track down the specific specs, which includes both ECE and WebPush. Did I find the right ones? (In particular, are these the right draft numbers for the draft specs? I reverse-engineered which draft to link based on the details of how the code currently behaves).


## Usage

To receive messages via WebPush, the receiver must generate an EC keypair and a symmetric authentication secret,
then distribute the public key and authentication secret to the sender:

```
let (keypair, auth_secret) = ece::generate_keypair_and_auth_secret()?;
let pubkey = keypair.pub_as_as_raw();
// Base64-encode the `pubkey` and `auth_secret` bytes and distribute them to the sender.
```

The sender can encrypt a Web Push message to the receiver's public key:

```
let ciphertext = ece::encrypt(pubkey, auth_secret, b"payload")?;
Copy link
Copy Markdown
Contributor Author

@rfk rfk May 23, 2019

Choose a reason for hiding this comment

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

I propose we remove the salt argument from this function, since it seems like a footgun we can easily help the caller avoid by just generating a fresh random salt each time. (So I did so pre-emptively in the docs here)

Copy link
Copy Markdown
Collaborator

@jrconlin jrconlin May 31, 2019

Choose a reason for hiding this comment

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

fair enough. The salt is mostly a legacy thing now with the sunset of aesgcm. If we have this default to using aes128gcm for encryption/decryption then there's no problem.

```

And the receiver can decrypt it using their private key:

```
let plaintext = ece::decrypt(keypair, auth_secret)?;
```

That's pretty much all there is to it! It's up to the higher-level library to manage distributing the encrypted payload,
typically by arranging for it to be included in a HTTP response with `Content-Encoding: aes128gcm` header.

### Legacy `aesgcm` encryption

The legacy `aesgcm` scheme is more complicated, because it communicates some encryption parameters in HTTP header fields
rather than as part of the encrypted payload. When used for encryption, the sender must deal with `Encryption` and
`Crypto-Key` headers in addition to the ciphertext:

```
let encrypted_block = ece::AesGcmEceWebPushImpl::encrypt(pubkey, auth_secret, b"payload")?;
for (header, &value) in encrypted_block.headers().iter() {
// Set header to corresponding value
}
// Send encrypted_block.ciphertext as the body
```

When receiving an `aesgcm` message, the receiver needs to parse encryption parameters from the `Encryption`
and `Crypto-Key` fields:

```
// Parse `rs`, `salt` and `dh` from the `Encryption` and `Crypto-Key` headers.
// You'll need to consult the spec for how to do this; we might add some helpers one day.
let encrypted_block = ece::AesGcmEncryptedBlock::new(dh, rs, salt, ciphertext);
let plaintext = ece::AesGcmEceWebPushImpl::decrypt(keypair, auth_secret, encrypted_block)?;
```

### Unimplemented Features

* We do not implement streaming encryption or decryption, although the ECE scheme is designed to permit it.
* The application cannot control what padding is apply during encryption; we randonly select a length of padding to add
during encryption, but the RFC suggests that the best choice of strategy [is application-dependant](https://tools.ietf.org/html/rfc8188#section-4.8)
Copy link
Copy Markdown
Contributor Author

@rfk rfk May 23, 2019

Choose a reason for hiding this comment

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

I'd like to add more explanation about how the padding thing works but I don't really understand it - are we just randomly selecting some length to pad to? I wonder if we should give the caller control over that rather than doing it behind the scenes.

Copy link
Copy Markdown
Collaborator

@jrconlin jrconlin May 31, 2019

Choose a reason for hiding this comment

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

The padding is mostly there to obscure the length of the actual data, with the provision that a minimum set be present. There's concern that left to the application, no padding is used which could compromise the encryption (as noted in the spec). I'll add that we don't currently support multi-segment messages, so it could be a bit easier to predict payloads if no (or consistently sized) padding is used. Using a randomized padding size like this does kind of remove a footgun.

Copy link
Copy Markdown
Contributor Author

@rfk rfk Jun 2, 2019

Choose a reason for hiding this comment

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

Thanks for the explanation.

I expect there are better things we could do than "random padding" here, such as bucketing to the nearest power of 2. IIUC the problem with adding random noise is that it's easy to remove when doing statistical analysis, you just have to collect a greater volume of traffic. But we can explore that in followups.

Copy link
Copy Markdown
Collaborator

@jrconlin jrconlin Jun 3, 2019

Choose a reason for hiding this comment

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

Seems reasonable to me.

Also nit on line 84: s/apply/applied/


## Cryptographic backends

This crate is designed to use pluggable backend implementations of low-level crypto primitives. different crypto
backends. At the moment only [openssl](https://github.com/sfackler/rust-openssl) is supported.