[PATCH v3 0/4] Allow multiple keyspecs in one environment variable

[PATCH v3 0/4] Allow multiple keyspecs in one environment variable

[Jonas Rebmann]

This contains the actual change to keytoc as well a migration Note.

Allowing any number of public keys to be provided via a single
environment variable eases integration in more complex setups where
multiple public keys per keyring are managed externally.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Changes in v3:
- Where possible, split refactoring/cleanup into a preparatory commit
- Split copying strsep_unescape into a separate commit
- Link to v2: https://lore.barebox.org/barebox/20260218-keytoc-multi-env-v2-0-3ea146c95d18@pengutronix.de

Changes in v2:
- Split up the keyspec environment variables when multiple are provided
  too.
- Use strsep_unescape instead of manual split/unescape
- Remove RFC tag
- Link to v1: https://lore.barebox.org/barebox/20260206-keytoc-multi-env-v1-1-638fbf2b3634@pengutronix.de

---
Jonas Rebmann (4):
      scripts: include: Add string_util.h for strsep_unescaped
      crypto: keytoc: Improve readability
      crypto: keytoc: Split env-provided full keyspec on spaces
      Documentation: migration-guides: Document change in keyspec env vars

 .../migration-guides/migration-master.rst          | 17 +++++
 scripts/include/string_util.h                      | 65 +++++++++++++++++++
 scripts/keytoc.c                                   | 72 +++++++++++++++-------
 3 files changed, 133 insertions(+), 21 deletions(-)
---
base-commit: ff814eff55e898037503e942df8e0ba8f1b13222
change-id: 20260206-keytoc-multi-env-4a3300292e4a

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>

[PATCH v3 1/4] scripts: include: Add string_util.h for strsep_unescaped

[Jonas Rebmann]

lib/string.c has strsep_unescaped() that will be useful for the keytoc
host tool to split ENV specs at spaces not preceded by a backslash in a
future commit.

In preparation, create scripts/include/string_util.h with a copy of that
function as to have it available in host tools.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 scripts/include/string_util.h | 65 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/scripts/include/string_util.h b/scripts/include/string_util.h
new file mode 100644
index 0000000000..e71aa60d26
--- /dev/null
+++ b/scripts/include/string_util.h
@@ -0,0 +1,65 @@
+#ifndef _TOOLS_STRING_UTIL_H_
+#define _TOOLS_STRING_UTIL_H_
+
+#include <linux/types.h>
+#include <stddef.h>
+
+// SPDX-SnippetBegin
+// SPDX-Snippet-Comment: Origin-URL: https://git.pengutronix.de/cgit/barebox/tree/lib/string.c?id=dfcf686f94a5a5387660f2afab79a714baab828a
+
+/**
+ * strsep_unescaped - Split a string into tokens, while ignoring escaped delimiters
+ * @s: The string to be searched
+ * @ct: The delimiter characters to search for
+ * @delim: optional pointer to store found delimiter into
+ *
+ * strsep_unescaped() behaves like strsep unless it meets an escaped delimiter.
+ * In that case, it shifts the string back in memory to overwrite the escape's
+ * backslash then continues the search until an unescaped delimiter is found.
+ *
+ * On end of string, this function returns NULL. As long as a non-NULL
+ * value is returned and @delim is not NULL, the found delimiter will
+ * be stored into *@delim.
+ */
+static char *strsep_unescaped(char **s, const char *ct, char *delim)
+{
+	char *sbegin = *s, *hay;
+	const char *needle;
+	size_t shift = 0;
+
+	if (sbegin == NULL)
+		return NULL;
+
+	for (hay = sbegin; *hay != '\0'; ++hay) {
+		*hay = hay[shift];
+
+		if (*hay == '\\') {
+			*hay = hay[++shift];
+			if (*hay != '\\')
+				continue;
+		}
+
+		for (needle = ct; *needle != '\0'; ++needle) {
+			if (*hay == *needle)
+				goto match;
+		}
+	}
+
+	*s = NULL;
+	if (delim)
+		*delim = '\0';
+	return sbegin;
+
+match:
+	if (delim)
+		*delim = *hay;
+	*hay = '\0';
+	*s = &hay[shift + 1];
+
+	return sbegin;
+}
+
+// SPDX-SnippetEnd
+
+
+#endif /* _TOOLS_STRING_UTIL_H_ */

-- 
2.53.0.308.g50d063e335

Re: [PATCH v3 1/4] scripts: include: Add string_util.h for strsep_unescaped

[Marco Felsch]

On 26-03-16, Jonas Rebmann wrote:
> lib/string.c has strsep_unescaped() that will be useful for the keytoc
> host tool to split ENV specs at spaces not preceded by a backslash in a
> future commit.
> 
> In preparation, create scripts/include/string_util.h with a copy of that
> function as to have it available in host tools.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>  scripts/include/string_util.h | 65 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/scripts/include/string_util.h b/scripts/include/string_util.h
> new file mode 100644
> index 0000000000..e71aa60d26
> --- /dev/null
> +++ b/scripts/include/string_util.h
> @@ -0,0 +1,65 @@
> +#ifndef _TOOLS_STRING_UTIL_H_
> +#define _TOOLS_STRING_UTIL_H_
> +
> +#include <linux/types.h>
> +#include <stddef.h>
> +
> +// SPDX-SnippetBegin
> +// SPDX-Snippet-Comment: Origin-URL: https://git.pengutronix.de/cgit/barebox/tree/lib/string.c?id=dfcf686f94a5a5387660f2afab79a714baab828a

Do we really need this if we reference code within our own repo?

> +/**
> + * strsep_unescaped - Split a string into tokens, while ignoring escaped delimiters
> + * @s: The string to be searched
> + * @ct: The delimiter characters to search for
> + * @delim: optional pointer to store found delimiter into
> + *
> + * strsep_unescaped() behaves like strsep unless it meets an escaped delimiter.
> + * In that case, it shifts the string back in memory to overwrite the escape's
> + * backslash then continues the search until an unescaped delimiter is found.
> + *
> + * On end of string, this function returns NULL. As long as a non-NULL
> + * value is returned and @delim is not NULL, the found delimiter will
> + * be stored into *@delim.
> + */
> +static char *strsep_unescaped(char **s, const char *ct, char *delim)

Is there a reason for defining this function within a .h instead of a .c
file?

Regards,
  Marco

> +{
> +	char *sbegin = *s, *hay;
> +	const char *needle;
> +	size_t shift = 0;
> +
> +	if (sbegin == NULL)
> +		return NULL;
> +
> +	for (hay = sbegin; *hay != '\0'; ++hay) {
> +		*hay = hay[shift];
> +
> +		if (*hay == '\\') {
> +			*hay = hay[++shift];
> +			if (*hay != '\\')
> +				continue;
> +		}
> +
> +		for (needle = ct; *needle != '\0'; ++needle) {
> +			if (*hay == *needle)
> +				goto match;
> +		}
> +	}
> +
> +	*s = NULL;
> +	if (delim)
> +		*delim = '\0';
> +	return sbegin;
> +
> +match:
> +	if (delim)
> +		*delim = *hay;
> +	*hay = '\0';
> +	*s = &hay[shift + 1];
> +
> +	return sbegin;
> +}
> +
> +// SPDX-SnippetEnd
> +
> +
> +#endif /* _TOOLS_STRING_UTIL_H_ */
> 
> -- 
> 2.53.0.308.g50d063e335
> 
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v3 1/4] scripts: include: Add string_util.h for strsep_unescaped

[Jonas Rebmann]

Hello,

On 2026-03-17 00:25, Marco Felsch wrote:
>> +// SPDX-SnippetBegin
>> +// SPDX-Snippet-Comment: Origin-URL: https://git.pengutronix.de/cgit/barebox/tree/lib/string.c?id=dfcf686f94a5a5387660f2afab79a714baab828a
> 
> Do we really need this if we reference code within our own repo?

Yes, we want any code copied in verbatim to be referenced so we can have
an eye on diverging implementations.

>> +/**
>> + * strsep_unescaped - Split a string into tokens, while ignoring escaped delimiters
>> + * @s: The string to be searched
>> + * @ct: The delimiter characters to search for
>> + * @delim: optional pointer to store found delimiter into
>> + *
>> + * strsep_unescaped() behaves like strsep unless it meets an escaped delimiter.
>> + * In that case, it shifts the string back in memory to overwrite the escape's
>> + * backslash then continues the search until an unescaped delimiter is found.
>> + *
>> + * On end of string, this function returns NULL. As long as a non-NULL
>> + * value is returned and @delim is not NULL, the found delimiter will
>> + * be stored into *@delim.
>> + */
>> +static char *strsep_unescaped(char **s, const char *ct, char *delim)
> 
> Is there a reason for defining this function within a .h instead of a .c
> file?

Yes, this is how it's done in scripts/include because those "scripts"
are single-source.

-- 
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

[PATCH v3 2/4] crypto: keytoc: Improve readability

[Jonas Rebmann]

In preparation to a bigger change in the handling of the arguments list,
update variable names, function names and comments to improve
readability.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 scripts/keytoc.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/scripts/keytoc.c b/scripts/keytoc.c
index 77ada3af45..1b99393fdc 100644
--- a/scripts/keytoc.c
+++ b/scripts/keytoc.c
@@ -6,9 +6,10 @@
  * URI to a C struct suitable to compile with barebox.
  *
  * TODO: Find a better way for reimport_key()
- *
  */
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations" /* ENGINE deprecated in OpenSSL 3.0 */
+
 #include <stdio.h>
 #include <string.h>
 #include <time.h>
@@ -784,7 +785,7 @@ static bool parse_info(char *p, struct keyinfo *out)
 	}
 }
 
-static bool get_name_path(const char *keyspec, struct keyinfo *out)
+static bool parse_keyspec(const char *keyspec, struct keyinfo *out)
 {
 	char *sep, *spec;
 
@@ -814,7 +815,7 @@ static bool get_name_path(const char *keyspec, struct keyinfo *out)
 
 int main(int argc, char *argv[])
 {
-	int i, opt, ret;
+	int argi, opt, ret;
 	char *outfile = NULL;
 	int keycount;
 	struct keyinfo *keylist;
@@ -855,9 +856,9 @@ int main(int argc, char *argv[])
 	keycount = argc - optind;
 	keylist = calloc(sizeof(struct keyinfo), keycount);
 
-	for (i = 0; i < keycount; i++) {
-		const char *keyspec = try_resolve_env(argv[optind + i]);
-		struct keyinfo *info = &keylist[i];
+	for (argi = 0; argi < keycount; argi++) {
+		const char *keyspec = try_resolve_env(argv[optind + argi]);
+		struct keyinfo *info = &keylist[argi];
 
 		if (!keyspec)
 			exit(1);
@@ -865,7 +866,7 @@ int main(int argc, char *argv[])
 		if (!strncmp(keyspec, "pkcs11:", 7)) { // legacy format of pkcs11 URI
 			info->path = strdup(keyspec);
 		} else {
-			if (!get_name_path(keyspec, info)) {
+			if (!parse_keyspec(keyspec, info)) {
 				fprintf(stderr, "invalid keyspec %i: %s\n", optind, keyspec);
 				exit(1);
 			}
@@ -885,14 +886,14 @@ int main(int argc, char *argv[])
 	}
 
 
-	for (i = 0; i < keycount; i++) {
-		struct keyinfo *info = &keylist[i];
+	for (argi = 0; argi < keycount; argi++) {
+		struct keyinfo *info = &keylist[argi];
 
 		/* resolve __ENV__ for name_hint and path */
 		info->name_hint = try_resolve_env(info->name_hint);
 		info->path = try_resolve_env(info->path);
 
-		if (asprintf(&info->name_c, "key_%i", i + 1) < 0)
+		if (asprintf(&info->name_c, "key_%i", argi + 1) < 0)
 			enomem_exit("asprintf");
 
 		/* unfortunately, the fit name hint is mandatory in the barebox codebase */
@@ -901,7 +902,7 @@ int main(int argc, char *argv[])
 
 		if (!info->keyring) {
 			info->keyring = strdup("fit");
-			fprintf(stderr, "Warning: No keyring provided in keyspec, defaulting to keyring=fit for %s\n", argv[optind + i]);
+			fprintf(stderr, "Warning: No keyring provided in keyspec, defaulting to keyring=fit for %s\n", argv[optind + argi]);
 		}
 
 		ret = gen_key(info);

-- 
2.53.0.308.g50d063e335

[PATCH v3 3/4] crypto: keytoc: Split env-provided full keyspec on spaces

[Jonas Rebmann]

keytoc/CONFIG_CRYPTO_PUBLIC_KEYS can work with a complete keyspec
provided by an environment variable as opposed to providing single URIs.
This would be a very useful feature if it could also provide any number
of keys. Kconfig however provides keytoc with regular keyspecs already
split at spaces so without furhter measures, the env variable can only
be expanded into a single key.

If a complete argument is provided via __ENV, split it at any space
character that is not escaped with a backslash in front of it.  An
actual backslash in a path needs to be escape with another backslash.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 scripts/keytoc.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/scripts/keytoc.c b/scripts/keytoc.c
index 1b99393fdc..c451ee081a 100644
--- a/scripts/keytoc.c
+++ b/scripts/keytoc.c
@@ -10,6 +10,8 @@
 
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations" /* ENGINE deprecated in OpenSSL 3.0 */
 
+#include "include/string_util.h"
+
 #include <stdio.h>
 #include <string.h>
 #include <time.h>
@@ -787,6 +789,11 @@ static bool parse_info(char *p, struct keyinfo *out)
 
 static bool parse_keyspec(const char *keyspec, struct keyinfo *out)
 {
+	if (!strncmp(keyspec, "pkcs11:", 7)) { /* legacy format of pkcs11 URI */
+		out->path = strdup(keyspec);
+		return true;
+	}
+
 	char *sep, *spec;
 
 	spec = strdup(keyspec);
@@ -817,7 +824,7 @@ int main(int argc, char *argv[])
 {
 	int argi, opt, ret;
 	char *outfile = NULL;
-	int keycount;
+	size_t keycount, num_positionals;
 	struct keyinfo *keylist;
 
 	outfilep = stdout;
@@ -853,22 +860,44 @@ int main(int argc, char *argv[])
 		exit(1);
 	}
 
-	keycount = argc - optind;
-	keylist = calloc(sizeof(struct keyinfo), keycount);
 
-	for (argi = 0; argi < keycount; argi++) {
-		const char *keyspec = try_resolve_env(argv[optind + argi]);
-		struct keyinfo *info = &keylist[argi];
+	num_positionals = argc - optind;
+	keycount = num_positionals;
 
-		if (!keyspec)
-			exit(1);
+	keylist = calloc(keycount, sizeof(*keylist));
+
+	if (!keylist)
+		enomem_exit("push");
+
+	int listi = 0;
+
+	for (argi = 0; argi < num_positionals; argi++) {
+		char *arg = strdup(argv[optind + argi]);
+		char *resolved = try_resolve_env(arg);
 
-		if (!strncmp(keyspec, "pkcs11:", 7)) { // legacy format of pkcs11 URI
-			info->path = strdup(keyspec);
+		if (arg == resolved) {
+			keylist[listi].path = arg;
+			listi++;
 		} else {
-			if (!parse_keyspec(keyspec, info)) {
-				fprintf(stderr, "invalid keyspec %i: %s\n", optind, keyspec);
-				exit(1);
+			char *keyspecs = strdup(resolved);
+			char *keyspec;
+
+			/* Keyspec given as env Variable,
+			 * remove it and add an arbitrary number of keyspecs from its contents
+			 */
+			keycount--;
+			while ((keyspec = strsep_unescaped(&keyspecs, " ", NULL))) {
+				keycount++;
+				keylist = reallocarray(keylist, keycount, sizeof(*keylist));
+				if (!keylist)
+					enomem_exit("realloc keylist");
+				bzero(keylist + (keycount - 1), sizeof(*keylist));
+				if (!parse_keyspec(keyspec, &keylist[listi])) {
+					fprintf(stderr, "invalid keyspec %i: %s\n", optind,
+						keyspec);
+					exit(1);
+				}
+				listi++;
 			}
 		}
 	}

-- 
2.53.0.308.g50d063e335

Re: [PATCH v3 3/4] crypto: keytoc: Split env-provided full keyspec on spaces

[Marco Felsch]

On 26-03-16, Jonas Rebmann wrote:
> keytoc/CONFIG_CRYPTO_PUBLIC_KEYS can work with a complete keyspec
> provided by an environment variable as opposed to providing single URIs.
> This would be a very useful feature if it could also provide any number
> of keys. Kconfig however provides keytoc with regular keyspecs already
> split at spaces so without furhter measures, the env variable can only
> be expanded into a single key.
> 
> If a complete argument is provided via __ENV, split it at any space
> character that is not escaped with a backslash in front of it.  An
> actual backslash in a path needs to be escape with another backslash.

I find your commit message a bit hard to read to be honest. I had to
read it twice to see what you actually want to add. So it could be
shorten to:

Add support to provide multiple keyspec entries via a single environemnt
(__ENV__) variable.

Furthermore please adapt the Kconfig help message to keep it in sync.

> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>  scripts/keytoc.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 42 insertions(+), 13 deletions(-)
> 
> diff --git a/scripts/keytoc.c b/scripts/keytoc.c
> index 1b99393fdc..c451ee081a 100644
> --- a/scripts/keytoc.c
> +++ b/scripts/keytoc.c
> @@ -10,6 +10,8 @@
>  
>  #pragma GCC diagnostic ignored "-Wdeprecated-declarations" /* ENGINE deprecated in OpenSSL 3.0 */
>  
> +#include "include/string_util.h"
> +
>  #include <stdio.h>
>  #include <string.h>
>  #include <time.h>
> @@ -787,6 +789,11 @@ static bool parse_info(char *p, struct keyinfo *out)
>  
>  static bool parse_keyspec(const char *keyspec, struct keyinfo *out)
>  {
> +	if (!strncmp(keyspec, "pkcs11:", 7)) { /* legacy format of pkcs11 URI */
> +		out->path = strdup(keyspec);
> +		return true;
> +	}
> +
>  	char *sep, *spec;
>  
>  	spec = strdup(keyspec);
> @@ -817,7 +824,7 @@ int main(int argc, char *argv[])
>  {
>  	int argi, opt, ret;
>  	char *outfile = NULL;
> -	int keycount;
> +	size_t keycount, num_positionals;

Why size_t?

>  	struct keyinfo *keylist;
>  
>  	outfilep = stdout;
> @@ -853,22 +860,44 @@ int main(int argc, char *argv[])
>  		exit(1);
>  	}
>  
> -	keycount = argc - optind;
> -	keylist = calloc(sizeof(struct keyinfo), keycount);
>  
> -	for (argi = 0; argi < keycount; argi++) {
> -		const char *keyspec = try_resolve_env(argv[optind + argi]);
> -		struct keyinfo *info = &keylist[argi];
> +	num_positionals = argc - optind;
> +	keycount = num_positionals;
>  
> -		if (!keyspec)
> -			exit(1);
> +	keylist = calloc(keycount, sizeof(*keylist));
> +

No newline required. Furthermore I would prefer a list now since this
would be much easier to read and to handle (without the below
reallocarray).

> +	if (!keylist)
> +		enomem_exit("push");
> +
> +	int listi = 0;
> +
> +	for (argi = 0; argi < num_positionals; argi++) {
> +		char *arg = strdup(argv[optind + argi]);
> +		char *resolved = try_resolve_env(arg);
		^
Why do you drop the 'const' here? Furthermore, why do you strdup()?

> -		if (!strncmp(keyspec, "pkcs11:", 7)) { // legacy format of pkcs11 URI
> -			info->path = strdup(keyspec);
> +		if (arg == resolved) {
> +			keylist[listi].path = arg;
> +			listi++;
>  		} else {
> -			if (!parse_keyspec(keyspec, info)) {
> -				fprintf(stderr, "invalid keyspec %i: %s\n", optind, keyspec);
> -				exit(1);
> +			char *keyspecs = strdup(resolved);
> +			char *keyspec;
> +
> +			/* Keyspec given as env Variable,
> +			 * remove it and add an arbitrary number of keyspecs from its contents
> +			 */
> +			keycount--;
> +			while ((keyspec = strsep_unescaped(&keyspecs, " ", NULL))) {
> +				keycount++;
> +				keylist = reallocarray(keylist, keycount, sizeof(*keylist));
> +				if (!keylist)
> +					enomem_exit("realloc keylist");
> +				bzero(keylist + (keycount - 1), sizeof(*keylist));
> +				if (!parse_keyspec(keyspec, &keylist[listi])) {
> +					fprintf(stderr, "invalid keyspec %i: %s\n", optind,
> +						keyspec);
> +					exit(1);
> +				}
> +				listi++;
>  			}

Did you tested that all the other usecases are not broken by this
change?

Regards,
  Marco

>  		}
>  	}
> 
> -- 
> 2.53.0.308.g50d063e335
> 
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v3 3/4] crypto: keytoc: Split env-provided full keyspec on spaces

[Jonas Rebmann]

Hello Marco,

On 2026-03-17 01:00, Marco Felsch wrote:
> On 26-03-16, Jonas Rebmann wrote:
>> keytoc/CONFIG_CRYPTO_PUBLIC_KEYS can work with a complete keyspec
>> provided by an environment variable as opposed to providing single URIs.
>> This would be a very useful feature if it could also provide any number
>> of keys. Kconfig however provides keytoc with regular keyspecs already
>> split at spaces so without furhter measures, the env variable can only
>> be expanded into a single key.
>>
>> If a complete argument is provided via __ENV, split it at any space
>> character that is not escaped with a backslash in front of it.  An
>> actual backslash in a path needs to be escape with another backslash.
> 
> I find your commit message a bit hard to read to be honest. I had to
> read it twice to see what you actually want to add. So it could be
> shorten to:
> 
> Add support to provide multiple keyspec entries via a single environemnt
> (__ENV__) variable.

No. Earlier iterations would only support a single environment variable
but we decided that this behavior was too inconsistent, hence that all
positionals on keytoc should be handled in the same way, and that a
single positional shouldn't be treated as a special case. This means any
positional can be provided as an environment variable, unlike the RFC
version of this patch that would only handle a single environment
variable.

> Furthermore please adapt the Kconfig help message to keep it in sync.

Could you point me to a part of the Kconfig help text that would be
outdated by this? My intention was to implement the expected behavior
given the current Kconfig help text, and I just double-checked.

>>
>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>> ---
>>   scripts/keytoc.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------------
>>   1 file changed, 42 insertions(+), 13 deletions(-)
>>
>> diff --git a/scripts/keytoc.c b/scripts/keytoc.c
>> index 1b99393fdc..c451ee081a 100644
>> --- a/scripts/keytoc.c
>> +++ b/scripts/keytoc.c
>> @@ -10,6 +10,8 @@
>>   
>>   #pragma GCC diagnostic ignored "-Wdeprecated-declarations" /* ENGINE deprecated in OpenSSL 3.0 */
>>   
>> +#include "include/string_util.h"
>> +
>>   #include <stdio.h>
>>   #include <string.h>
>>   #include <time.h>
>> @@ -787,6 +789,11 @@ static bool parse_info(char *p, struct keyinfo *out)
>>   
>>   static bool parse_keyspec(const char *keyspec, struct keyinfo *out)
>>   {
>> +	if (!strncmp(keyspec, "pkcs11:", 7)) { /* legacy format of pkcs11 URI */
>> +		out->path = strdup(keyspec);
>> +		return true;
>> +	}
>> +
>>   	char *sep, *spec;
>>   
>>   	spec = strdup(keyspec);
>> @@ -817,7 +824,7 @@ int main(int argc, char *argv[])
>>   {
>>   	int argi, opt, ret;
>>   	char *outfile = NULL;
>> -	int keycount;
>> +	size_t keycount, num_positionals;
> 
> Why size_t?

It used to be correct that keycount is an int because it was bounded by
argc. After this change, that is not the case anymore. keycount is an
index to an arbitrary-sized array hence size_t is correct.

Not that I'm expecting over 2147483647 to be a use case here, this is
solely correctness.

>>   	struct keyinfo *keylist;
>>   
>>   	outfilep = stdout;
>> @@ -853,22 +860,44 @@ int main(int argc, char *argv[])
>>   		exit(1);
>>   	}
>>   
>> -	keycount = argc - optind;
>> -	keylist = calloc(sizeof(struct keyinfo), keycount);
>>   
>> -	for (argi = 0; argi < keycount; argi++) {
>> -		const char *keyspec = try_resolve_env(argv[optind + argi]);
>> -		struct keyinfo *info = &keylist[argi];
>> +	num_positionals = argc - optind;
>> +	keycount = num_positionals;
>>   
>> -		if (!keyspec)
>> -			exit(1);
>> +	keylist = calloc(keycount, sizeof(*keylist));
>> +
> 
> No newline required. Furthermore I would prefer a list now since this
> would be much easier to read and to handle (without the below
> reallocarray).

I disagree.

On v2 you asked me to reduce the changes per patch to the bare minimum.

Since this used to be an array and I'm leaving it an array. Converting
it to a linked list, regardless of which variant one finds more
pleasing, would bloat this commit. The conversion to a linked list could
be done in a separate commit but I personally don't find reallocarray
problematic the context, especially given that the array is only ever
resized in the case that was previously broken (that is complete
multi-key keyspec in an environment variable).

>> +	if (!keylist)
>> +		enomem_exit("push");
>> +
>> +	int listi = 0;
>> +
>> +	for (argi = 0; argi < num_positionals; argi++) {
>> +		char *arg = strdup(argv[optind + argi]);
>> +		char *resolved = try_resolve_env(arg);
> 		^
> Why do you drop the 'const' here? Furthermore, why do you strdup()?

try_resolve_env does not return const, if that's what you mean by "drop
the 'const'". It returns either its non-const input or the non-const
return value of getenv. Are you suggesting it should be made const? It
could be.

The strdup of arg is not technically needed here given argv is non-const
and honestly I just wrongly assumed argv as const when I strudped here,
and maybe it should be. But with non-const argv it's not technically
needed. Do you want me to drop it?

If you're referring to the strdup of the result of try_resolve_env; If
it did expand an env variable, it needs to be duplicated before
splitting via strsep_unescaped or a successive getenv call on the same
variable name would unexpectedly return variable contents modified by
the later strsep_unescaped. If this bothers you, I could move that
strdup into try_resolve_env but for the other usages of try_resolve_env
that would not be neccessary.

> 
>> -		if (!strncmp(keyspec, "pkcs11:", 7)) { // legacy format of pkcs11 URI
>> -			info->path = strdup(keyspec);
>> +		if (arg == resolved) {
>> +			keylist[listi].path = arg;
>> +			listi++;
>>   		} else {
>> -			if (!parse_keyspec(keyspec, info)) {
>> -				fprintf(stderr, "invalid keyspec %i: %s\n", optind, keyspec);
>> -				exit(1);
>> +			char *keyspecs = strdup(resolved);
>> +			char *keyspec;
>> +
>> +			/* Keyspec given as env Variable,
>> +			 * remove it and add an arbitrary number of keyspecs from its contents
>> +			 */
>> +			keycount--;
>> +			while ((keyspec = strsep_unescaped(&keyspecs, " ", NULL))) {
>> +				keycount++;
>> +				keylist = reallocarray(keylist, keycount, sizeof(*keylist));
>> +				if (!keylist)
>> +					enomem_exit("realloc keylist");
>> +				bzero(keylist + (keycount - 1), sizeof(*keylist));
>> +				if (!parse_keyspec(keyspec, &keylist[listi])) {
>> +					fprintf(stderr, "invalid keyspec %i: %s\n", optind,
>> +						keyspec);
>> +					exit(1);
>> +				}
>> +				listi++;
>>   			}
> 
> Did you tested that all the other usecases are not broken by this
> change?

That's a too broad question to answer but Patch 4/4 explains backwards
compatibility of this change to the best of my knowledge.

> Regards,
>    Marco
> 
>>   		}
>>   	}
>>
>> -- 
>> 2.53.0.308.g50d063e335
>>
>>
>>
> 

-- 
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

[PATCH v3 4/4] Documentation: migration-guides: Document change in keyspec env vars

[Jonas Rebmann]

Only users providing complete keyspecs containing backslashes or spaces
via an environment variable are affected by this change in the handling
of keytoc's command line arguments.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 Documentation/migration-guides/migration-master.rst | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/migration-guides/migration-master.rst b/Documentation/migration-guides/migration-master.rst
index 42e370d42f..9d1756bdc4 100644
--- a/Documentation/migration-guides/migration-master.rst
+++ b/Documentation/migration-guides/migration-master.rst
@@ -1,5 +1,22 @@
 :orphan:
 
+CONFIG_CRYPTO_PUBLIC_KEYS
+-------------------------
+
+The syntax of keytoc keyspecs when fully provided via an environment variable
+was slightly changed to allow any number of keyspecs to be provided via an
+environment variable. Such environment variables are now split at spaces to be
+interpreted as multiple keyspecs. Any literal spaces and backslashes contained
+in such keyspecs need to be escaped with a backslash.
+
+This only applies to the form:
+
+  CONFIG_CRYPTO_PUBLIC_KEYS="__ENV__A"
+
+While the interpretation of environment variables specifying hint or URI remains unchanged:
+
+  CONFIG_CRYPTO_PUBLIC_KEYS="keyring=kr:__ENV__B"
+
 ARM i.MX HAB
 ------------
 

-- 
2.53.0.308.g50d063e335