Kbs key release rework

[stevenhorsman]

The KbsKeyRelease tests have had errors setting the secret key, which are revealed when trace is on:

=== RUN   TestLibvirtKbsKeyRelease
time="2024-09-24T13:53:12Z" level=info msg="set key resource: ../../kbs/config/kubernetes/overlays/x86_64/key.bin"
time="2024-09-24T13:53:12Z" level=trace msg="./kbs-client --url http://192.168.122.140:31680 config --auth-private-key ../../kbs/config/kubernetes/base/kbs.key set-resource --path reponame/workload_key/key.bin --resource-file ../../kbs/config/kubernetes/overlays/x86_64/key.bin, output: Error: Request Failed, Response: \"{\\\"type\\\":\\\"https://github.com/confidential-containers/kbs/errors/SetSecretFailed\\\",\\\"detail\\\":\\\"Set secret failed: write local fs\\\"}\"\n"

I believe this is base the reponame/workload_key directory is created by the KBS at start-up, so it owns the resource. It also leads to error is the KBS log:

[2024-09-24T11:11:40Z INFO  kbs] Using config file /etc/kbs/kbs-config.toml
[2024-09-24T11:11:40Z WARN  attestation_service::rvps] No RVPS address provided and will launch a built-in rvps
[2024-09-24T11:11:40Z INFO  attestation_service::token::simple] No Token Signer key in config file, create an ephemeral key and without CA pubkey cert
[2024-09-24T11:11:40Z INFO  kbs] Starting HTTP server at [0.0.0.0:8080]
[2024-09-24T11:11:40Z INFO  actix_server::builder] starting 4 workers
[2024-09-24T11:11:40Z INFO  actix_server::server] Tokio runtime found; starting in existing Tokio runtime
[2024-09-24T13:07:50Z ERROR kbs::http::error] Set secret failed: write local fs

which isn't helpful when debugging.

This PR changes the secret to be in a different directory and also does some refactoring so that a more dynamic secret can be set to avoid use finding errors late and reduce duplication.

[wainersm]

Hi @stevenhorsman !

Could you do me a favor? I need the following change to be able to install kbs on my environment (Fedora); according to uname man-page, -i is a non-portable option and in my env it returns unknown (rather than x86_64). I don't know if that change breaks on s390x (I hope note).

diff --git a/src/cloud-api-adaptor/test/provisioner/trustee_kbs.go b/src/cloud-api-adaptor/test/provisioner/trustee_kbs.go
index a628b1f..cf97351 100644
--- a/src/cloud-api-adaptor/test/provisioner/trustee_kbs.go
+++ b/src/cloud-api-adaptor/test/provisioner/trustee_kbs.go
@@ -29,7 +29,7 @@ import (
 const TRUSTEE_REPO_PATH = "../trustee"
 
 func getHardwarePlatform() (string, error) {
-       out, err := exec.Command("uname", "-i").Output()
+       out, err := exec.Command("uname", "-m").Output()
        return strings.TrimSuffix(string(out), "\n"), err
 }

[stevenhorsman]

Hi @stevenhorsman !

Could you do me a favor? I need the following change to be able to install kbs on my environment (Fedora); according to uname man-page, -i is a non-portable option and in my env it returns unknown (rather than x86_64). I don't know if that change breaks on s390x (I hope note).

diff --git a/src/cloud-api-adaptor/test/provisioner/trustee_kbs.go b/src/cloud-api-adaptor/test/provisioner/trustee_kbs.go
index a628b1f..cf97351 100644
--- a/src/cloud-api-adaptor/test/provisioner/trustee_kbs.go
+++ b/src/cloud-api-adaptor/test/provisioner/trustee_kbs.go
@@ -29,7 +29,7 @@ import (
 const TRUSTEE_REPO_PATH = "../trustee"
 
 func getHardwarePlatform() (string, error) {
-       out, err := exec.Command("uname", "-i").Output()
+       out, err := exec.Command("uname", "-m").Output()
        return strings.TrimSuffix(string(out), "\n"), err
 }

Sure - we use uname -m everywhere else, so I can swap it here

[mkulke]

I also attempted to reduce the brittleness of kbs-related tests and ended up with

cloud-api-adaptor/src/cloud-api-adaptor/test/e2e/remote_attestation.go

Line 12 in a670a9e

func DoTestRemoteAttestation(t *testing.T, e env.Environment, assert CloudAssert, kbsEndpoint string) {

this test will only acquire a kbs token that could be used to retrieve secrets, but it actually doesn't retrieve any, so we don't have to bother about secret provisioning, but it'll still cover einitdata, cc_kbc configuration and remote attestation.

[stevenhorsman]

I also attempted to reduce the brittleness of kbs-related tests and ended up with

cloud-api-adaptor/src/cloud-api-adaptor/test/e2e/remote_attestation.go

Line 12 in a670a9e

func DoTestRemoteAttestation(t *testing.T, e env.Environment, assert CloudAssert, kbsEndpoint string) {

this test will only acquire a kbs token that could be used to retrieve secrets, but it actually doesn't retrieve any, so we don't have to bother about secret provisioning, but it'll still cover einitdata, cc_kbc configuration and remote attestation.

Cool - I knew I'd seen a PR from you updating this code recently, but couldn't find it. It feels like that test is mostly a subset of the testing of TestKbsKeyRelease, but doesn't require the KBS to be deployed in our set-up, so maybe there are overlaps with TestTrusteeOperatorKeyReleaseForSpecificKey which I think is trying to test a similar remote attestation set-up?

[mkulke]

Cool - I knew I'd seen a PR from you updating this code recently, but couldn't find it. It feels like that test is mostly a subset of the testing of TestKbsKeyRelease, but doesn't require the KBS to be deployed in our set-up, so maybe there are overlaps with TestTrusteeOperatorKeyReleaseForSpecificKey which I think is trying to test a similar remote attestation set-up?

well. it does require KBS to be deployed, since this is the remote party of the remote attestation. what it doesn't need is actual secrets to be copied around and asserted.

[stevenhorsman]

well. it does require KBS to be deployed, since this is the remote party of the remote attestation. what it doesn't need is actual secrets to be copied around and asserted.

But the KBS doesn't need to be deployed by our test code for this test, just the KBS endpoint supplied as an envvar?

[mkulke]

But the KBS doesn't need to be deployed by our test code for this test, just the KBS endpoint supplied as an envvar?

it does need to be deployed, otherwise it wouldn't be able to test remote attestation. what the test doesn't bother with is secrets.

kbs is actually 2 services (logically, since you can run them in a single instance). a token-kbs and a resource-kbs, the token-kbs does the remote attestation, hands out a token, and using this token you can go to resource-kbs to get the secret.

that's why the cdh config has seemingly redundant kbs url entries. in a real deployment you might want token-kbs and resource-kbs to be different systems.

[wainersm]

Hi @stevenhorsman ! LGTM. Thanks for this!