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 <rsalz@openssl.org>

5 files changed, 119 insertions, 68 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;