From 0c20802c6a6008b28bfb0eac67d69f536edc60a7 Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni Date: Tue, 2 Feb 2016 00:37:41 -0500 Subject: [PATCH] Fix pkeyutl/rsautl empty encrypt-input/decrypt-output handling Also fix option processing in pkeyutl to allow use of (formerly) "out-of-order" switches that were needless implementation limitations. Handle documented "ENGINE" form with -keyform and -peerform. Better handling of OPENSSL_NO_ENGINE and OPENSSL_NO_RSA. RT2018 Reviewed-by: Rich Salz --- apps/apps.c | 43 +++++++++++++----- apps/apps.h | 3 +- apps/opt.c | 27 +++++++---- apps/pkeyutl.c | 105 ++++++++++++++++++++++++++----------------- apps/rsautl.c | 9 ++-- doc/apps/pkeyutl.pod | 8 ++-- doc/apps/rsautl.pod | 5 +++ 7 files changed, 128 insertions(+), 72 deletions(-) diff --git a/apps/apps.c b/apps/apps.c index 9b55f820e1..7a4608f75c 100644 --- a/apps/apps.c +++ b/apps/apps.c @@ -763,20 +763,22 @@ EVP_PKEY *load_key(const char *file, int format, int maybe_stdin, BIO_printf(bio_err, "no keyfile specified\n"); goto end; } -#ifndef OPENSSL_NO_ENGINE if (format == FORMAT_ENGINE) { - if (!e) + if (e == NULL) BIO_printf(bio_err, "no engine specified\n"); else { +#ifndef OPENSSL_NO_ENGINE pkey = ENGINE_load_private_key(e, file, ui_method, &cb_data); - if (!pkey) { + if (pkey == NULL) { BIO_printf(bio_err, "cannot load %s from engine\n", key_descrip); ERR_print_errors(bio_err); } +#else + BIO_printf(bio_err, "engines not supported\n"); +#endif } goto end; } -#endif if (file == NULL && maybe_stdin) { unbuffer(stdin); key = dup_bio_in(format); @@ -831,15 +833,22 @@ EVP_PKEY *load_pubkey(const char *file, int format, int maybe_stdin, BIO_printf(bio_err, "no keyfile specified\n"); goto end; } -#ifndef OPENSSL_NO_ENGINE if (format == FORMAT_ENGINE) { - if (!e) + if (e == NULL) BIO_printf(bio_err, "no engine specified\n"); - else + else { +#ifndef OPENSSL_NO_ENGINE pkey = ENGINE_load_public_key(e, file, ui_method, &cb_data); + if (pkey == NULL) { + BIO_printf(bio_err, "cannot load %s from engine\n", key_descrip); + ERR_print_errors(bio_err); + } +#else + BIO_printf(bio_err, "engines not supported\n"); +#endif + } goto end; } -#endif if (file == NULL && maybe_stdin) { unbuffer(stdin); key = dup_bio_in(format); @@ -850,8 +859,8 @@ EVP_PKEY *load_pubkey(const char *file, int format, int maybe_stdin, if (format == FORMAT_ASN1) { pkey = d2i_PUBKEY_bio(key, NULL); } -#ifndef OPENSSL_NO_RSA else if (format == FORMAT_ASN1RSA) { +#ifndef OPENSSL_NO_RSA RSA *rsa; rsa = d2i_RSAPublicKey_bio(key, NULL); if (rsa) { @@ -860,8 +869,12 @@ EVP_PKEY *load_pubkey(const char *file, int format, int maybe_stdin, EVP_PKEY_set1_RSA(pkey, rsa); RSA_free(rsa); } else +#else + BIO_printf(bio_err, "RSA keys not supported\n"); +#endif pkey = NULL; } else if (format == FORMAT_PEMRSA) { +#ifndef OPENSSL_NO_RSA RSA *rsa; rsa = PEM_read_bio_RSAPublicKey(key, NULL, (pem_password_cb *)password_callback, @@ -872,9 +885,11 @@ EVP_PKEY *load_pubkey(const char *file, int format, int maybe_stdin, EVP_PKEY_set1_RSA(pkey, rsa); RSA_free(rsa); } else +#else + BIO_printf(bio_err, "RSA keys not supported\n"); +#endif pkey = NULL; } -#endif else if (format == FORMAT_PEM) { pkey = PEM_read_bio_PUBKEY(key, NULL, (pem_password_cb *)password_callback, @@ -1907,7 +1922,11 @@ int bio_to_mem(unsigned char **out, int maxlen, BIO *in) else len = 1024; len = BIO_read(in, tbuf, len); - if (len <= 0) + if (len < 0) { + BIO_free(mem); + return -1; + } + if (len == 0) break; if (BIO_write(mem, tbuf, len) != len) { BIO_free(mem); @@ -1924,7 +1943,7 @@ int bio_to_mem(unsigned char **out, int maxlen, BIO *in) return ret; } -int pkey_ctrl_string(EVP_PKEY_CTX *ctx, char *value) +int pkey_ctrl_string(EVP_PKEY_CTX *ctx, const char *value) { int rv; char *stmp, *vtmp = NULL; diff --git a/apps/apps.h b/apps/apps.h index 5ea148d9f3..93172b5eb0 100644 --- a/apps/apps.h +++ b/apps/apps.h @@ -382,6 +382,7 @@ typedef struct string_int_pair_st { # define OPT_FMT_TEXT (1L << 8) # define OPT_FMT_HTTP (1L << 9) # define OPT_FMT_PVK (1L << 10) +# define OPT_FMT_PDE (OPT_FMT_PEMDER | OPT_FMT_ENGINE) # define OPT_FMT_ANY ( \ OPT_FMT_PEMDER | OPT_FMT_PKCS12 | OPT_FMT_SMIME | \ OPT_FMT_ENGINE | OPT_FMT_MSBLOB | OPT_FMT_NETSCAPE | \ @@ -522,7 +523,7 @@ int args_verify(char ***pargs, int *pargc, int *badarg, X509_VERIFY_PARAM **pm); void policies_print(X509_STORE_CTX *ctx); int bio_to_mem(unsigned char **out, int maxlen, BIO *in); -int pkey_ctrl_string(EVP_PKEY_CTX *ctx, char *value); +int pkey_ctrl_string(EVP_PKEY_CTX *ctx, const char *value); int init_gen_str(EVP_PKEY_CTX **pctx, const char *algname, ENGINE *e, int do_param); int do_X509_sign(X509 *x, EVP_PKEY *pkey, const EVP_MD *md, diff --git a/apps/opt.c b/apps/opt.c index 2fbc9fe8b2..badff26aac 100644 --- a/apps/opt.c +++ b/apps/opt.c @@ -182,8 +182,9 @@ char *opt_init(int ac, char **av, const OPTIONS *o) assert(o->name[0] != '-'); assert(o->retval > 0); switch (i) { - case 0: case '-': case '/': case '<': case '>': case 'F': case 'M': - case 'L': case 'U': case 'f': case 'n': case 'p': case 's': case 'u': + case 0: case '-': case '/': case '<': case '>': case 'E': case 'F': + case 'M': case 'U': case 'f': case 'l': case 'n': case 'p': case 's': + case 'u': break; default: assert(0); @@ -734,7 +735,7 @@ int opt_next(void) return -1; } break; - case 'L': + case 'l': if (!opt_long(arg, &lval)) { BIO_printf(bio_err, "%s: Invalid number \"%s\" for -%s\n", @@ -750,9 +751,11 @@ int opt_next(void) return -1; } break; - case 'f': + case 'E': case 'F': + case 'f': if (opt_format(arg, + o->valtype == 'E' ? OPT_FMT_PDE : o->valtype == 'F' ? OPT_FMT_PEMDER : OPT_FMT_ANY, &ival)) break; @@ -823,15 +826,23 @@ static const char *valtype2param(const OPTIONS *o) case '>': return "outfile"; case 'p': - return "pnum"; + return "+int"; case 'n': - return "num"; + return "int"; + case 'l': + return "long"; case 'u': - return "unum"; + return "ulong"; + case 'E': + return "PEM|DER|ENGINE"; case 'F': - return "der/pem"; + return "PEM|DER"; case 'f': return "format"; + case 'M': + return "intmax"; + case 'U': + return "uintmax"; } return "parm"; } diff --git a/apps/pkeyutl.c b/apps/pkeyutl.c index 362415e960..03febd5c77 100644 --- a/apps/pkeyutl.c +++ b/apps/pkeyutl.c @@ -67,10 +67,11 @@ #define KEY_CERT 3 static EVP_PKEY_CTX *init_ctx(int *pkeysize, - char *keyfile, int keyform, int key_type, + const char *keyfile, int keyform, int key_type, char *passinarg, int pkey_op, ENGINE *e); -static int setup_peer(EVP_PKEY_CTX *ctx, int peerform, const char *file); +static int setup_peer(EVP_PKEY_CTX *ctx, int peerform, const char *file, + ENGINE *e); static int do_keyop(EVP_PKEY_CTX *ctx, int pkey_op, unsigned char *out, size_t *poutlen, @@ -91,22 +92,22 @@ OPTIONS pkeyutl_options[] = { {"out", OPT_OUT, '>', "Output file"}, {"pubin", OPT_PUBIN, '-', "Input is a public key"}, {"certin", OPT_CERTIN, '-', "Input is a cert with a public key"}, - {"asn1parse", OPT_ASN1PARSE, '-'}, + {"asn1parse", OPT_ASN1PARSE, '-', "asn1parse the output data"}, {"hexdump", OPT_HEXDUMP, '-', "Hex dump output"}, {"sign", OPT_SIGN, '-', "Sign with private key"}, {"verify", OPT_VERIFY, '-', "Verify with public key"}, {"verifyrecover", OPT_VERIFYRECOVER, '-', "Verify with public key, recover original data"}, - {"rev", OPT_REV, '-'}, + {"rev", OPT_REV, '-', "Reverse the input buffer"}, {"encrypt", OPT_ENCRYPT, '-', "Encrypt with public key"}, {"decrypt", OPT_DECRYPT, '-', "Decrypt with private key"}, {"derive", OPT_DERIVE, '-', "Derive shared secret"}, {"sigfile", OPT_SIGFILE, '<', "Signature file (verify operation only)"}, {"inkey", OPT_INKEY, 's', "Input key"}, - {"peerkey", OPT_PEERKEY, 's'}, + {"peerkey", OPT_PEERKEY, 's', "Peer key file used in key derivation"}, {"passin", OPT_PASSIN, 's', "Pass phrase source"}, - {"peerform", OPT_PEERFORM, 'F'}, - {"keyform", OPT_KEYFORM, 'F', "Private key format - default PEM"}, + {"peerform", OPT_PEERFORM, 'E', "Peer key format - default PEM"}, + {"keyform", OPT_KEYFORM, 'E', "Private key format - default PEM"}, {"pkeyopt", OPT_PKEYOPT, 's', "Public key options as opt:value"}, #ifndef OPENSSL_NO_ENGINE {"engine", OPT_ENGINE, 's', "Use engine, possibly a hardware device"}, @@ -128,6 +129,9 @@ int pkeyutl_main(int argc, char **argv) int keysize = -1, pkey_op = EVP_PKEY_OP_SIGN, key_type = KEY_PRIVKEY; int ret = 1, rv = -1; size_t buf_outlen; + const char *inkey = NULL; + const char *peerkey = NULL; + STACK_OF(OPENSSL_STRING) *pkeyopts = NULL; prog = opt_init(argc, argv, pkeyutl_options); while ((o = opt_next()) != OPT_EOF) { @@ -151,27 +155,20 @@ int pkeyutl_main(int argc, char **argv) sigfile = opt_arg(); break; case OPT_INKEY: - ctx = init_ctx(&keysize, opt_arg(), keyform, key_type, - passinarg, pkey_op, e); - if (ctx == NULL) { - BIO_puts(bio_err, "%s: Error initializing context\n"); - ERR_print_errors(bio_err); - goto opthelp; - } + inkey = opt_arg(); break; case OPT_PEERKEY: - if (!setup_peer(ctx, peerform, opt_arg())) - goto opthelp; + peerkey = opt_arg(); break; case OPT_PASSIN: passinarg = opt_arg(); break; case OPT_PEERFORM: - if (!opt_format(opt_arg(), OPT_FMT_PEMDER, &peerform)) + if (!opt_format(opt_arg(), OPT_FMT_PDE, &peerform)) goto opthelp; break; case OPT_KEYFORM: - if (!opt_format(opt_arg(), OPT_FMT_PEMDER, &keyform)) + if (!opt_format(opt_arg(), OPT_FMT_PDE, &keyform)) goto opthelp; break; case OPT_ENGINE: @@ -198,9 +195,6 @@ int pkeyutl_main(int argc, char **argv) case OPT_VERIFYRECOVER: pkey_op = EVP_PKEY_OP_VERIFYRECOVER; break; - case OPT_REV: - rev = 1; - break; case OPT_ENCRYPT: pkey_op = EVP_PKEY_OP_ENCRYPT; break; @@ -210,15 +204,14 @@ int pkeyutl_main(int argc, char **argv) case OPT_DERIVE: pkey_op = EVP_PKEY_OP_DERIVE; break; + case OPT_REV: + rev = 1; + break; case OPT_PKEYOPT: - if (ctx == NULL) { - BIO_printf(bio_err, - "%s: Must have -inkey before -pkeyopt\n", prog); - goto opthelp; - } - if (pkey_ctrl_string(ctx, opt_arg()) <= 0) { - BIO_printf(bio_err, "%s: Can't set parameter:\n", prog); - ERR_print_errors(bio_err); + if ((pkeyopts == NULL && + (pkeyopts = sk_OPENSSL_STRING_new_null()) == NULL) || + sk_OPENSSL_STRING_push(pkeyopts, *++argv) == 0) { + BIO_puts(bio_err, "out of memory\n"); goto end; } break; @@ -227,9 +220,37 @@ int pkeyutl_main(int argc, char **argv) argc = opt_num_rest(); argv = opt_rest(); - if (ctx == NULL) + if (inkey == NULL || + (peerkey != NULL && pkey_op != EVP_PKEY_OP_DERIVE)) goto opthelp; + ctx = init_ctx(&keysize, inkey, keyform, key_type, + passinarg, pkey_op, e); + if (ctx == NULL) { + BIO_printf(bio_err, "%s: Error initializing context\n", prog); + ERR_print_errors(bio_err); + goto end; + } + if (peerkey != NULL && !setup_peer(ctx, peerform, peerkey, e)) { + BIO_printf(bio_err, "%s: Error setting up peer key\n", prog); + ERR_print_errors(bio_err); + goto end; + } + if (pkeyopts != NULL) { + int num = sk_OPENSSL_STRING_num(pkeyopts); + int i; + + for (i = 0; i < num; ++i) { + const char *opt = sk_OPENSSL_STRING_value(pkeyopts, i); + + if (pkey_ctrl_string(ctx, opt) <= 0) { + BIO_printf(bio_err, "%s: Can't set parameter:\n", prog); + ERR_print_errors(bio_err); + goto end; + } + } + } + if (sigfile && (pkey_op != EVP_PKEY_OP_VERIFY)) { BIO_printf(bio_err, "%s: Signature file specified for non verify\n", prog); @@ -262,7 +283,7 @@ int pkeyutl_main(int argc, char **argv) } siglen = bio_to_mem(&sig, keysize * 10, sigbio); BIO_free(sigbio); - if (siglen <= 0) { + if (siglen < 0) { BIO_printf(bio_err, "Error reading signature data\n"); goto end; } @@ -271,7 +292,7 @@ int pkeyutl_main(int argc, char **argv) if (in) { /* Read the input data */ buf_inlen = bio_to_mem(&buf_in, keysize * 10, in); - if (buf_inlen <= 0) { + if (buf_inlen < 0) { BIO_printf(bio_err, "Error reading input Data\n"); exit(1); } @@ -299,13 +320,13 @@ int pkeyutl_main(int argc, char **argv) } rv = do_keyop(ctx, pkey_op, NULL, (size_t *)&buf_outlen, buf_in, (size_t)buf_inlen); - if (rv > 0) { + if (rv > 0 && buf_outlen != 0) { buf_out = app_malloc(buf_outlen, "buffer output"); rv = do_keyop(ctx, pkey_op, buf_out, (size_t *)&buf_outlen, buf_in, (size_t)buf_inlen); } - if (rv <= 0) { + if (rv < 0) { ERR_print_errors(bio_err); goto end; } @@ -326,11 +347,12 @@ int pkeyutl_main(int argc, char **argv) OPENSSL_free(buf_in); OPENSSL_free(buf_out); OPENSSL_free(sig); + sk_OPENSSL_STRING_free(pkeyopts); return ret; } static EVP_PKEY_CTX *init_ctx(int *pkeysize, - char *keyfile, int keyform, int key_type, + const char *keyfile, int keyform, int key_type, char *passinarg, int pkey_op, ENGINE *e) { EVP_PKEY *pkey = NULL; @@ -416,17 +438,16 @@ static EVP_PKEY_CTX *init_ctx(int *pkeysize, } -static int setup_peer(EVP_PKEY_CTX *ctx, int peerform, const char *file) +static int setup_peer(EVP_PKEY_CTX *ctx, int peerform, const char *file, + ENGINE* e) { EVP_PKEY *peer = NULL; + ENGINE* engine = NULL; int ret; - if (!ctx) { - BIO_puts(bio_err, "-peerkey command before -inkey\n"); - return 0; - } - - peer = load_pubkey(file, peerform, 0, NULL, NULL, "Peer Key"); + if (peerform == FORMAT_ENGINE) + engine = e; + peer = load_pubkey(file, peerform, 0, NULL, engine, "Peer Key"); if (!peer) { BIO_printf(bio_err, "Error reading peer key %s\n", file); ERR_print_errors(bio_err); diff --git a/apps/rsautl.c b/apps/rsautl.c index 5d6bdc0242..b576ca0b76 100644 --- a/apps/rsautl.c +++ b/apps/rsautl.c @@ -87,7 +87,7 @@ OPTIONS rsautl_options[] = { {"in", OPT_IN, '<', "Input file"}, {"out", OPT_OUT, '>', "Output file"}, {"inkey", OPT_INKEY, '<', "Input key"}, - {"keyform", OPT_KEYFORM, 'F', "Private key format - default PEM"}, + {"keyform", OPT_KEYFORM, 'E', "Private key format - default PEM"}, {"pubin", OPT_PUBIN, '-', "Input is an RSA public"}, {"certin", OPT_CERTIN, '-', "Input is a cert carrying an RSA public key"}, {"ssl", OPT_SSL, '-', "Use SSL v2 padding"}, @@ -137,7 +137,7 @@ int rsautl_main(int argc, char **argv) ret = 0; goto end; case OPT_KEYFORM: - if (!opt_format(opt_arg(), OPT_FMT_PEMDER, &keyformat)) + if (!opt_format(opt_arg(), OPT_FMT_PDE, &keyformat)) goto opthelp; break; case OPT_IN: @@ -262,7 +262,7 @@ int rsautl_main(int argc, char **argv) /* Read the input data */ rsa_inlen = BIO_read(in, rsa_in, keysize * 2); - if (rsa_inlen <= 0) { + if (rsa_inlen < 0) { BIO_printf(bio_err, "Error reading input Data\n"); goto end; } @@ -294,10 +294,9 @@ int rsautl_main(int argc, char **argv) rsa_outlen = RSA_private_decrypt(rsa_inlen, rsa_in, rsa_out, rsa, pad); break; - } - if (rsa_outlen <= 0) { + if (rsa_outlen < 0) { BIO_printf(bio_err, "RSA operation error\n"); ERR_print_errors(bio_err); goto end; diff --git a/doc/apps/pkeyutl.pod b/doc/apps/pkeyutl.pod index 3c29b3a67c..d44f73aeec 100644 --- a/doc/apps/pkeyutl.pod +++ b/doc/apps/pkeyutl.pod @@ -11,10 +11,10 @@ B B [B<-out file>] [B<-sigfile file>] [B<-inkey file>] -[B<-keyform PEM|DER>] +[B<-keyform PEM|DER|ENGINE>] [B<-passin arg>] [B<-peerkey file>] -[B<-peerform PEM|DER>] +[B<-peerform PEM|DER|ENGINE>] [B<-pubin>] [B<-certin>] [B<-rev>] @@ -52,7 +52,7 @@ default. the input key file, by default it should be a private key. -=item B<-keyform PEM|DER> +=item B<-keyform PEM|DER|ENGINE> the key format PEM, DER or ENGINE. @@ -66,7 +66,7 @@ see the B section in L. the peer key file, used by key derivation (agreement) operations. -=item B<-peerform PEM|DER> +=item B<-peerform PEM|DER|ENGINE> the peer key format PEM, DER or ENGINE. diff --git a/doc/apps/rsautl.pod b/doc/apps/rsautl.pod index 6b98b51864..92b8150cee 100644 --- a/doc/apps/rsautl.pod +++ b/doc/apps/rsautl.pod @@ -10,6 +10,7 @@ B B [B<-in file>] [B<-out file>] [B<-inkey file>] +[B<-keyform PEM|DER|ENGINE>] [B<-pubin>] [B<-certin>] [B<-sign>] @@ -45,6 +46,10 @@ default. the input key file, by default it should be an RSA private key. +=item B<-keyform PEM|DER|ENGINE> + +the key format PEM, DER or ENGINE. + =item B<-pubin> the input file is an RSA public key. -- GitLab