add the fields for signing to crd

[chr15p]

Add fields to the CRD to support kmod signing

This PR just adds the fields, the functionality to actually use them will come in a later PR. This is mostly intended for discussion, to ensure the actual code is built on foundations we agree on, and to try and keep the size of that future PR down to a reviewable and mergeable size (which my previous attempts have not been great at!)

Edit:
updated commit message to say

this add four fields to the crd in their own subsection:

FileList - a list of kmod files within the container to sign

KeySecret and CertSecret - these are the names of secrets containing the private and public keys respectivly that
will be used to sign the kmods. The public key should be the one added into the uefi moklist for secureboot,
and KeySecret is its private component. (See the kernel docs e.g.
https://www.kernel.org/doc/html/v4.15/admin-guide/module-signing.html#manually-signing-modules
for details of this process)

unsignedImage - the optional name of a dtk image containing the unsigned knods, used only if their is no build stage.

The expected workflow will be (not implemented in this commit):
if there is only a kernelmapping.build section it will produce the image kernelmapping.containerImage
(for the non-secureboot case)

if there is both km.build and km.sign then build will produce an intermediate image and sign will
consume that to produce km.containerImage

if there is only km.sign signing will consumse km.sign.unsignedImage and produce km.containerImage
(this is for the case where a vendor supplies prebuilt images that need signing)

the generation of the intermediate image name will be handled automatically within the controller
(again in a different commit) but will probably be something like km.containerImage + "-unsigned"

[k8s-ci-robot]

Hi @chr15p. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[ybettan]

/ok-to-test

[ybettan]

@qbarrand We need to trigger the Github actions here as well. I don't think I have enough privileges to do it (can't see the workflow tab).

[ybettan]

Can you please add a commit message describing those changes?

[ybettan]

Why do we need a public key for signing purposes?

[ybettan]

Few questions:
What is the expected flow here? Can we have it in the commit message (and PR description as well)? Are we pulling from the unsignedImage, signing, and then what? pushing somewhere? are we overriding the unsigned image? pushing a new tag? using the same tag as the containerImage? Do we need to mention the new tag name in the Sign object as we do for the unsignedImage?

[chr15p]

written a better commit message that hopefully explains my thinking (I've been staring at this stuff so long I forget it isn't obvious to other people! )

The signing process itself will use the sign-file binary distributed with the kernel source (see https://www.kernel.org/doc/html/v4.15/admin-guide/module-signing.html#manually-signing-modules) and that requires the public and private key, so the simple answer is we need both keys/certs as input parameters to that code. We could write our own signing code but that seems fragile.

[ybettan]

Thanks for updating the commit message.

unsignedImage - the optional name of a dtk image containing the unsigned knods, used only if their is no build stage.

How DTK is related here? We don't have DTK u/s. I guess you meant "driver container" instead of "dtk"?! (If this is the case, please update the commit message as well).

Besides that, LGTM.

[ybettan]

/approve
/cc @qbarrand in case he also have something to say.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chr15p, ybettan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ybettan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[qbarrand]

Two small comments, lgtm otherwise. Thanks!

[codecov-commenter]

Codecov Report

Base: 73.46% // Head: 73.46% // No change to project coverage 👍

Coverage data is based on head (9e40749) compared to base (2f48ff2).
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #91   +/-   ##
=======================================
  Coverage   73.46%   73.46%           
=======================================
  Files          17       17           
  Lines        1771     1771           
=======================================
  Hits         1301     1301           
  Misses        404      404           
  Partials       66       66           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[qbarrand]

/lgtm