2021-02-10 08:42:28

by Yang Song

[permalink] [raw]
Subject: [PATCH] sign-file: add openssl engine support

Use a customized signature service supported by openssl engine
to sign the kernel module.
Add command line parameters that support engine for sign-file
to use the customized openssl engine service to sign kernel modules.

Signed-off-by: Yang Song <[email protected]>
---
scripts/sign-file.c | 65 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 62 insertions(+), 3 deletions(-)

diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index fbd34b8e8f57..276068e9b595 100644
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -70,7 +70,7 @@ static __attribute__((noreturn))
void format(void)
{
fprintf(stderr,
- "Usage: scripts/sign-file [-dp] <hash algo> <key> <x509> <module> [<dest>]\n");
+ "Usage: scripts/sign-file [-edp] [<openssl engine>] <hash algo> <key> <x509> <module> [<dest>]\n");
fprintf(stderr,
" scripts/sign-file -s <raw sig> <hash algo> <x509> <module> [<dest>]\n");
exit(2);
@@ -206,12 +206,54 @@ static X509 *read_x509(const char *x509_name)
return x509;
}

+/* Try to load an engine in a shareable library */
+static ENGINE *try_load_engine(const char *engine)
+{
+ ENGINE *e = ENGINE_by_id("dynamic");
+ if (e) {
+ if (!ENGINE_ctrl_cmd_string(e, "SO_PATH", engine, 0)
+ || !ENGINE_ctrl_cmd_string(e, "LOAD", NULL, 0)) {
+ ENGINE_free(e);
+ e = NULL;
+ }
+ }
+ return e;
+}
+
+static ENGINE *setup_engine(const char *engine)
+{
+ ENGINE *e = NULL;
+
+ if (engine) {
+ e = ENGINE_by_id(engine);
+ if (e == NULL) {
+ e = try_load_engine(engine);
+ if (e == NULL) {
+ ERR(1, "invalid engine \"%s\"\n", engine);
+ return NULL;
+ }
+ }
+
+ if (!ENGINE_set_default(e, ENGINE_METHOD_ALL)) {
+ ERR(1, "can't use that engine\n");
+ ENGINE_free(e);
+ return NULL;
+ }
+
+ fprintf(stdout, "engine \"%s\" set.\n", ENGINE_get_id(e));
+ }
+
+ return e;
+}
+
int main(int argc, char **argv)
{
struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 };
+ char *ossl_engine = NULL;
char *hash_algo = NULL;
char *private_key_name = NULL, *raw_sig_name = NULL;
char *x509_name, *module_name, *dest_name;
+ bool use_engine = false;
bool save_sig = false, replace_orig;
bool sign_only = false;
bool raw_sig = false;
@@ -242,8 +284,9 @@ int main(int argc, char **argv)
#endif

do {
- opt = getopt(argc, argv, "sdpk");
+ opt = getopt(argc, argv, "sedpk");
switch (opt) {
+ case 'e': use_engine = true; break;
case 's': raw_sig = true; break;
case 'p': save_sig = true; break;
case 'd': sign_only = true; save_sig = true; break;
@@ -257,13 +300,18 @@ int main(int argc, char **argv)

argc -= optind;
argv += optind;
- if (argc < 4 || argc > 5)
+ if (argc < 4 || argc > 6)
format();

if (raw_sig) {
raw_sig_name = argv[0];
hash_algo = argv[1];
} else {
+ if (use_engine) {
+ ossl_engine = argv[0];
+ argc--;
+ argv++;
+ }
hash_algo = argv[0];
private_key_name = argv[1];
}
@@ -291,6 +339,17 @@ int main(int argc, char **argv)
ERR(!bm, "%s", module_name);

if (!raw_sig) {
+ if (use_engine) {
+ if (ossl_engine == NULL) {
+ fprintf(stderr, "Input openssl engine is null\n");
+ exit(1);
+ }
+
+ // Engine setup
+ ENGINE_load_builtin_engines();
+ setup_engine(ossl_engine);
+ }
+
/* Read the private key and the X.509 cert the PKCS#7 message
* will point to.
*/
--
2.19.1.3.ge56e4f7


2021-02-10 08:45:53

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] sign-file: add openssl engine support



On 10 February 2021 07:45:54 GMT, Yang Song <[email protected]> wrote:
>Use a customized signature service supported by openssl engine
>to sign the kernel module.
>Add command line parameters that support engine for sign-file
>to use the customized openssl engine service to sign kernel modules.
>
>Signed-off-by: Yang Song <[email protected]>

Aren't engines already obsolete in the latest versions of OpenSSL, as well as being an implementation detail of one particular crypto library? They aren't really a concept we should be exposing in *our* user interface.

Better to make sign-file automatically recognise RFC7512 PKCS#11 URIs and handle them by automatically loading the PKCS#11 engine.

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

2021-02-10 08:48:35

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] sign-file: add openssl engine support

Yang Song <[email protected]> wrote:

> + "Usage: scripts/sign-file [-edp] [<openssl engine>] <hash algo> <key> <x509> <module> [<dest>]\n");

Do you mean:

"Usage: scripts/sign-file [-dp] [-e <openssl engine>] <hash algo> <key> <x509> <module> [<dest>]\n");

> + opt = getopt(argc, argv, "sedpk");

"se:dpk"?

> + if (use_engine) {
> + ossl_engine = argv[0];

use_engine ought to be a redundant variable.

David

2021-02-10 15:05:19

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] sign-file: add openssl engine support

On Wed, 2021-02-10 at 08:01 +0000, David Woodhouse wrote:
>
> On 10 February 2021 07:45:54 GMT, Yang Song <
> [email protected]> wrote:
> > Use a customized signature service supported by openssl engine
> > to sign the kernel module.
> > Add command line parameters that support engine for sign-file
> > to use the customized openssl engine service to sign kernel
> > modules.
> >
> > Signed-off-by: Yang Song <[email protected]>
>
> Aren't engines already obsolete in the latest versions of OpenSSL, as
> well as being an implementation detail of one particular crypto
> library?

Um, no, they're getting renamed providers with some annoying API
changes that require a bit of a rewrite but the concept of a crypto
"engine" plug in to the code base isn't going away.

> They aren't really a concept we should be exposing in *our* user
> interface.

We already do ... grep ENGINE in scripts/sign-file.c

Just by the way in case anyone is interested in history:

https://lore.kernel.org/keyrings/[email protected]/

> Better to make sign-file automatically recognise RFC7512 PKCS#11 URIs
> and handle them by automatically loading the PKCS#11 engine.

PKCS11 can't cover everyting engines can. Engines are mostly used for
accelerators, which are not in the PKCS11 API and even for external
keys, PKCS11 can't cope if the key isn't inside what PKCS11 thinks of
as a token.

James