Update the public API to remove footguns, and document it.

This is a significant refactor of the public API of the crate, simplifying
the API surface and removing some of the footgun potential noted by Martin
in his review at mozilla/application-services#1068.

In particular:

* The public `encrypt` functions no longer take a `salt` parameter. The
  right thing to do is to generate a new random `salt` for each encryption
  so we just do that for you automatically.
* Many internal implementation details are now `pub(crate)` rather than `pub`,
  to avoid potential confusion from consumers.
* We refuse to encrypt or decrypt across multiple records, because our only
  consumer in practice is webpush, and webpush restricts consumers to using
  only a single record.

We still have the code lying around to encrypt/decrypt across record
boundaries, but we don't have high confidence that it works correctly
and intend to remove it in a future commit. So, may as well adjust the
interface to reflect that while we're in here making breaking changes.

To go along with the revised interface, this commit also significantly
expands to docs in order to help set consumer expectations and context.

Author: rfk

Cargo.toml

@@ -12,16 +12,14 @@ keywords = ["http-ece", "web-push"]
 byteorder = "1.3"
 thiserror = "1.0"
 base64 = "0.12"
+hex = "0.4"
 hkdf = { version = "0.9", optional = true }
 lazy_static = { version = "1.4", optional = true }
 once_cell = "1.4"
 openssl = { version = "0.10", optional = true }
 serde = { version = "1.0", features = ["derive"], optional = true }
 sha2 = { version = "0.9", optional = true }
 
-[dev-dependencies]
-hex = "0.4"
-
 [features]
 default = ["backend-openssl", "serializable-keys"]
 serializable-keys = ["serde"]

README.md

@@ -5,22 +5,95 @@
 [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_)
+
+## 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_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")?;
+```
+
+And the receiver can decrypt it using their private key:
+
+```
+let plaintext = ece::decrypt(&keypair, &auth_secret, &ciphertext)?;
+```
+
+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::legacy::encrypt_aesgcm(pubkey, auth_secret, b"payload")?;
+for (header, &value) in encrypted_block.headers().iter() {
+  // Set header to corresponding value
+}
+// Send encrypted_block.body() 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::legacy::decrypt_aesgcm(keypair, auth_secret, encrypted_block)?;
+```
+
+### Unimplemented Features
+
+* We do not implement streaming encryption or decryption, although the ECE scheme is designed to permit it.
+* We do not implement support for encrypting or decrypting across multiple records, although the ECE scheme is designed to permit it.
+* We do not support customizing the record size parameter during encryption, but do check it during decryption.
+  * The default record size is 4096 bytes, which effectively limits the size of plaintext that can be encrypted to 3993 bytes.
+* We do not support customizing the padding bytes added during encryption.
+  * We currently select the padding length at random for each encryption, but this is an implementation detail and
+    should not be relied on.
+
+These restrictions might be lifted in future, if it turns out that we need them.
+
+## 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.
 
 ## Release process
 
@@ -33,5 +106,4 @@ make sure you have it installed and then:
 2. Run `cargo release --dry-run -vv [major|minor|patch]` and check that the things
    it's proposing to do seem sensible.
 3. Run `cargo release [major|minor|patch]` to prepare, commit, tag and publish the release.
-4. Make a PR from your `release-vX.Y.Z` branch to request it be merged to the main branch.
-
+4. Make a PR from your `release-vX.Y.Z` branch to request it be merged to the main branch.

src/aes128gcm.rs

@@ -29,27 +29,8 @@ const ECE_AES128GCM_NONCE_INFO: &str = "Content-Encoding: nonce\0";
 /// Web Push encryption structure for the AES128GCM encoding scheme ([RFC8591](https://tools.ietf.org/html/rfc8291))
 ///
 /// This structure is meant for advanced use. For simple encryption/decryption, use the top-level [`encrypt`](crate::encrypt) and [`decrypt`](crate::decrypt) functions.
-pub struct Aes128GcmEceWebPush;
+pub(crate) struct Aes128GcmEceWebPush;
 impl Aes128GcmEceWebPush {
-    /// Encrypts a Web Push message using the "aes128gcm" scheme. This function
-    /// automatically generates an ephemeral ECDH key pair.
-    pub fn encrypt(
-        remote_pub_key: &dyn RemotePublicKey,
-        auth_secret: &[u8],
-        plaintext: &[u8],
-        params: WebPushParams,
-    ) -> Result<Vec<u8>> {
-        let cryptographer = crypto::holder::get_cryptographer();
-        let local_prv_key = cryptographer.generate_ephemeral_keypair()?;
-        Self::encrypt_with_keys(
-            &*local_prv_key,
-            remote_pub_key,
-            auth_secret,
-            plaintext,
-            params,
-        )
-    }
-
     /// Encrypts a Web Push message using the "aes128gcm" scheme, with an explicit
     /// sender key. The sender key can be reused.
     pub fn encrypt_with_keys(

src/aesgcm.rs

@@ -25,21 +25,20 @@ const ECE_WEBPUSH_RAW_KEY_LENGTH: usize = 65;
 const ECE_WEBPUSH_IKM_LENGTH: usize = 32;
 
 pub struct AesGcmEncryptedBlock {
-    pub dh: Vec<u8>,
-    pub salt: Vec<u8>,
-    pub rs: u32,
-    pub ciphertext: Vec<u8>,
+    pub(crate) dh: Vec<u8>,
+    pub(crate) salt: Vec<u8>,
+    pub(crate) rs: u32,
+    pub(crate) ciphertext: Vec<u8>,
 }
 
 impl AesGcmEncryptedBlock {
-    pub fn aesgcm_rs(rs: u32) -> u32 {
+    fn aesgcm_rs(rs: u32) -> u32 {
         if rs > u32::max_value() - ECE_TAG_LENGTH as u32 {
             return 0;
         }
         rs + ECE_TAG_LENGTH as u32
     }
 
-    /// Create a new block from the various header strings and body content.
     pub fn new(
         dh: &[u8],
         salt: &[u8],
@@ -87,35 +86,20 @@ impl AesGcmEncryptedBlock {
     }
 
     /// Encode the body as a String.
-    /// If you need the bytes, probably just call .ciphertext directly
     pub fn body(&self) -> String {
         base64::encode_config(&self.ciphertext, base64::URL_SAFE_NO_PAD)
     }
 }
-/// Web Push encryption structure for the legacy AESGCM encoding scheme ([Web Push Encryption Draft 4](https://tools.ietf.org/html/draft-ietf-webpush-encryption-04))
+
+/// Web Push encryption structure for the legacy AESGCM encoding scheme
+/// ([Web Push Encryption Draft 4](https://tools.ietf.org/html/draft-ietf-webpush-encryption-04))
 ///
-/// This structure is meant for advanced use. For simple encryption/decryption, use the top-level [`encrypt_aesgcm`](crate::legacy::encrypt_aesgcm) and [`decrypt_aesgcm`](crate::legacy::decrypt_aesgcm) functions.
-pub struct AesGcmEceWebPush;
-impl AesGcmEceWebPush {
-    /// Encrypts a Web Push message using the "aesgcm" scheme. This function
-    /// automatically generates an ephemeral ECDH key pair.
-    pub fn encrypt(
-        remote_pub_key: &dyn RemotePublicKey,
-        auth_secret: &[u8],
-        plaintext: &[u8],
-        params: WebPushParams,
-    ) -> Result<AesGcmEncryptedBlock> {
-        let cryptographer = crypto::holder::get_cryptographer();
-        let local_prv_key = cryptographer.generate_ephemeral_keypair()?;
-        Self::encrypt_with_keys(
-            &*local_prv_key,
-            remote_pub_key,
-            auth_secret,
-            plaintext,
-            params,
-        )
-    }
+/// This structure is meant for advanced use. For simple encryption/decryption, use the top-level
+/// [`encrypt_aesgcm`](crate::legacy::encrypt_aesgcm) and [`decrypt_aesgcm`](crate::legacy::decrypt_aesgcm)
+/// functions.
+pub(crate) struct AesGcmEceWebPush;
 
+impl AesGcmEceWebPush {
     /// Encrypts a Web Push message using the "aesgcm" scheme, with an explicit
     /// sender key. The sender key can be reused.
     pub fn encrypt_with_keys(
@@ -201,7 +185,7 @@ impl EceWebPush for AesGcmEceWebPush {
         Ok(&block[(2 + padding_size)..])
     }
 
-    /// Derives the "aesgcm" decryption keyn and nonce given the receiver private
+    /// Derives the "aesgcm" decryption key and nonce given the receiver private
     /// key, sender public key, authentication secret, and sender salt.
     fn derive_key_and_nonce(
         ece_mode: EceMode,

src/common.rs

@@ -10,53 +10,42 @@ use byteorder::{BigEndian, ByteOrder};
 use std::cmp::min;
 
 // From keys.h:
-pub const ECE_AES_KEY_LENGTH: usize = 16;
-pub const ECE_NONCE_LENGTH: usize = 12;
+pub(crate) const ECE_AES_KEY_LENGTH: usize = 16;
+pub(crate) const ECE_NONCE_LENGTH: usize = 12;
 
 // From ece.h:
-pub const ECE_SALT_LENGTH: usize = 16;
-pub const ECE_TAG_LENGTH: usize = 16;
-//const ECE_WEBPUSH_PRIVATE_KEY_LENGTH: usize = 32;
-pub const ECE_WEBPUSH_PUBLIC_KEY_LENGTH: usize = 65;
-pub const ECE_WEBPUSH_AUTH_SECRET_LENGTH: usize = 16;
-const ECE_WEBPUSH_DEFAULT_RS: u32 = 4096;
+pub(crate) const ECE_SALT_LENGTH: usize = 16;
+pub(crate) const ECE_TAG_LENGTH: usize = 16;
+pub(crate) const ECE_WEBPUSH_PUBLIC_KEY_LENGTH: usize = 65;
+pub(crate) const ECE_WEBPUSH_AUTH_SECRET_LENGTH: usize = 16;
+pub(crate) const ECE_WEBPUSH_DEFAULT_RS: u32 = 4096;
 
 // TODO: Make it nicer to use with a builder pattern.
-pub struct WebPushParams {
+pub(crate) struct WebPushParams {
     pub rs: u32,
     pub pad_length: usize,
     pub salt: Option<Vec<u8>>,
 }
 
-impl WebPushParams {
-    /// Random salt, record size = 4096 and padding length = 0.
-    pub fn default() -> Self {
+impl Default for WebPushParams {
+    fn default() -> Self {
+        // Random salt, record size = 4096 and padding length = 0.
         Self {
             rs: ECE_WEBPUSH_DEFAULT_RS,
-            pad_length: 2,
+            pad_length: 0,
             salt: None,
         }
     }
-
-    /// Never use the same salt twice as it will derive the same content encryption
-    /// key for multiple messages if the same sender private key is used!
-    pub fn new(rs: u32, pad_length: usize, salt: Vec<u8>) -> Self {
-        Self {
-            rs,
-            pad_length,
-            salt: Some(salt),
-        }
-    }
 }
 
-pub enum EceMode {
+pub(crate) enum EceMode {
     ENCRYPT,
     DECRYPT,
 }
 
-pub type KeyAndNonce = (Vec<u8>, Vec<u8>);
+pub(crate) type KeyAndNonce = (Vec<u8>, Vec<u8>);
 
-pub trait EceWebPush {
+pub(crate) trait EceWebPush {
     fn common_encrypt(
         local_prv_key: &dyn LocalKeyPair,
         remote_pub_key: &dyn RemotePublicKey,
@@ -151,6 +140,12 @@ pub trait EceWebPush {
             plaintext_start = plaintext_end;
             counter += 1;
         }
+        // Cheap way to error out if the plaintext didn't fit in a single record.
+        // We're going to refactor away the multi-record stuff entirely in a future PR,
+        // but doing this here now lets us set API expectations for the caller.
+        if counter > 1 {
+            return Err(Error::PlaintextTooLong);
+        }
         Ok(ciphertext)
     }
 
@@ -184,6 +179,12 @@ pub trait EceWebPush {
         )?;
         let chunks = ciphertext.chunks(rs as usize);
         let records_count = chunks.len();
+        // Cheap way to error out if there are multiple records.
+        // We're going to refactor away the multi-record stuff entirely in a future PR,
+        // but doing this here now lets us set API expectations for the caller.
+        if records_count > 1 {
+            return Err(Error::MultipleRecordsNotSupported);
+        }
         let items = chunks
             .enumerate()
             .map(|(count, record)| {