Fix EVP_PKEY_FLAG_AUTOARGLEN behavior.

Converting check_autoarg from a macro to a function lost the behavior. Instead,
just move the logic into p_rsa.c which was the only EVP_PKEY implementation
that even needed the flag.

Also document this behavior on each of the functions. Make note of the out =
NULL case only returning the maximum output size, and not necessarily the
actual size.

For testing, update example_sign to determine the signature size using the NULL
behavior rather than querying the RSA key.

Change-Id: Iec6c2862028a5cfdefe8faa0e8c471755070898a
Reviewed-on: https://boringssl-review.googlesource.com/1121
Reviewed-by: Adam Langley <agl@google.com>

6 files changed, 86 insertions, 74 deletions

diff --git a/crypto/evp/evp.h b/crypto/evp/evp.h
index c6f5ab31..91b1a376 100644
--- a/crypto/evp/evp.h
+++ b/crypto/evp/evp.h
@@ -422,10 +422,13 @@ int EVP_PKEY_CTX_ctrl(EVP_PKEY_CTX *ctx, int keytype, int optype, int cmd,
 int EVP_PKEY_sign_init(EVP_PKEY_CTX *ctx);
 
 /* EVP_PKEY_sign signs |data_len| bytes from |data| using |ctx|. If |sig| is
- * NULL, the size of the signature is written to |out_sig_len|. Otherwise,
- * |*sig_len| must contain the number of bytes of space available at |sig|. If
- * sufficient, the signature will be written to |sig| and |*sig_len| updated
- * with the true length.
+ * NULL, the maximum size of the signature is written to
+ * |out_sig_len|. Otherwise, |*sig_len| must contain the number of bytes of
+ * space available at |sig|. If sufficient, the signature will be written to
+ * |sig| and |*sig_len| updated with the true length.
+ *
+ * WARNING: Setting |out| to NULL only gives the maximum size of the
+ * plaintext. The actual plaintext may be smaller.
  *
  * It returns one on success or zero on error. (Note: this differs from
  * OpenSSL, which can also return negative values to indicate an error. ) */
@@ -454,8 +457,14 @@ int EVP_PKEY_verify(EVP_PKEY_CTX *ctx, const uint8_t *sig, size_t sig_len,
  * usual return value convention. */
 int EVP_PKEY_encrypt_init(EVP_PKEY_CTX *ctx);
 
-/* EVP_PKEY_encrypt encrypts |in_len| bytes from |in| and writes it to |out|.
- * TODO(fork): need more details on |out_len|.
+/* EVP_PKEY_encrypt encrypts |in_len| bytes from |in|. If |out| is NULL, the
+ * maximum size of the ciphertext is written to |out_len|. Otherwise, |*out_len|
+ * must contain the number of bytes of space available at |out|. If sufficient,
+ * the ciphertext will be written to |out| and |*out_len| updated with the true
+ * length.
+ *
+ * WARNING: Setting |out| to NULL only gives the maximum size of the
+ * ciphertext. The actual ciphertext may be smaller.
  *
  * It returns one on success or <= 0 on error. (Note: this differs from
  * OpenSSL, which can also return negative values to indicate an error. ) */
@@ -469,8 +478,14 @@ int EVP_PKEY_encrypt(EVP_PKEY_CTX *ctx, uint8_t *out, size_t *out_len,
  * usual return value convention. */
 int EVP_PKEY_decrypt_init(EVP_PKEY_CTX *ctx);
 
-/* EVP_PKEY_decrypt decrypts |in_len| bytes from |in|, writes it to |out| and
- * sets |*outlen| to the number of bytes written.
+/* EVP_PKEY_decrypt decrypts |in_len| bytes from |in|. If |out| is NULL, the
+ * maximum size of the plaintext is written to |out_len|. Otherwise, |*out_len|
+ * must contain the number of bytes of space available at |out|. If sufficient,
+ * the ciphertext will be written to |out| and |*out_len| updated with the true
+ * length.
+ *
+ * WARNING: Setting |out| to NULL only gives the maximum size of the
+ * plaintext. The actual plaintext may be smaller.
  *
  * It returns one on success or <= 0 on error. (Note: this differs from
  * OpenSSL, which can also return negative values to indicate an error. ) */
@@ -496,7 +511,10 @@ int EVP_PKEY_derive_set_peer(EVP_PKEY_CTX *ctx, EVP_PKEY *peer);
  * |ctx|. If |key| is non-NULL then, on entry, |out_key_len| must contain the
  * amount of space at |key|. If sufficient then the shared key will be written
  * to |key| and |*out_key_len| will be set to the length. If |key| is NULL then
- * |out_key_len| will be set the length.
+ * |out_key_len| will be set to the maximum length.
+ *
+ * WARNING: Setting |out| to NULL only gives the maximum size of the key. The
+ * actual key may be smaller.
  *
  * It returns one on success and <= 0 on error. WARNING: this differs from the
  * usual return convention. */
@@ -772,6 +790,8 @@ struct evp_pkey_st {
 #define EVP_F_i2d_PublicKey 149
 #define EVP_F_rsa_pub_decode 150
 #define EVP_F_EVP_PKEY_get1_DSA 151
+#define EVP_F_pkey_rsa_encrypt 152
+#define EVP_F_pkey_rsa_decrypt 153
 #define EVP_R_UNSUPPORTED_PUBLIC_KEY_TYPE 100
 #define EVP_R_UNSUPPORTED_SIGNATURE_TYPE 101
 #define EVP_R_INVALID_DIGEST_TYPE 102
diff --git a/crypto/evp/evp_ctx.c b/crypto/evp/evp_ctx.c
index 5c3755aa..1365dae7 100644
--- a/crypto/evp/evp_ctx.c
+++ b/crypto/evp/evp_ctx.c
@@ -254,27 +254,6 @@ int EVP_PKEY_sign_init(EVP_PKEY_CTX *ctx) {
   return ret;
 }
 
-static int check_autoarg(const EVP_PKEY_CTX *ctx, const uint8_t *arg,
-                         size_t *arg_len) {
-  size_t size;
-
-  if (0 == (ctx->pmeth->flags & EVP_PKEY_FLAG_AUTOARGLEN)) {
-    return 1;
-  }
-
-  size = EVP_PKEY_size(ctx->pkey);
-  if (arg == NULL) {
-    *arg_len = size;
-    return 1;
-  }
-
-  if (*arg_len < size) {
-    return 0;
-  }
-
-  return 1;
-}
-
 int EVP_PKEY_sign(EVP_PKEY_CTX *ctx, uint8_t *sig, size_t *sig_len,
                   const uint8_t *data, size_t data_len) {
   if (!ctx || !ctx->pmeth || !ctx->pmeth->sign) {
@@ -286,11 +265,6 @@ int EVP_PKEY_sign(EVP_PKEY_CTX *ctx, uint8_t *sig, size_t *sig_len,
     OPENSSL_PUT_ERROR(EVP, EVP_PKEY_sign, EVP_R_OPERATON_NOT_INITIALIZED);
     return 0;
   }
-  if (!check_autoarg(ctx, sig, sig_len)) {
-    OPENSSL_PUT_ERROR(EVP, EVP_PKEY_sign,
-                      EVP_R_BUFFER_TOO_SMALL);
-    return 0;
-  }
   return ctx->pmeth->sign(ctx, sig, sig_len, data, data_len);
 }
 
@@ -356,10 +330,6 @@ int EVP_PKEY_encrypt(EVP_PKEY_CTX *ctx, uint8_t *out, size_t *outlen,
     OPENSSL_PUT_ERROR(EVP, EVP_PKEY_encrypt, EVP_R_OPERATON_NOT_INITIALIZED);
     return -1;
   }
-  if (!check_autoarg(ctx, out, outlen)) {
-    OPENSSL_PUT_ERROR(EVP, EVP_PKEY_encrypt, EVP_R_BUFFER_TOO_SMALL);
-    return 0;
-  }
   return ctx->pmeth->encrypt(ctx, out, outlen, in, inlen);
 }
 
@@ -393,10 +363,6 @@ int EVP_PKEY_decrypt(EVP_PKEY_CTX *ctx, uint8_t *out, size_t *outlen,
     OPENSSL_PUT_ERROR(EVP, EVP_PKEY_decrypt, EVP_R_OPERATON_NOT_INITIALIZED);
     return -1;
   }
-  if (!check_autoarg(ctx, out, outlen)) {
-    OPENSSL_PUT_ERROR(EVP, EVP_PKEY_decrypt, EVP_R_BUFFER_TOO_SMALL);
-    return 0;
-  }
   return ctx->pmeth->decrypt(ctx, out, outlen, in, inlen);
 }
 
@@ -493,10 +459,6 @@ int EVP_PKEY_derive(EVP_PKEY_CTX *ctx, uint8_t *key, size_t *out_key_len) {
     OPENSSL_PUT_ERROR(EVP, EVP_PKEY_derive, EVP_R_OPERATON_NOT_INITIALIZED);
     return -1;
   }
-  if (!check_autoarg(ctx, key, out_key_len)) {
-    OPENSSL_PUT_ERROR(EVP, EVP_PKEY_derive, EVP_R_BUFFER_TOO_SMALL);
-    return 0;
-  }
   return ctx->pmeth->derive(ctx, key, out_key_len);
 }
 
diff --git a/crypto/evp/evp_error.c b/crypto/evp/evp_error.c
index 0b4d2f2d..c349d303 100644
--- a/crypto/evp/evp_error.c
+++ b/crypto/evp/evp_error.c
@@ -61,6 +61,8 @@ const ERR_STRING_DATA EVP_error_string_data[] = {
   {ERR_PACK(ERR_LIB_EVP, EVP_F_pkey_ec_paramgen, 0), "pkey_ec_paramgen"},
   {ERR_PACK(ERR_LIB_EVP, EVP_F_pkey_ec_sign, 0), "pkey_ec_sign"},
   {ERR_PACK(ERR_LIB_EVP, EVP_F_pkey_rsa_ctrl, 0), "pkey_rsa_ctrl"},
+  {ERR_PACK(ERR_LIB_EVP, EVP_F_pkey_rsa_decrypt, 0), "pkey_rsa_decrypt"},
+  {ERR_PACK(ERR_LIB_EVP, EVP_F_pkey_rsa_encrypt, 0), "pkey_rsa_encrypt"},
   {ERR_PACK(ERR_LIB_EVP, EVP_F_pkey_rsa_sign, 0), "pkey_rsa_sign"},
   {ERR_PACK(ERR_LIB_EVP, EVP_F_rsa_algor_to_md, 0), "rsa_algor_to_md"},
   {ERR_PACK(ERR_LIB_EVP, EVP_F_rsa_item_verify, 0), "rsa_item_verify"},
diff --git a/crypto/evp/example_sign.c b/crypto/evp/example_sign.c
index 4bbe1942..df7156bb 100644
--- a/crypto/evp/example_sign.c
+++ b/crypto/evp/example_sign.c
@@ -101,7 +101,7 @@ int example_EVP_DigestSignInit() {
   RSA *rsa = NULL;
   const uint8_t *derp = kExampleRSAKeyDER;
   uint8_t *sig = NULL;
-  size_t sig_len;
+  size_t sig_len = 0;
   EVP_MD_CTX md_ctx;
 
   EVP_MD_CTX_init(&md_ctx);
@@ -110,18 +110,29 @@ int example_EVP_DigestSignInit() {
     goto out;
   }
 
-  sig_len = RSA_size(rsa);
-
   pkey = EVP_PKEY_new();
   sig = malloc(sig_len);
   if (pkey == NULL ||
-      sig == NULL ||
       !EVP_PKEY_set1_RSA(pkey, rsa) ||
       EVP_DigestSignInit(&md_ctx, NULL, EVP_sha256(), NULL, pkey) != 1 ||
-      EVP_DigestSignUpdate(&md_ctx, kMsg, sizeof(kMsg)) != 1 ||
+      EVP_DigestSignUpdate(&md_ctx, kMsg, sizeof(kMsg)) != 1) {
+    goto out;
+  }
+  /* Determine the size of the signature. */
+  if (EVP_DigestSignFinal(&md_ctx, NULL, &sig_len) != 1) {
+    goto out;
+  }
+  /* Sanity check for testing. */
+  if (sig_len != RSA_size(rsa)) {
+    fprintf(stderr, "sig_len mismatch\n");
+    goto out;
+  }
+  sig = malloc(sig_len);
+  if (sig == NULL ||
       EVP_DigestSignFinal(&md_ctx, sig, &sig_len) != 1) {
     goto out;
   }
+
   ret = 1;
 
 out:
diff --git a/crypto/evp/internal.h b/crypto/evp/internal.h
index 76f206e7..8561960d 100644
--- a/crypto/evp/internal.h
+++ b/crypto/evp/internal.h
@@ -175,11 +175,6 @@ struct evp_pkey_ctx_st {
   void *app_data;
 } /* EVP_PKEY_CTX */;
 
-/* EVP_PKEY_FLAG_AUTOARGLEN causes wrapper functions to automatically check the
- * argument length to various functions (signing, decrypting etc) is equal to
- * the value of |EVP_PKEY_size|. */
-#define EVP_PKEY_FLAG_AUTOARGLEN 2
-
 struct evp_pkey_method_st {
   int pkey_id;
   int flags;
diff --git a/crypto/evp/p_rsa.c b/crypto/evp/p_rsa.c
index 0128950e..4858b908 100644
--- a/crypto/evp/p_rsa.c
+++ b/crypto/evp/p_rsa.c
@@ -172,6 +172,14 @@ static int pkey_rsa_sign(EVP_PKEY_CTX *ctx, uint8_t *sig, size_t *siglen,
   RSA_PKEY_CTX *rctx = ctx->data;
   RSA *rsa = ctx->pkey->pkey.rsa;
 
+  if (!sig) {
+    *siglen = RSA_size(rsa);
+    return 1;
+  } else if (*siglen < (size_t)RSA_size(rsa)) {
+    OPENSSL_PUT_ERROR(EVP, pkey_rsa_sign, EVP_R_BUFFER_TOO_SMALL);
+    return 0;
+  }
+
   if (rctx->md) {
     if (tbslen != EVP_MD_size(rctx->md)) {
       OPENSSL_PUT_ERROR(EVP, pkey_rsa_sign, EVP_R_INVALID_DIGEST_LENGTH);
@@ -266,9 +274,18 @@ static int pkey_rsa_encrypt(EVP_PKEY_CTX *ctx, uint8_t *out, size_t *outlen,
                             const uint8_t *in, size_t inlen) {
   int ret;
   RSA_PKEY_CTX *rctx = ctx->data;
+  RSA *rsa = ctx->pkey->pkey.rsa;
+
+  if (!out) {
+    *outlen = RSA_size(rsa);
+    return 1;
+  } else if (*outlen < (size_t)RSA_size(rsa)) {
+    OPENSSL_PUT_ERROR(EVP, pkey_rsa_encrypt, EVP_R_BUFFER_TOO_SMALL);
+    return 0;
+  }
 
   if (rctx->pad_mode == RSA_PKCS1_OAEP_PADDING) {
-    int klen = RSA_size(ctx->pkey->pkey.rsa);
+    int klen = RSA_size(rsa);
     if (!setup_tbuf(rctx, ctx)) {
       return -1;
     }
@@ -277,11 +294,9 @@ static int pkey_rsa_encrypt(EVP_PKEY_CTX *ctx, uint8_t *out, size_t *outlen,
                                          rctx->md, rctx->mgf1md)) {
       return -1;
     }
-    ret = RSA_public_encrypt(klen, rctx->tbuf, out, ctx->pkey->pkey.rsa,
-                             RSA_NO_PADDING);
+    ret = RSA_public_encrypt(klen, rctx->tbuf, out, rsa, RSA_NO_PADDING);
   } else {
-    ret =
-        RSA_public_encrypt(inlen, in, out, ctx->pkey->pkey.rsa, rctx->pad_mode);
+    ret = RSA_public_encrypt(inlen, in, out, rsa, rctx->pad_mode);
   }
 
   if (ret < 0) {
@@ -297,13 +312,21 @@ static int pkey_rsa_decrypt(EVP_PKEY_CTX *ctx, uint8_t *out,
                             size_t inlen) {
   int ret;
   RSA_PKEY_CTX *rctx = ctx->data;
+  RSA *rsa = ctx->pkey->pkey.rsa;
+
+  if (!out) {
+    *outlen = RSA_size(rsa);
+    return 1;
+  } else if (*outlen < (size_t)RSA_size(rsa)) {
+    OPENSSL_PUT_ERROR(EVP, pkey_rsa_decrypt, EVP_R_BUFFER_TOO_SMALL);
+    return 0;
+  }
 
   if (rctx->pad_mode == RSA_PKCS1_OAEP_PADDING) {
     if (!setup_tbuf(rctx, ctx)) {
       return -1;
     }
-    ret = RSA_private_decrypt(inlen, in, rctx->tbuf, ctx->pkey->pkey.rsa,
-                              RSA_NO_PADDING);
+    ret = RSA_private_decrypt(inlen, in, rctx->tbuf, rsa, RSA_NO_PADDING);
     if (ret <= 0) {
       return ret;
     }
@@ -311,8 +334,7 @@ static int pkey_rsa_decrypt(EVP_PKEY_CTX *ctx, uint8_t *out,
         out, ret, rctx->tbuf, ret, rctx->oaep_label,
         rctx->oaep_labellen, rctx->md, rctx->mgf1md);
   } else {
-    ret = RSA_private_decrypt(inlen, in, out, ctx->pkey->pkey.rsa,
-                              rctx->pad_mode);
+    ret = RSA_private_decrypt(inlen, in, out, rsa, rctx->pad_mode);
   }
 
   if (ret < 0) {
@@ -507,14 +529,14 @@ static int pkey_rsa_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey) {
 }
 
 const EVP_PKEY_METHOD rsa_pkey_meth = {
-    EVP_PKEY_RSA,        EVP_PKEY_FLAG_AUTOARGLEN, pkey_rsa_init,
-    pkey_rsa_copy,       pkey_rsa_cleanup,         0 /* paramgen_init */,
-    0 /* paramgen */,    0 /* keygen_init */,      pkey_rsa_keygen,
-    0 /* sign_init */,   pkey_rsa_sign,            0 /* verify_init */,
-    pkey_rsa_verify,     0,                        0,
-    0,                   0,                        0 /* encrypt_init */,
-    pkey_rsa_encrypt,    0 /* decrypt_init */,     pkey_rsa_decrypt,
-    0 /* derive_init */, 0 /* derive */,           pkey_rsa_ctrl,
+    EVP_PKEY_RSA,           0 /* flags */,        pkey_rsa_init,
+    pkey_rsa_copy,          pkey_rsa_cleanup,     0 /* paramgen_init */,
+    0 /* paramgen */,       0 /* keygen_init */,  pkey_rsa_keygen,
+    0 /* sign_init */,      pkey_rsa_sign,        0 /* verify_init */,
+    pkey_rsa_verify,        0 /* signctx_init */, 0 /* signctx */,
+    0 /* verifyctx_init */, 0 /* verifyctx */,    0 /* encrypt_init */,
+    pkey_rsa_encrypt,       0 /* decrypt_init */, pkey_rsa_decrypt,
+    0 /* derive_init */,    0 /* derive */,       pkey_rsa_ctrl,
 };
 
 int EVP_PKEY_CTX_set_rsa_padding(EVP_PKEY_CTX *ctx, int padding) {