Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

APPS/pkeyutl: improve -rawin usability and doc #22910

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

APPS/pkeyutl: improve -rawin usability and doc #22910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c7afe95
APPS/pkeyutl: improve -rawin usability (implied by Ed25519 and Ed448)…
DDvO Dec 2, 2023
d6ecb4d
fixup! APPS/pkeyutl: improve -rawin usability (implied by Ed25519 and…
DDvO Apr 12, 2024
119ae50
fixup! fixup! APPS/pkeyutl: improve -rawin usability (implied by Ed25…
DDvO Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ OpenSSL 3.4

*Damian Hobson-Garcia*

* The `-rawin` option of the `pkeyutl` apps is now implied (and thus no more
required) when signing or verifying with an Ed25519 or Ed448 key.

*David von Oheimb*

* Added support to build Position Independent Executables (PIE). Configuration
option `enable-pie` configures the cflag '-fPIE' and ldflag '-pie' to
support Address Space Layout Randomization (ASLR) in the openssl executable,
Expand Down
117 changes: 69 additions & 48 deletions apps/pkeyutl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@
#define KEY_PUBKEY 2
#define KEY_CERT 3

static EVP_PKEY *get_pkey(const char *kdfalg,
const char *keyfile, int keyform, int key_type,
char *passinarg, int pkey_op, ENGINE *e);
static EVP_PKEY_CTX *init_ctx(const char *kdfalg, int *pkeysize,
const char *keyfile, int keyform, int key_type,
char *passinarg, int pkey_op, ENGINE *e,
const int impl, int rawin, EVP_PKEY **ppkey,
int pkey_op, ENGINE *e,
const int impl, int rawin,
EVP_PKEY *pkey /* may get deallocated */,
EVP_MD_CTX *mctx, const char *digestname,
OSSL_LIB_CTX *libctx, const char *propq);

Expand All @@ -39,6 +42,14 @@ static int do_raw_keyop(int pkey_op, EVP_MD_CTX *mctx,
int filesize, unsigned char *sig, int siglen,
unsigned char **out, size_t *poutlen);

static int only_rawin(const EVP_PKEY *pkey)
{
if (pkey == NULL)
return 0;
return EVP_PKEY_is_a(pkey, "ED25519")
|| EVP_PKEY_is_a(pkey, "ED448");
}

beldmit marked this conversation as resolved.
Show resolved Hide resolved
typedef enum OPTION_choice {
OPT_COMMON,
OPT_ENGINE, OPT_ENGINE_IMPL, OPT_IN, OPT_OUT,
Expand Down Expand Up @@ -68,7 +79,7 @@ const OPTIONS pkeyutl_options[] = {

OPT_SECTION("Input"),
{"in", OPT_IN, '<', "Input file - default stdin"},
{"rawin", OPT_RAWIN, '-', "Indicate the input data is in raw form"},
{"rawin", OPT_RAWIN, '-', "Indicate that signature input data is not hashed"},
{"inkey", OPT_INKEY, 's', "Input key, by default private key"},
{"pubin", OPT_PUBIN, '-', "Input key is a public key"},
{"passin", OPT_PASSIN, 's', "Input file pass phrase source"},
Expand Down Expand Up @@ -259,26 +270,6 @@ int pkeyutl_main(int argc, char **argv)
if (!app_RAND_load())
goto end;

if (rawin && pkey_op != EVP_PKEY_OP_SIGN && pkey_op != EVP_PKEY_OP_VERIFY) {
BIO_printf(bio_err,
"%s: -rawin can only be used with -sign or -verify\n",
prog);
goto opthelp;
}

if (digestname != NULL && !rawin) {
BIO_printf(bio_err,
"%s: -digest can only be used with -rawin\n",
prog);
goto opthelp;
}

if (rawin && rev) {
BIO_printf(bio_err, "%s: -rev cannot be used with raw input\n",
prog);
goto opthelp;
}

if (kdfalg != NULL) {
if (kdflen == 0) {
BIO_printf(bio_err,
Expand All @@ -295,14 +286,37 @@ int pkeyutl_main(int argc, char **argv)
goto opthelp;
}

pkey = get_pkey(kdfalg, inkey, keyform, key_type, passinarg, pkey_op, e);
if (pkey_op == EVP_PKEY_OP_SIGN || pkey_op == EVP_PKEY_OP_VERIFY) {
if (only_rawin(pkey))
rawin = 1; /* implied for Ed25519 and Ed448 */
} else if (rawin) {
BIO_printf(bio_err,
"%s: -rawin can only be used with -sign or -verify\n", prog);
EVP_PKEY_free(pkey);
goto opthelp;
}
if (digestname != NULL && !rawin) {
BIO_printf(bio_err,
"%s: -digest can only be used with -rawin\n", prog);
EVP_PKEY_free(pkey);
goto opthelp;
}

if (rawin && rev) {
BIO_printf(bio_err, "%s: -rev cannot be used with raw input\n", prog);
DDvO marked this conversation as resolved.
Show resolved Hide resolved
EVP_PKEY_free(pkey);
goto opthelp;
}

if (rawin) {
if ((mctx = EVP_MD_CTX_new()) == NULL) {
BIO_printf(bio_err, "Error: out of memory\n");
EVP_PKEY_free(pkey);
goto end;
}
}
ctx = init_ctx(kdfalg, &keysize, inkey, keyform, key_type,
passinarg, pkey_op, e, engine_impl, rawin, &pkey,
ctx = init_ctx(kdfalg, &keysize, pkey_op, e, engine_impl, rawin, pkey,
mctx, digestname, libctx, app_get0_propq());
if (ctx == NULL) {
BIO_printf(bio_err, "%s: Error initializing context\n", prog);
Expand Down Expand Up @@ -356,8 +370,10 @@ int pkeyutl_main(int argc, char **argv)
goto end;
}
} else {
/* Get password as a passin argument: First split option name
* and passphrase argument into two strings */
/*
* Get password as a passin argument: First split option name
* and passphrase argument into two strings
*/
*passin = 0;
passin++;
if (app_passwd(passin, NULL, &passwd, NULL) == 0) {
Expand Down Expand Up @@ -429,6 +445,7 @@ int pkeyutl_main(int argc, char **argv)
size_t i;
unsigned char ctmp;
size_t l = (size_t)buf_inlen;

for (i = 0; i < l / 2; i++) {
ctmp = buf_in[i];
buf_in[i] = buf_in[l - 1 - i];
Expand Down Expand Up @@ -519,29 +536,23 @@ int pkeyutl_main(int argc, char **argv)
return ret;
}

static EVP_PKEY_CTX *init_ctx(const char *kdfalg, int *pkeysize,
const char *keyfile, int keyform, int key_type,
char *passinarg, int pkey_op, ENGINE *e,
const int engine_impl, int rawin,
EVP_PKEY **ppkey, EVP_MD_CTX *mctx, const char *digestname,
OSSL_LIB_CTX *libctx, const char *propq)
static EVP_PKEY *get_pkey(const char *kdfalg,
const char *keyfile, int keyform, int key_type,
char *passinarg, int pkey_op, ENGINE *e)
{
EVP_PKEY *pkey = NULL;
EVP_PKEY_CTX *ctx = NULL;
ENGINE *impl = NULL;
char *passin = NULL;
int rv = -1;
X509 *x;

if (((pkey_op == EVP_PKEY_OP_SIGN) || (pkey_op == EVP_PKEY_OP_DECRYPT)
|| (pkey_op == EVP_PKEY_OP_DERIVE))
&& (key_type != KEY_PRIVKEY && kdfalg == NULL)) {
BIO_printf(bio_err, "A private key is needed for this operation\n");
goto end;
return NULL;
}
if (!app_passwd(passinarg, NULL, &passin, NULL)) {
BIO_printf(bio_err, "Error getting password\n");
goto end;
return NULL;
}
switch (key_type) {
case KEY_PRIVKEY:
Expand All @@ -564,6 +575,20 @@ static EVP_PKEY_CTX *init_ctx(const char *kdfalg, int *pkeysize,
break;

}
OPENSSL_free(passin);
return pkey;
}

static EVP_PKEY_CTX *init_ctx(const char *kdfalg, int *pkeysize,
int pkey_op, ENGINE *e,
const int engine_impl, int rawin,
EVP_PKEY *pkey /* ownership is passed to ctx */,
EVP_MD_CTX *mctx, const char *digestname,
OSSL_LIB_CTX *libctx, const char *propq)
{
EVP_PKEY_CTX *ctx = NULL;
ENGINE *impl = NULL;
int rv = -1;

#ifndef OPENSSL_NO_ENGINE
if (engine_impl)
Expand All @@ -578,7 +603,7 @@ static EVP_PKEY_CTX *init_ctx(const char *kdfalg, int *pkeysize,
if (kdfnid == NID_undef) {
BIO_printf(bio_err, "The given KDF \"%s\" is unknown.\n",
kdfalg);
goto end;
return NULL;
}
}
if (impl != NULL)
Expand All @@ -587,20 +612,18 @@ static EVP_PKEY_CTX *init_ctx(const char *kdfalg, int *pkeysize,
ctx = EVP_PKEY_CTX_new_from_name(libctx, kdfalg, propq);
} else {
if (pkey == NULL)
goto end;
return NULL;

*pkeysize = EVP_PKEY_get_size(pkey);
if (impl != NULL)
ctx = EVP_PKEY_CTX_new(pkey, impl);
else
ctx = EVP_PKEY_CTX_new_from_pkey(libctx, pkey, propq);
if (ppkey != NULL)
*ppkey = pkey;
EVP_PKEY_free(pkey);
}

if (ctx == NULL)
goto end;
return NULL;

if (rawin) {
EVP_MD_CTX_set_pkey_ctx(mctx, ctx);
Expand Down Expand Up @@ -650,8 +673,6 @@ static EVP_PKEY_CTX *init_ctx(const char *kdfalg, int *pkeysize,
ctx = NULL;
}

end:
OPENSSL_free(passin);
return ctx;

}
Expand Down Expand Up @@ -682,6 +703,7 @@ static int do_keyop(EVP_PKEY_CTX *ctx, int pkey_op,
const unsigned char *in, size_t inlen)
{
int rv = 0;

switch (pkey_op) {
case EVP_PKEY_OP_VERIFYRECOVER:
rv = EVP_PKEY_verify_recover(ctx, out, poutlen, in, inlen);
Expand Down Expand Up @@ -720,8 +742,7 @@ static int do_raw_keyop(int pkey_op, EVP_MD_CTX *mctx,
int buf_len = 0;

/* Some algorithms only support oneshot digests */
if (EVP_PKEY_get_id(pkey) == EVP_PKEY_ED25519
|| EVP_PKEY_get_id(pkey) == EVP_PKEY_ED448) {
if (only_rawin(pkey)) {
if (filesize < 0) {
BIO_printf(bio_err,
"Error: unable to determine file size for oneshot operation\n");
Expand Down
10 changes: 6 additions & 4 deletions doc/man1/openssl-pkeyutl.pod.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,11 @@ if this option is not specified.

=item B<-rawin>

This indicates that the input data is raw data, which is not hashed by any
message digest algorithm. The user can specify a digest algorithm by using
the B<-digest> option. This option can only be used with B<-sign> and
B<-verify> and must be used with the Ed25519 and Ed448 algorithms.
This indicates that signature input data is raw data, which for most signature
algorithms (but not EdDSA) needs to be hashed by some message digest algorithm.
The user can specify a digest algorithm by using the B<-digest> option.
This option can only be used with B<-sign> and B<-verify>
and is implied by the Ed25519 and Ed448 algorithms.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this still be true if/when the Ed25519ph and Ed448ph algorithms are supported?
Presumably the key algorithm will be the same, and the user will need some way to indicate whether to prehash the data with SHA-512? (I'm not saying this will be a problem, just asking the question, we should have some idea of how the PH variants should be supported before making changes that could hypothetically get in the way).


=item B<-digest> I<algorithm>

Expand Down Expand Up @@ -120,6 +121,7 @@ The input is a certificate containing a public key.

Reverse the order of the input buffer. This is useful for some libraries
(such as CryptoAPI) which represent the buffer in little endian format.
This cannot be used in conjunction with B<-rawin>.

=item B<-sign>

Expand Down
32 changes: 27 additions & 5 deletions test/recipes/20-test_pkeyutl.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use File::Compare qw/compare_text/;

setup("test_pkeyutl");

plan tests => 14;
plan tests => 18;

# For the tests below we use the cert itself as the TBS file

Expand Down Expand Up @@ -54,19 +54,19 @@ SKIP: {
}

SKIP: {
skip "Skipping tests that require ECX", 4
skip "Skipping tests that require ECX", 6
if disabled("ecx");

# Ed25519
ok(run(app(([ 'openssl', 'pkeyutl', '-sign', '-in',
srctop_file('test', 'certs', 'server-ed25519-cert.pem'),
'-inkey', srctop_file('test', 'certs', 'server-ed25519-key.pem'),
'-out', 'Ed25519.sig', '-rawin']))),
'-out', 'Ed25519.sig']))),
"Sign a piece of data using Ed25519");
ok(run(app(([ 'openssl', 'pkeyutl', '-verify', '-certin', '-in',
srctop_file('test', 'certs', 'server-ed25519-cert.pem'),
'-inkey', srctop_file('test', 'certs', 'server-ed25519-cert.pem'),
'-sigfile', 'Ed25519.sig', '-rawin']))),
'-sigfile', 'Ed25519.sig']))),
"Verify an Ed25519 signature against a piece of data");

# Ed448
Expand All @@ -80,6 +80,16 @@ SKIP: {
'-inkey', srctop_file('test', 'certs', 'server-ed448-cert.pem'),
'-sigfile', 'Ed448.sig', '-rawin']))),
"Verify an Ed448 signature against a piece of data");
ok(run(app(([ 'openssl', 'pkeyutl', '-sign', '-in',
srctop_file('test', 'certs', 'server-ed448-cert.pem'),
'-inkey', srctop_file('test', 'certs', 'server-ed448-key.pem'),
'-out', 'Ed448.sig']))),
"Sign a piece of data using Ed448 -rawin no more needed");
ok(run(app(([ 'openssl', 'pkeyutl', '-verify', '-certin', '-in',
srctop_file('test', 'certs', 'server-ed448-cert.pem'),
'-inkey', srctop_file('test', 'certs', 'server-ed448-cert.pem'),
'-sigfile', 'Ed448.sig']))),
"Verify an Ed448 signature against a piece of data, no -rawin");
}

sub tsignverify {
Expand Down Expand Up @@ -183,7 +193,7 @@ SKIP: {
}

SKIP: {
skip "EdDSA is not supported by this OpenSSL build", 2
skip "EdDSA is not supported by this OpenSSL build", 4
if disabled("ecx");

subtest "Ed2559 CLI signature generation and verification" => sub {
Expand All @@ -199,4 +209,16 @@ SKIP: {
srctop_file("test","tested448pub.pem"),
"-rawin");
};

subtest "Ed2559 CLI signature generation and verification, no -rawin" => sub {
tsignverify("Ed25519",
srctop_file("test","tested25519.pem"),
srctop_file("test","tested25519pub.pem"));
};

subtest "Ed448 CLI signature generation and verification, no -rawin" => sub {
tsignverify("Ed448",
srctop_file("test","tested448.pem"),
srctop_file("test","tested448pub.pem"));
};
}