Add help for pkeyopt values for the genpkey commandline app. #19931

slontis · 2022-12-19T06:07:20Z

Checklist

documentation is added or updated
tests are added or updated

slontis · 2022-12-19T06:58:45Z

Example output for

openssl genpkey -algorithm DHX -help

Usage: genpkey [options]

General options:
 -help               Display this summary
 -engine val         Use engine, possibly a hardware device
 -paramfile infile   Parameters file
 -algorithm val      The public key algorithm
 -verbose            Output status while generating keys
 -quiet              Do not output status while generating keys
 -pkeyopt val        Set the public key algorithm option as opt:value
 -config infile      Load a configuration file (this may load modules)

Output options:
 -out outfile        Output file
 -outform PEM|DER    output format (DER or PEM)
 -pass val           Output file pass phrase source
 -genparam           Generate parameters, not key
 -text               Print the in text
 -*                  Cipher to use to encrypt the key

Provider options:
 -provider-path val  Provider load path (must be before 'provider' argument if required)
 -provider val       Provider to load (can be specified multiple times)
 -propquery val      Property query used when fetching algorithms
Order of options may be important!  See the documentation.

pkeyopt values are
    type:string
    group:string
    priv_len:int
    pbits:uint
    qbits:uint
    digest:string
    properties:string
    gindex:int
    hexseed:string
    pcounter:int
    hindex:int

apps/genpkey.c

doc/man1/openssl-genpkey.pod.in

hlandau · 2022-12-21T10:18:53Z

apps/genpkey.c

+        return;
+    ctx = EVP_PKEY_CTX_new_from_name(libctx, algname, propq);
+    if (ctx == NULL)
+        return;


Should we not have some kind of output in the event of error (or at least a process exit code)?

hlandau · 2022-12-21T10:19:44Z

apps/genpkey.c

+        const char *name = param_datatype_2name(params[i].data_type, &ishex);
+
+        if (name != NULL)
+            BIO_printf(bio_err, "    %s%s:%s\n", ishex ? "hex" : "", params[i].key, name);


This would print e.g. hexpropname:string... this seems confusing?

Perhaps propname:string (hex) / propname:string?

I think it still needs the hex in front of the name as that is what you do to parse hex format.

paulidale · 2023-01-11T06:02:41Z

test/recipes/15-test_genpkey.t

+foreach (@algs) {
+    my $alg = $_;
+
+    ok(run(app([ 'openssl', 'genpkey', '-algorithm', $alg, '-help'])),


Should the output be checked for minimal sanity?

E.g. the string "pkeyopt values are" appears. Perhaps an option as well.

levitte · 2023-01-11T07:22:40Z

You do know, I hope, that you're kinda sorta reinventing the wheel? Have a look at apps/include/app_params.h and apps/lib/app_params.c. (openssl list uses those functions)

Of course, the output from those functions is bit more verbose, but I suspect it wouldn't be too hard to add a verbosity parameter.

Also, I would expect that this sort of help will find its way to all the other commands that use these sorts of opts too... not asking for it in this PR, but I'd really like it if that was the intent, at least.

slontis · 2023-01-12T01:09:14Z

You do know, I hope, that you're kinda sorta reinventing the wheel?.

This is NOT trying to print out every possible param value like the list does.. Only ones that are actually usable by the command (e.g. OSSL_PARAM_UTF8_PTR is not something you can set from here).

slontis · 2023-01-12T01:10:26Z

Also, I would expect that this sort of help will find its way to all the other commands that use these sorts of opts too... not asking for it in this PR, but I'd really like it if that was the intent, at least.

Is there really that many of them? This particular one is the one I keep on looking up all the time.

levitte · 2023-01-12T09:06:49Z

Also, I would expect that this sort of help will find its way to all the other commands that use these sorts of opts too... not asking for it in this PR, but I'd really like it if that was the intent, at least.

Is there really that many of them? This particular one is the one I keep on looking up all the time.

I haven't looked thoroughly, but I suspect that all pkey commands (openssl pkeyutl, openssl pkeyparam, openssl pkey) do, and I'm pretty sure that the newer operation commands (openssl mac, openssl kdf) do. Also, for example openssl dgst has things like -sigopt and -macopt, although I'm unsure how relevant that is.

paulidale · 2023-01-15T23:16:04Z

Having the command I want to run produce a list is useful IMO.

Knowing that list can also produce such (but in a less convenient verbosity) isn't a totally natural step. It ought to be but it isn't.

openssl-machine · 2023-02-15T00:09:57Z

This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago

tmshort · 2023-02-20T15:32:06Z

@hlandau ?

openssl-machine · 2023-02-21T03:00:21Z

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

stale

tmshort · 2023-02-23T15:10:38Z

As @hlandau removed their stale review, this has passed the 24 hour threshold.

tmshort · 2023-02-23T15:17:06Z

Merged to master. Thank you for your contribution!

Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Todd Short <todd.short@me.com> (Merged from #19931)

Add help for pkeyopt values for the genpkey commandline app.

b119771

slontis added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member labels Dec 19, 2022

t8m added the triaged: feature The issue/pr requests/adds a feature label Dec 20, 2022

t8m reviewed Dec 20, 2022

View reviewed changes

apps/genpkey.c Outdated Show resolved Hide resolved

t8m reviewed Dec 20, 2022

View reviewed changes

doc/man1/openssl-genpkey.pod.in Outdated Show resolved Hide resolved

t8m added the tests: present The PR has suitable tests present label Dec 20, 2022

fixup! Add help for pkeyopt values for the genpkey commandline app.

5fede0c

t8m approved these changes Dec 21, 2022

View reviewed changes

t8m removed the approval: otc review pending This pull request needs review by an OTC member label Dec 21, 2022

hlandau previously requested changes Dec 21, 2022

View reviewed changes

paulidale reviewed Jan 11, 2023

View reviewed changes

paulidale approved these changes Feb 20, 2023

View reviewed changes

paulidale added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Feb 20, 2023

hlandau requested review from hlandau and removed request for hlandau February 22, 2023 05:25

tmshort self-assigned this Feb 23, 2023

tmshort closed this Feb 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add help for pkeyopt values for the genpkey commandline app. #19931

Add help for pkeyopt values for the genpkey commandline app. #19931

slontis commented Dec 19, 2022

slontis commented Dec 19, 2022

hlandau Dec 21, 2022

hlandau Dec 21, 2022

slontis Jan 11, 2023

paulidale Jan 11, 2023

levitte commented Jan 11, 2023 •

edited

slontis commented Jan 12, 2023

slontis commented Jan 12, 2023

levitte commented Jan 12, 2023

paulidale commented Jan 15, 2023

openssl-machine commented Feb 15, 2023

tmshort commented Feb 20, 2023

openssl-machine commented Feb 21, 2023

tmshort commented Feb 23, 2023

tmshort commented Feb 23, 2023

Add help for pkeyopt values for the genpkey commandline app. #19931

Add help for pkeyopt values for the genpkey commandline app. #19931

Conversation

slontis commented Dec 19, 2022

Checklist

slontis commented Dec 19, 2022

hlandau Dec 21, 2022

Choose a reason for hiding this comment

hlandau Dec 21, 2022

Choose a reason for hiding this comment

slontis Jan 11, 2023

Choose a reason for hiding this comment

paulidale Jan 11, 2023

Choose a reason for hiding this comment

levitte commented Jan 11, 2023 • edited

slontis commented Jan 12, 2023

slontis commented Jan 12, 2023

levitte commented Jan 12, 2023

paulidale commented Jan 15, 2023

openssl-machine commented Feb 15, 2023

tmshort commented Feb 20, 2023

openssl-machine commented Feb 21, 2023

tmshort commented Feb 23, 2023

tmshort commented Feb 23, 2023

levitte commented Jan 11, 2023 •

edited