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>
diff --git a/crypto/evp/evp.h b/crypto/evp/evp.h
index c6f5ab3..91b1a37 100644
--- a/crypto/evp/evp.h
+++ b/crypto/evp/evp.h
@@ -422,10 +422,13 @@
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 @@
* 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 @@
* 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 @@
* |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 @@
#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 5c3755a..1365dae 100644
--- a/crypto/evp/evp_ctx.c
+++ b/crypto/evp/evp_ctx.c
@@ -254,27 +254,6 @@
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 @@
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 @@
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 @@
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 @@
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 0b4d2f2..c349d30 100644
--- a/crypto/evp/evp_error.c
+++ b/crypto/evp/evp_error.c
@@ -61,6 +61,8 @@
{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 4bbe194..df7156b 100644
--- a/crypto/evp/example_sign.c
+++ b/crypto/evp/example_sign.c
@@ -101,7 +101,7 @@
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 @@
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 76f206e..8561960 100644
--- a/crypto/evp/internal.h
+++ b/crypto/evp/internal.h
@@ -175,11 +175,6 @@
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 0128950..4858b90 100644
--- a/crypto/evp/p_rsa.c
+++ b/crypto/evp/p_rsa.c
@@ -172,6 +172,14 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
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) {
