AS: Add support for selectable verifier suites

[Xynnn007]

Proposed in #565 by @tylerfanelli

We soon have two needs when building an AS binary / docker image to decouple verifier dependencies.

1. Select specific verifier suites when building AS binary
2. Install specific verifier software stack due to the selected verifier suites when build docker images

This patch accomplish 1, by disabling all verifiers in CoCoAS basic code and providing a VERIFIER env for makefile to specify the verifier suites.

For 2, we add make dockerfile function which also follows VERIFIER env to install related verifier software stack.
By default the generation of dockerfiles will use all-verifier.

We add target platform detecting logic to the KBS crate cargo toml to determine the built-in AS features.

Also, the CI pipeline of CoCoAS is updated.

Close #568

The RVPS features are a little messy. It will be tidied in #553

cc @tylerfanelli @fitzthum @mkulke

[mkulke]

I wonder whether we can get avoid the dockerfile templating: would something like this work:

# Define the build argument with a default value
ARG INSTALL_EXTRA_DEPS=false

# Use the argument in a conditional statement within the RUN command
RUN if [ "$INSTALL_EXTRA_DEPS" = "true" ]; then \
      echo "Installing extra dependencies" && \
      apt-get update && \
      apt-get install -y some-extra-package; \
    else \
      echo "Skipping extra dependencies"; \
    fi

[Xynnn007]

@mkulke all work but not for COPY command for tss libraries here.

If we add extra RUN rm ... the image will still be huge because the original layer exists.

I did not have a good way for this. Do you have any ideas / Is it a reason?

[mkulke]

@mkulke all work but not for COPY command for tss libraries here.

If we add extra RUN rm ... the image will still be huge because the original layer exists.

I did not have a good way for this. Do you have any ideas / Is it a reason?

why do we copy the tss runtime libraries install of installing them via apt?

edit: the tpm runtime libs should amount to ~1mb on the filesystem, so I wouldn't bother about a toggle for them

[tylerfanelli]

The FEATURES flag was specifically not named VERIFIER because it could be used for more than just verifiers. For example, it could be used like:

FEATURES=restful-bin,rvps-grpc,snp-verifier make grpc-as

to expand to:

cargo build --bin grpc-as --release \
                        --no-default-features \
                        --features grpc-bin,restful-bin,rvps-grpc,snp-verifier

snp-verifier was only one feature that was conditionally enabled. I'm not sure if removing this altogether and just specifying a VERIFIER flag without an alternative for other features is desired.

[Xynnn007]

Yea, I had thought about this by checking inside the existing features. They are three types

1. verifiers: Specify which platform can be supported to verify
2. rvps-*: indicate how to integrate rvps. (grpc or builtin)
3. *-bin: grpc or restful extra dependencies of the binary

1 must be included to Makefile env.

3 is determined by the target binary to be built. If grpc-bin is enabled when building restful-bin, the binary size will not grow. Thus it is not needed to be included in the Makefile env.

What confuses is 2. There are two ways. The PR uses the relatively simplest way that by default both rvps-grpc and rvps-builtin are enabled. Another way is to add an extra flag to control the enablement of rvps-grpc and rvps-builtin. Seeing that cargo build could perform also any feature composition, I chose the way mentioned for ease.

Wdyt?

[tylerfanelli]

Yea, I had thought about this by checking inside the existing features. They are three types

1. verifiers: Specify which platform can be supported to verify

I think this is covered here. Either you specify the verifiers you want with VERIFIER= or it defaults to all-verifier. This sounds good to me.

2. rvps-*: indicate how to integrate rvps. (grpc or builtin)
3. *-bin: grpc or restful extra dependencies of the binary

1 must be included to Makefile env.

3 is determined by the target binary to be built. If grpc-bin is enabled when building restful-bin, the binary size will not grow. Thus it is not needed to be included in the Makefile env.

What confuses is 2. There are two ways. The PR uses the relatively simplest way that by default both rvps-grpc and rvps-builtin are enabled. Another way is to add an extra flag to control the enablement of rvps-grpc and rvps-builtin. Seeing that cargo build could perform also any feature composition, I chose the way mentioned for ease.

Wdyt?

There's no reason someone would want both rvps-grpc and rvps-builtin, correct? They're mutually exclusive. You could add a RVPS= variable, in which a user would specify:

RVPS=grpc make...
RVPS=builtin make...

And it defaults to one of those options...

[Xynnn007]

There's no reason someone would want both rvps-grpc and rvps-builtin, correct? They're mutually exclusive. You could add a RVPS= variable, in which a user would specify:

This is only the build time choosing. The concrete behavior whether grpc or builtin would be used is determined by the config file at runtime. I know that
in customized scenarios, broad-spectrum support is not required, only one-to-one runtime requirements are supported.

Another flag is also good to me. I still want to listen to your ideas with my comments : )

[Xynnn007]

I have a plan to merge rvps-builtin feature into AS code and deprecate the feature as 1) make the configs dependency relationship clear and 2) it does not imports many new crates in #553.

Then, based on your suggestion a flag named GRPC_RVPS can be added to Makefile to indicate whether rvps_grpc is enabled when building restful-as

[tylerfanelli]

Great, let me know when this is ready for another review.

[Xynnn007]

Thanks. I have pushed the changes with a TODO indicating what is going on after this PR

[Xynnn007]

hi @tylerfanelli ptal if it looks good to you

[BbolroC]

Please, update what I've mentioned. Otherwise, the workflow push-as-image-to-ghcr.yaml will trigger unnecessary 6 failing jobs.

[BbolroC]

LGTM thanks @Xynnn007 !

[fitzthum]

LGTM. Let's merge this so we can try to sneak it into the Kata release

[tylerfanelli]

LGTM, thanks for addressing this.