Re: [docs] [PATCH v2] Document shared state signing

["Antonin Godard" <antonin.godard@bootlin.com>]

Hi,

On Mon Apr 27, 2026 at 5:08 PM CEST, Quentin Schulz wrote:
> Hi Antonin,
>
> On 4/27/26 4:43 PM, Antonin Godard wrote:
>> Hi,
>> 
>> On Fri Apr 24, 2026 at 5:18 PM CEST, Quentin Schulz via lists.yoctoproject.org wrote:
>>> Hi Antonin,
>>>
>>> On 4/21/26 9:19 AM, Antonin Godard via lists.yoctoproject.org wrote:
> [...]
>>>> +   :term:`SSTATE_VALID_SIGS`
>>>> +      When verifying :ref:`shared state <overview-manual/concepts:setscene tasks
>>>> +      and shared state>` artifacts (when :term:`SSTATE_VERIFY_SIG` is set to
>>>> +      "1"), the :term:`SSTATE_VALID_SIGS` variable is a space-separated list of
>>>> +      :wikipedia:`GPG <GNU_Privacy_Guard>` key identifiers to use to verify their
>>>> +      signature.
>>>> +
>>>> +      It must contain the short form identifier of the key pair. For example,
>>>> +      when running the ``gpg --list-keys`` command:
>>>> +
>
> What happens if this variable is empty? My reading of 
> meta/lib/oe/gpg_sign.py:LocalSigner:verify() hints at *any* key that 
> verifies the signature will do.

I think that's what happens, when reading again. I'll add this to the variable's
description.

>>>> +      .. parsed-literal::
>>>> +
>>>
>>> You could use
>>>
>>> .. code-block:: console
>>>
>>>      $ gpg --list-keys
>>>      pub....
>>>
>>> to avoid having to use a literal include.
>> 
>> This use of parsed-literal was intended, as it shows the last 16 characters in
>> bold. I'll add " (in bold text below)" before the colon.
>> 
>
> We can also use
>
> :emphasize-lines: <line-no>[, <line-no>...]
>
> in code-blocks, see 
> https://www.sphinx-doc.org/en/master/usage/restructuredtext/directives.html#directive-option-code-block-emphasize-lines

I want to highlight a specific portion of a line, I don't think this does that?

> [...]
>
>>>> +.. note::
>>>> +
>>>> +   The `siginfo` file is not related to GPG signing.
>>>> +
>>>
>>> But the first file also not since we haven't enabled signing yet, what
>>> did you want to say here?
>> 
>> Avoid confusion as "siginfo" could make someone think of "signatures" and hence
>> signing/gpg. Is adding the note more confusing?
>> 
>
> Ah, yes this makes sense to specify this.
>
> What about
>
> Despite its name, the `siginfo` file is unrelated to GPG signing.

Agreed, clearer

>>>
>>>> +Enabling Shared State Signing
>>>> +-----------------------------
>>>> +
>>>> +Create a new :term:`configuration file` on your host **in a safe location** and
>>>
>>> What makes a location safe?
>> 
>>> I would suggest to put it somewhere in $HOME/.config/ maybe?
>> 
>> Maybe, but I don't think that makes it safer?
>> 
>
> It puts it outside of a possibly versioned repository, but in itself, 
> not better no.
>
>>> Since it contains a secret, we should probably also recommend to do
>>>
>>> chmod o-rwx <file>
>> 
>> Yes, I'll add that.
>> 
>>> no (and possibly chown $USER:$USER <file> ?)
>> 
>> This should be the default when creating the file but I can add that.
>> 
>
> Fair enough.
>
> [...]
>
>>>> +It is advised to put these statements in a separate file as those contain
>>>> +secrets and should not be shared. For this example, let's assume this file is
>>>> +``conf/sstate-sig-key.conf``.
>>>> +
>>>> +These statements define:
>>>> +
>>>> +-  :term:`SSTATE_VERIFY_SIG`: setting this variable to "1" enables the shared
>>>> +   state signing feature.
>>>> +
>>>> +-  :term:`SSTATE_SIG_KEY`: the GPG key identifier for signing shared state
>>>> +   artifacts with the private key.
>>>> +
>>>> +   In our example, this corresponds to the identifier printed with the ``gpg
>>>> +   --list-keys`` command :ref:`above <security-manual/sstate-signing:Generating
>>>> +   A Public And Private Key Pair With GPG>`.
>>>> +
>>>> +-  :term:`SSTATE_SIG_PASSPHRASE`: the passphrase used to protect your private
>>>> +   key when creating the key, chosen when creating the key pair.
>>>> +
>>>
>>> Am I the only bothered this is an option? Is there no way to use a
>>> keyring or the gpg-agent instead?
>> 
>> I don't think so, no.
>> 
>
> Terrible security practice :(

Yeah I agree. Maybe it's worth opening an enhancement "bug" on Bugzilla for
this, what do you think?

>>>> +Let's test our configuration:
>>>> +
>>>> +#. Continuing with our ``gettext-minimal-native`` example, let's first clean the
>>>> +   existing shared state artifacts:
>>>> +
>>>> +   .. code-block:: console
>>>> +
>>>> +      $ bitbake gettext-minimal-native -c cleansstate
>>>> +
>>>
>>> Why do we need to clean the cache (I'm assuming because you want to make
>>> sure the artifacts you sign are the one you just generated and not
>>> malicious ones, but I think it'd be worth explaining this is not a
>>> limitation but a design decision (IFF I really guessed that right)).
>> 
>> I'm giving an example here, and want to make sure my share state artifacts are
>> re-generated.
>> 
>
> Then say this :)

Fair enough, I will!

> [...]
>
>>>> +#. Then, run the task again:
>>>> +
>>>> +   .. code-block:: console
>>>> +
>>>> +      $ bitbake gettext-minimal-native -c create_recipe_spdx
>>>> +
>>>> +#. To make sure the shared state artifact was successfully used, look for the
>>>> +   :ref:`setscene <overview-manual/concepts:setscene tasks and shared state>` task
>>>> +   for ``create_recipe_spdx`` in the latest log file from :term:`BitBake`:
>>>> +
>>>> +   .. code-block:: console
>>>> +
>>>> +      $ grep create_recipe_spdx_setscene tmp/log/cooker/<machine>/console-latest.log
>>>> +      NOTE: Running setscene task 1 of 1 (../layers/openembedded-core/meta/recipes-core/gettext/gettext-minimal-native_1.0.bb:do_create_recipe_spdx_setscene)
>>>> +      NOTE: recipe gettext-minimal-native-1.0-r0: task do_create_recipe_spdx_setscene: Started
>>>> +      NOTE: recipe gettext-minimal-native-1.0-r0: task do_create_recipe_spdx_setscene: Succeeded
>>>> +
>>>> +   Our shared state was successfully verified and used!
>>>
>>> Is there any indication in the logs that verification was actually
>>> performed?
>> 
>> No, I didn't find any in the code, or with any verbosity level
>> 
>
> Maybe something we should add? This seems like an important piece of 
> information to have.

Yes, I agree.

> What about
>
> diff --git a/meta/lib/oe/gpg_sign.py b/meta/lib/oe/gpg_sign.py
> index ede6186c84..fa339644fd 100644
> --- a/meta/lib/oe/gpg_sign.py
> +++ b/meta/lib/oe/gpg_sign.py
> @@ -132,8 +132,11 @@ class LocalSigner(object):
>           status = subprocess.run(cmd, stdout=subprocess.PIPE, 
> stderr=subprocess.PIPE)
>           # Valid if any key matches if unspecified
>           if not valid_sigs:
> -            ret = False if status.returncode else True
> -            return ret
> +            if status.returncode:
> +                return False
> +
> +            bb.note("Signature file %s successfully verified with unspecified key" % (sig_file))

Maybe `bb.debug`?

> +            return True
>
>           import re
>           goodsigs = []
> @@ -145,6 +148,7 @@ class LocalSigner(object):
>
>           for sig in valid_sigs.split():
>               if sig in goodsigs:
> +                bb.note("Signature file %s successfully verified with key %s" % (sig_file, sig))
>                   return True
>           if len(goodsigs):
>               bb.warn('No accepted signatures found. Good signatures 
> found: %s.' % ' '.join(goodsigs))
>
>
> ? I think we may have a few people resisting this as I'm guessing this 
> is on the hot path so adding messages isn't the best (even if disabled 
> most of the time).

I see what you mean. You might be a bit more convincing by passing them through
bb.debug?

Thanks,
Antonin