Implement an improved version of the EVP AEAD API. The

EVP_AEAD_CTX_{open,seal} functions previously returned an ssize_t that was
overloaded to indicate success/failure, along with the number of bytes
written as output. This change adds an explicit *out_len argument which
is used to return the number of output bytes and the return value is now
an int that is purely used to identify success or failure.

This change effectively rides the last libcrypto crank (although I do not
expect there to be many users of the EVP AEAD API currently).

Thanks to Adam Langley for providing the improved code that this diff is
based on.

ok miod@

5 files changed, 109 insertions, 106 deletions

diff --git a/lib/libssl/src/crypto/evp/e_aes.c b/lib/libssl/src/crypto/evp/e_aes.c
index 2e81495e5f4..0276cc2bd32 100644
--- a/lib/libssl/src/crypto/evp/e_aes.c
+++ b/lib/libssl/src/crypto/evp/e_aes.c
@@ -1281,9 +1281,10 @@ aead_aes_gcm_init(EVP_AEAD_CTX *ctx, const unsigned char *key, size_t key_len,
 	struct aead_aes_gcm_ctx *gcm_ctx;
 	const size_t key_bits = key_len * 8;
 
+	/* EVP_AEAD_CTX_init should catch this. */
 	if (key_bits != 128 && key_bits != 256) {
 		EVPerr(EVP_F_AEAD_AES_GCM_INIT, EVP_R_BAD_KEY_LENGTH);
-		return 0;  /* EVP_AEAD_CTX_init should catch this. */
+		return 0;
 	}
 
 	if (tag_len == EVP_AEAD_DEFAULT_TAG_LENGTH)
@@ -1324,88 +1325,92 @@ aead_aes_gcm_cleanup(EVP_AEAD_CTX *ctx)
 	free(gcm_ctx);
 }
 
-static ssize_t
-aead_aes_gcm_seal(const EVP_AEAD_CTX *ctx, unsigned char *out,
+static int
+aead_aes_gcm_seal(const EVP_AEAD_CTX *ctx, unsigned char *out, size_t *out_len,
     size_t max_out_len, const unsigned char *nonce, size_t nonce_len,
     const unsigned char *in, size_t in_len, const unsigned char *ad,
     size_t ad_len)
 {
-	size_t bulk = 0;
 	const struct aead_aes_gcm_ctx *gcm_ctx = ctx->aead_state;
 	GCM128_CONTEXT gcm;
+	size_t bulk = 0;
 
 	if (max_out_len < in_len + gcm_ctx->tag_len) {
 		EVPerr(EVP_F_AEAD_AES_GCM_SEAL, EVP_R_BUFFER_TOO_SMALL);
-		return -1;
+		return 0;
 	}
 
 	memcpy(&gcm, &gcm_ctx->gcm, sizeof(gcm));
 	CRYPTO_gcm128_setiv(&gcm, nonce, nonce_len);
 
 	if (ad_len > 0 && CRYPTO_gcm128_aad(&gcm, ad, ad_len))
-		return -1;
+		return 0;
 
 	if (gcm_ctx->ctr) {
 		if (CRYPTO_gcm128_encrypt_ctr32(&gcm, in + bulk, out + bulk,
 		    in_len - bulk, gcm_ctx->ctr))
-			return -1;
+			return 0;
 	} else {
 		if (CRYPTO_gcm128_encrypt(&gcm, in + bulk, out + bulk,
 		    in_len - bulk))
-			return -1;
+			return 0;
 	}
 
 	CRYPTO_gcm128_tag(&gcm, out + in_len, gcm_ctx->tag_len);
-	return in_len + gcm_ctx->tag_len;
+	*out_len = in_len + gcm_ctx->tag_len;
+
+	return 1;
 }
 
-static ssize_t
-aead_aes_gcm_open(const EVP_AEAD_CTX *ctx, unsigned char *out,
+static int
+aead_aes_gcm_open(const EVP_AEAD_CTX *ctx, unsigned char *out, size_t *out_len,
     size_t max_out_len, const unsigned char *nonce, size_t nonce_len,
     const unsigned char *in, size_t in_len, const unsigned char *ad,
     size_t ad_len)
 {
-	size_t bulk = 0;
 	const struct aead_aes_gcm_ctx *gcm_ctx = ctx->aead_state;
 	unsigned char tag[EVP_AEAD_AES_GCM_TAG_LEN];
-	size_t out_len;
 	GCM128_CONTEXT gcm;
+	size_t plaintext_len;
+	size_t bulk = 0;
 
 	if (in_len < gcm_ctx->tag_len) {
 		EVPerr(EVP_F_AEAD_AES_GCM_OPEN, EVP_R_BAD_DECRYPT);
-		return -1;
+		return 0;
 	}
 
-	out_len = in_len - gcm_ctx->tag_len;
+	plaintext_len = in_len - gcm_ctx->tag_len;
 
-	if (max_out_len < out_len) {
+	if (max_out_len < plaintext_len) {
 		EVPerr(EVP_F_AEAD_AES_GCM_OPEN, EVP_R_BUFFER_TOO_SMALL);
-		return -1;
+		return 0;
 	}
 
 	memcpy(&gcm, &gcm_ctx->gcm, sizeof(gcm));
 	CRYPTO_gcm128_setiv(&gcm, nonce, nonce_len);
 
 	if (CRYPTO_gcm128_aad(&gcm, ad, ad_len))
-		return -1;
+		return 0;
 
 	if (gcm_ctx->ctr) {
 		if (CRYPTO_gcm128_decrypt_ctr32(&gcm, in + bulk, out + bulk,
 		    in_len - bulk - gcm_ctx->tag_len, gcm_ctx->ctr))
-			return -1;
+			return 0;
 	} else {
 		if (CRYPTO_gcm128_decrypt(&gcm, in + bulk, out + bulk,
 		    in_len - bulk - gcm_ctx->tag_len))
-			return -1;
+			return 0;
 	}
 
 	CRYPTO_gcm128_tag(&gcm, tag, gcm_ctx->tag_len);
-	if (CRYPTO_memcmp(tag, in + out_len, gcm_ctx->tag_len) != 0) {
+	if (CRYPTO_memcmp(tag, in + plaintext_len, gcm_ctx->tag_len) != 0) {
 		EVPerr(EVP_F_AEAD_AES_GCM_OPEN, EVP_R_BAD_DECRYPT);
-		return -1;
+		return 0;
 	}
 
-	return out_len;
+	*out_len = plaintext_len;
+
+	return 1;
 }
 
 static const EVP_AEAD aead_aes_128_gcm = {
diff --git a/lib/libssl/src/crypto/evp/e_chacha20poly1305.c b/lib/libssl/src/crypto/evp/e_chacha20poly1305.c
index 7e32668b7fc..7aeaf8775dd 100644
--- a/lib/libssl/src/crypto/evp/e_chacha20poly1305.c
+++ b/lib/libssl/src/crypto/evp/e_chacha20poly1305.c
@@ -82,8 +82,9 @@ aead_chacha20_poly1305_init(EVP_AEAD_CTX *ctx, const unsigned char *key,
 		return 0;
 	}
 
+	/* Internal error - EVP_AEAD_CTX_init should catch this. */
 	if (key_len != sizeof(c20_ctx->key))
-		return 0;  /* internal error - EVP_AEAD_CTX_init should catch this. */
+		return 0;
 
 	c20_ctx = malloc(sizeof(struct aead_chacha20_poly1305_ctx));
 	if (c20_ctx == NULL)
@@ -100,6 +101,7 @@ static void
 aead_chacha20_poly1305_cleanup(EVP_AEAD_CTX *ctx)
 {
 	struct aead_chacha20_poly1305_ctx *c20_ctx = ctx->aead_state;
+
 	OPENSSL_cleanse(c20_ctx->key, sizeof(c20_ctx->key));
 	free(c20_ctx);
 }
@@ -121,11 +123,11 @@ poly1305_update_with_length(poly1305_state *poly1305,
 	CRYPTO_poly1305_update(poly1305, length_bytes, sizeof(length_bytes));
 }
 
-static ssize_t
+static int
 aead_chacha20_poly1305_seal(const EVP_AEAD_CTX *ctx, unsigned char *out,
-    size_t max_out_len, const unsigned char *nonce, size_t nonce_len,
-    const unsigned char *in, size_t in_len, const unsigned char *ad,
-    size_t ad_len)
+    size_t *out_len, size_t max_out_len, const unsigned char *nonce,
+    size_t nonce_len, const unsigned char *in, size_t in_len,
+    const unsigned char *ad, size_t ad_len)
 {
 	const struct aead_chacha20_poly1305_ctx *c20_ctx = ctx->aead_state;
 	unsigned char poly1305_key[32];
@@ -139,20 +141,20 @@ aead_chacha20_poly1305_seal(const EVP_AEAD_CTX *ctx, unsigned char *out,
 	 * 32-bits and this produces a warning because it's always false.
 	 * Casting to uint64_t inside the conditional is not sufficient to stop
 	 * the warning. */
-	if (in_len_64 >= (1ull << 32)*64 - 64) {
+	if (in_len_64 >= (1ULL << 32) * 64 - 64) {
 		EVPerr(EVP_F_AEAD_CHACHA20_POLY1305_SEAL, EVP_R_TOO_LARGE);
-		return -1;
+		return 0;
 	}
 
 	if (max_out_len < in_len + c20_ctx->tag_len) {
 		EVPerr(EVP_F_AEAD_CHACHA20_POLY1305_SEAL,
 		    EVP_R_BUFFER_TOO_SMALL);
-		return -1;
+		return 0;
 	}
 
 	if (nonce_len != CHACHA20_NONCE_LEN) {
 		EVPerr(EVP_F_AEAD_CHACHA20_POLY1305_SEAL, EVP_R_IV_TOO_LARGE);
-		return -1;
+		return 0;
 	}
 
 	memset(poly1305_key, 0, sizeof(poly1305_key));
@@ -168,29 +170,31 @@ aead_chacha20_poly1305_seal(const EVP_AEAD_CTX *ctx, unsigned char *out,
 		unsigned char tag[POLY1305_TAG_LEN];
 		CRYPTO_poly1305_finish(&poly1305, tag);
 		memcpy(out + in_len, tag, c20_ctx->tag_len);
-		return in_len + c20_ctx->tag_len;
+		*out_len = in_len + c20_ctx->tag_len;
+		return 1;
 	}
 
 	CRYPTO_poly1305_finish(&poly1305, out + in_len);
-	return in_len + POLY1305_TAG_LEN;
+	*out_len = in_len + POLY1305_TAG_LEN;
+	return 1;
 }
 
-static ssize_t
+static int
 aead_chacha20_poly1305_open(const EVP_AEAD_CTX *ctx, unsigned char *out,
-    size_t max_out_len, const unsigned char *nonce, size_t nonce_len,
-    const unsigned char *in, size_t in_len, const unsigned char *ad,
-    size_t ad_len)
+    size_t *out_len, size_t max_out_len, const unsigned char *nonce,
+    size_t nonce_len, const unsigned char *in, size_t in_len,
+    const unsigned char *ad, size_t ad_len)
 {
 	const struct aead_chacha20_poly1305_ctx *c20_ctx = ctx->aead_state;
 	unsigned char mac[POLY1305_TAG_LEN];
 	unsigned char poly1305_key[32];
-	size_t out_len;
 	poly1305_state poly1305;
 	const uint64_t in_len_64 = in_len;
+	size_t plaintext_len;
 
 	if (in_len < c20_ctx->tag_len) {
 		EVPerr(EVP_F_AEAD_CHACHA20_POLY1305_OPEN, EVP_R_BAD_DECRYPT);
-		return -1;
+		return 0;
 	}
 
 	/* The underlying ChaCha implementation may not overflow the block
@@ -200,22 +204,22 @@ aead_chacha20_poly1305_open(const EVP_AEAD_CTX *ctx, unsigned char *out,
 	 * 32-bits and this produces a warning because it's always false.
 	 * Casting to uint64_t inside the conditional is not sufficient to stop
 	 * the warning. */
-	if (in_len_64 >= (1ull << 32)*64 - 64) {
+	if (in_len_64 >= (1ULL << 32) * 64 - 64) {
 		EVPerr(EVP_F_AEAD_CHACHA20_POLY1305_OPEN, EVP_R_TOO_LARGE);
-		return -1;
+		return 0;
 	}
 
 	if (nonce_len != CHACHA20_NONCE_LEN) {
 		EVPerr(EVP_F_AEAD_CHACHA20_POLY1305_OPEN, EVP_R_IV_TOO_LARGE);
-		return -1;
+		return 0;
 	}
 
-	out_len = in_len - c20_ctx->tag_len;
+	plaintext_len = in_len - c20_ctx->tag_len;
 
-	if (max_out_len < out_len) {
+	if (max_out_len < plaintext_len) {
 		EVPerr(EVP_F_AEAD_CHACHA20_POLY1305_OPEN,
 		    EVP_R_BUFFER_TOO_SMALL);
-		return -1;
+		return 0;
 	}
 
 	memset(poly1305_key, 0, sizeof(poly1305_key));
@@ -224,17 +228,17 @@ aead_chacha20_poly1305_open(const EVP_AEAD_CTX *ctx, unsigned char *out,
 
 	CRYPTO_poly1305_init(&poly1305, poly1305_key);
 	poly1305_update_with_length(&poly1305, ad, ad_len);
-	poly1305_update_with_length(&poly1305, in, out_len);
+	poly1305_update_with_length(&poly1305, in, plaintext_len);
 	CRYPTO_poly1305_finish(&poly1305, mac);
 
-	if (CRYPTO_memcmp(mac, in + out_len, c20_ctx->tag_len) != 0) {
+	if (CRYPTO_memcmp(mac, in + plaintext_len, c20_ctx->tag_len) != 0) {
 		EVPerr(EVP_F_AEAD_CHACHA20_POLY1305_OPEN, EVP_R_BAD_DECRYPT);
-		return -1;
+		return 0;
 	}
 
-	CRYPTO_chacha_20(out, in, out_len, c20_ctx->key, nonce, 1);
-
-	return out_len;
+	CRYPTO_chacha_20(out, in, plaintext_len, c20_ctx->key, nonce, 1);
+	*out_len = plaintext_len;
+	return 1;
 }
 
 static const EVP_AEAD aead_chacha20_poly1305 = {
diff --git a/lib/libssl/src/crypto/evp/evp.h b/lib/libssl/src/crypto/evp/evp.h
index 5cd125894fc..3bd36e92661 100644
--- a/lib/libssl/src/crypto/evp/evp.h
+++ b/lib/libssl/src/crypto/evp/evp.h
@@ -1258,49 +1258,51 @@ int EVP_AEAD_CTX_init(EVP_AEAD_CTX *ctx, const EVP_AEAD *aead,
 void EVP_AEAD_CTX_cleanup(EVP_AEAD_CTX *ctx);
 
 /* EVP_AEAD_CTX_seal encrypts and authenticates the input and authenticates
- * any additional data (AD). The result is written as output, with the number
- * of bytes written being returned, or -1 on error.
+ * any additional data (AD), the result being written as output. One is
+ * returned on success, otherwise zero.
  *
  * This function may be called (with the same EVP_AEAD_CTX) concurrently with
  * itself or EVP_AEAD_CTX_open.
  *
  * At most max_out_len bytes are written as output and, in order to ensure
  * success, this value should be the length of the input plus the result of
- * EVP_AEAD_overhead.
+ * EVP_AEAD_overhead. On successful return, out_len is set to the actual
+ * number of bytes written.
  *
  * The length of the nonce is must be equal to the result of
  * EVP_AEAD_nonce_length for this AEAD.
  *
  * EVP_AEAD_CTX_seal never results in a partial output. If max_out_len is
- * insufficient, -1 will be returned.
+ * insufficient, zero will be returned and out_len will be set to zero.
  *
  * If the input and output are aliased then out must be <= in. */
-ssize_t EVP_AEAD_CTX_seal(const EVP_AEAD_CTX *ctx, unsigned char *out,
-    size_t max_out_len, const unsigned char *nonce, size_t nonce_len,
-    const unsigned char *in, size_t in_len, const unsigned char *ad,
-    size_t ad_len);
+int EVP_AEAD_CTX_seal(const EVP_AEAD_CTX *ctx, unsigned char *out,
+    size_t *out_len, size_t max_out_len, const unsigned char *nonce,
+    size_t nonce_len, const unsigned char *in, size_t in_len,
+    const unsigned char *ad, size_t ad_len);
 
 /* EVP_AEAD_CTX_open authenticates the input and additional data, decrypting
- * the input and writing it as output. The number of bytes decrypted and
- * written as output is returned, or -1 on error.
+ * the input and writing it as output. One is returned on success, otherwise
+ * zero.
  *
  * This function may be called (with the same EVP_AEAD_CTX) concurrently with
  * itself or EVP_AEAD_CTX_seal.
  *
  * At most the number of input bytes are written as output. In order to ensure
- * success, max_out_len should be at least the same as the input length.
+ * success, max_out_len should be at least the same as the input length. On
+ * successful return out_len is set to the actual number of bytes written.
  *
  * The length of nonce must be equal to the result of EVP_AEAD_nonce_length
  * for this AEAD.
  *
  * EVP_AEAD_CTX_open never results in a partial output. If max_out_len is
- * insufficient, -1 will be returned.
+ * insufficient, zero will be returned and out_len will be set to zero.
  *
  * If the input and output are aliased then out must be <= in. */
-ssize_t EVP_AEAD_CTX_open(const EVP_AEAD_CTX *ctx, unsigned char *out,
-    size_t max_out_len, const unsigned char *nonce, size_t nonce_len,
-    const unsigned char *in, size_t in_len, const unsigned char *ad,
-    size_t ad_len);
+int EVP_AEAD_CTX_open(const EVP_AEAD_CTX *ctx, unsigned char *out,
+    size_t *out_len, size_t max_out_len, const unsigned char *nonce,
+    size_t nonce_len, const unsigned char *in, size_t in_len,
+    const unsigned char *ad, size_t ad_len);
 
 void EVP_add_alg_module(void);
 
diff --git a/lib/libssl/src/crypto/evp/evp_aead.c b/lib/libssl/src/crypto/evp/evp_aead.c
index c8ba1df54a2..427bf054677 100644
--- a/lib/libssl/src/crypto/evp/evp_aead.c
+++ b/lib/libssl/src/crypto/evp/evp_aead.c
@@ -126,67 +126,59 @@ check_alias(const unsigned char *in, size_t in_len, const unsigned char *out)
 	return 0;
 }
 
-ssize_t
-EVP_AEAD_CTX_seal(const EVP_AEAD_CTX *ctx, unsigned char *out,
+int
+EVP_AEAD_CTX_seal(const EVP_AEAD_CTX *ctx, unsigned char *out, size_t *out_len,
     size_t max_out_len, const unsigned char *nonce, size_t nonce_len,
     const unsigned char *in, size_t in_len, const unsigned char *ad,
     size_t ad_len)
 {
 	size_t possible_out_len = in_len + ctx->aead->overhead;
-	ssize_t r;
 
-	if (possible_out_len < in_len /* overflow */ ||
-	    possible_out_len > SSIZE_MAX /* return value cannot be
-					    represented */) {
-		EVPerr(EVP_F_EVP_AEAD_CTX_SEAL, EVP_R_TOO_LARGE);
+	/* Overflow. */
+	if (possible_out_len < in_len) {
+		EVPerr(EVP_F_AEAD_CTX_SEAL, EVP_R_TOO_LARGE);
 		goto error;
 	}
 
 	if (!check_alias(in, in_len, out)) {
-		EVPerr(EVP_F_EVP_AEAD_CTX_SEAL, EVP_R_OUTPUT_ALIASES_INPUT);
+		EVPerr(EVP_F_AEAD_CTX_SEAL, EVP_R_OUTPUT_ALIASES_INPUT);
 		goto error;
 	}
 
-	r = ctx->aead->seal(ctx, out, max_out_len, nonce, nonce_len,
-	    in, in_len, ad, ad_len);
-	if (r >= 0)
-		return r;
+	if (ctx->aead->seal(ctx, out, out_len, max_out_len, nonce, nonce_len,
+	    in, in_len, ad, ad_len)) {
+		return 1;
+	}
 
 error:
 	/* In the event of an error, clear the output buffer so that a caller
 	 * that doesn't check the return value doesn't send raw data. */
 	memset(out, 0, max_out_len);
-	return -1;
+	*out_len = 0;
+	return 0;
 }
 
-ssize_t
-EVP_AEAD_CTX_open(const EVP_AEAD_CTX *ctx, unsigned char *out,
+int
+EVP_AEAD_CTX_open(const EVP_AEAD_CTX *ctx, unsigned char *out, size_t *out_len,
     size_t max_out_len, const unsigned char *nonce, size_t nonce_len,
     const unsigned char *in, size_t in_len, const unsigned char *ad,
     size_t ad_len)
 {
-	ssize_t r;
-
-	if (in_len > SSIZE_MAX) {
-		EVPerr(EVP_F_EVP_AEAD_CTX_OPEN, EVP_R_TOO_LARGE);
-		goto error;  /* may not be able to represent return value. */
-	}
-
 	if (!check_alias(in, in_len, out)) {
-		EVPerr(EVP_F_EVP_AEAD_CTX_OPEN, EVP_R_OUTPUT_ALIASES_INPUT);
+		EVPerr(EVP_F_AEAD_CTX_OPEN, EVP_R_OUTPUT_ALIASES_INPUT);
 		goto error;
 	}
 
-	r = ctx->aead->open(ctx, out, max_out_len, nonce, nonce_len,
-	    in, in_len, ad, ad_len);
-
-	if (r >= 0)
-		return r;
+	if (ctx->aead->open(ctx, out, out_len, max_out_len, nonce, nonce_len,
+	    in, in_len, ad, ad_len)) {
+		return 1;
+	}
 
 error:
 	/* In the event of an error, clear the output buffer so that a caller
 	 * that doesn't check the return value doesn't try and process bad
 	 * data. */
 	memset(out, 0, max_out_len);
-	return -1;
+	*out_len = 0;
+	return 0;
 }
diff --git a/lib/libssl/src/crypto/evp/evp_locl.h b/lib/libssl/src/crypto/evp/evp_locl.h
index 6f9218eafc9..3083f30975d 100644
--- a/lib/libssl/src/crypto/evp/evp_locl.h
+++ b/lib/libssl/src/crypto/evp/evp_locl.h
@@ -354,13 +354,13 @@ struct evp_aead_st {
 	    size_t key_len, size_t tag_len);
 	void (*cleanup)(struct evp_aead_ctx_st*);
 
-	ssize_t (*seal)(const struct evp_aead_ctx_st *ctx, unsigned char *out,
-	    size_t max_out_len, const unsigned char *nonce, size_t nonce_len,
-	    const unsigned char *in, size_t in_len, const unsigned char *ad,
-	    size_t ad_len);
-
-	ssize_t (*open)(const struct evp_aead_ctx_st *ctx, unsigned char *out,
-	    size_t max_out_len, const unsigned char *nonce, size_t nonce_len,
-	    const unsigned char *in, size_t in_len, const unsigned char *ad,
-	    size_t ad_len);
+	int (*seal)(const struct evp_aead_ctx_st *ctx, unsigned char *out,
+	    size_t *out_len, size_t max_out_len, const unsigned char *nonce,
+	    size_t nonce_len, const unsigned char *in, size_t in_len,
+	    const unsigned char *ad, size_t ad_len);
+
+	int (*open)(const struct evp_aead_ctx_st *ctx, unsigned char *out,
+	    size_t *out_len, size_t max_out_len, const unsigned char *nonce,
+	    size_t nonce_len, const unsigned char *in, size_t in_len,
+	    const unsigned char *ad, size_t ad_len);
 };