Re: [PATCH 1/4] docs: Introduce live patch signing

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On Mon, May 12, 2025 at 1:19 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 09.05.2025 18:18, Ross Lagerwall wrote:
> > --- a/docs/misc/livepatch.pandoc
> > +++ b/docs/misc/livepatch.pandoc
> > @@ -917,6 +917,58 @@ The normal sequence of events is to:
> >   3. *XEN_SYSCTL_LIVEPATCH_ACTION* with *LIVEPATCH_ACTION_APPLY* to apply 
> > the patch.
> >   4. *XEN_SYSCTL_LIVEPATCH_GET* to check the `->rc`. If in *-XEN_EAGAIN* 
> > spin. If zero exit with success.
> >
> > +## Signature Checking
> > +
> > +While loading live patches would generally be restricted to a privileged
> > +process in dom0, in certain cases signature checking in Xen may be 
> > required.
> > +For example, when Secure Boot is enabled live patches need to be verified
> > +before being loaded.
> > +
> > +Xen live patches are ELF binaries but there is no standardized mechanism 
> > for
> > +signing ELF binaries. One approach used by Linux is to append a signature 
> > to
> > +the end of the binary, outside of the ELF container. While this works, it 
> > tends
> > +to be fragile since tools that handle ELF binaries do not correctly handle 
> > the
> > +signature. Instead, the approach taken here is to use an ELF note for the
> > +signature.
> > +
> > +The ELF note section name shall be `.note.Xen.signature` with note name 
> > `Xen`
> > +and type `0`.
> > +
> > +The note data shall contain a header followed by the signature data:
> > +
> > +    #define SIGNATURE_SUPPORTED_VERION 1
>
> I don't think this is a good name (leaving aside the typo); conceptually
> multiple versions could be supported. Otoh live patches are hypervisor
> build specific anyway, so it's hard to see why a version would be needed
> in the first place. Alternatively should version or ...

How about LIVEPATCH_SIGNATURE_VERSION (analogous to
LIVEPATCH_PAYLOAD_VERSION)?

I think keeping the version is harmless and may cover some future
scenario I haven't considered, even if it is not strictly necessary at
the moment.

>
> > +    #define SIGNATURE_ALGORITHM_RSA 0
> > +    #define SIGNATURE_HASH_SHA256 0
>
> ... these two be encoded in the note's type, instead of leaving that
> effectively unused?

That's a good idea. The version, algorithm, and hash can be encoded in
the type and the signature length can be encoded in the note descriptor
length and that would allow removing the signature header completely.

>
> > +    struct payload_signature {
> > +        uint16_t version;
> > +        uint8_t algo;        /* Public-key crypto algorithm */
> > +        uint8_t hash;        /* Digest algorithm */
> > +        uint32_t sig_len;    /* Length of signature data */
>
> Should there be a minimum length specified, to ensure security at least
> for the moment (before e.g. quantum computers can break things)?

I'd prefer to leave that policy to the distros who use this
functionality rather than trying to keep up with what is currently
considered secure.

>
> > +    };
> > +
> > +To sign a live patch:
> > +
> > +1) Add a new note section with a populated payload signature and zeroed out
> > +   signature.
> > +2) Generate a detached signature over the entire binary.
> > +3) Fill in the signature in the note section.
> > +
> > +During live patch load, Xen shall verify the signature using the following
> > +steps:
> > +
> > +1) Copy the signature out of the note section.
> > +2) Zero the signature.
> > +3) Generate a detached signature over the entire binary.
> > +4) Compare against the signature from (1).
> > +
> > +Initially, to avoid including DER / X.509 parsing of certificates, handling
> > +chains, etc. Xen shall verify signatures against a compiled in RSA key in
> > +exponent/modulus form.
>
> And this is sufficient to satisfy (Microsoft's?) requirements?
>

To my knowledge Microsoft has not stated explicitly how various
components in the OS should be verified / secured. However, their
current requirement is that firmware is verified using at least RSA-2048
with SHA-256 so with a sufficiently large key this would cover that,
assuming they apply the same requirement to the OS.

Ross