[PATCH v3 1/2] binman: bintool: add extra_env parameter to run_cmd

[Sergio Prado]

Hi Quentin,

On 4/28/26 X:XX PM, Quentin Schulz wrote:
> Yeah, I'm not convinced this is useful.
>
> We would need to propagate this new variable to whatever calls those
> methods as well. And it quickly balloons.
>
> Just mock the environment with
>
> with unittest.mock.patch.dict('os.environ', {'MYVAR': 'myval'}): like in
> testFitSignPKCS11Object for example?
>
> I'm also very skeptical of the need for binman to *set* environment
> variables to setup openssl.

Yes, that makes sense. It does add unnecessary complexity, especially
if it requires
propagating state across multiple layers. I'll drop this commit and rework
the approach.

> Look at what's been done for testFitSignPKCS11Object, we externally
> configure openssl and softhsm so that binman doesn't have to know about
> it. This avoids having to play smart and adding a bunch of new
> variables. Is it possible to implement something like that for the x509
> certificates? For FIT, we do use fit,engine,
> fit,engine-keydir/key-name-hint with mkimage to (possibly) use pkcs11,
> custom openssl engines, etc..

Agreed. I’ll align with that approach and move the OpenSSL/PKCS#11
configuration fully outside of binman, similar to what is done for FIT
signing.

> Also, I don't think we should decide for the user whether they are
> forced to use a provider or an engine based on the presence of the
> provider? just let them configure openssl to use the one they want
> externally?

Yes, I agree. Letting the user control this via their OpenSSL configuration
is the cleanest approach.

One caveat is that the legacy engine-based flow (e.g. pkcs11 via libp11)
does not integrate transparently with OpenSSL 3’s STORE path (e.g. -key
pkcs11:...) and typically still requires explicit -engine/-keyform engine
usage. I'll elaborate on this in my reply to the second patch, since it
directly affects that part of the implementation.

Thanks for the feedback.

Cheers,

Sergio Prado


Em seg., 27 de abr. de 2026 às 06:48, Quentin Schulz <
quentin.schulz at cherry.de> escreveu:
>
> Hi Sergio,
>
> On 4/23/26 5:45 PM, Sergio Prado wrote:
> > [You don't often get email from sergio.prado at e-labworks.com. Learn why
this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Add an optional extra_env dict parameter to run_cmd() and
> > run_cmd_result(). When provided, the key/value pairs are merged into
> > the subprocess environment for that invocation only, leaving the parent
> > process environment unchanged.
> >
> > This might be needed by bintools that must inject environment variables
> > into specific tool calls without polluting the global environment.
> >
>
> Yeah, I'm not convinced this is useful.
>
> We would need to propagate this new variable to whatever calls those
> methods as well. And it quickly balloons.
>
> Just mock the environment with
>
> with unittest.mock.patch.dict('os.environ', {'MYVAR': 'myval'}): like in
> testFitSignPKCS11Object for example?
>
> I'm also very skeptical of the need for binman to *set* environment
> variables to setup openssl.
>
> Look at what's been done for testFitSignPKCS11Object, we externally
> configure openssl and softhsm so that binman doesn't have to know about
> it. This avoids having to play smart and adding a bunch of new
> variables. Is it possible to implement something like that for the x509
> certificates? For FIT, we do use fit,engine,
> fit,engine-keydir/key-name-hint with mkimage to (possibly) use pkcs11,
> custom openssl engines, etc..
>
> Also, I don't think we should decide for the user whether they are
> forced to use a provider or an engine based on the presence of the
> provider? just let them configure openssl to use the one they want
> externally?
>
> Cheers,
> Quentin