initdata: use annotation to provision config files

[huoqifeng]

Fixes: #1895

• depends on kata-containers change in initdata: add initdata annotation in hypervisor config kata-containers/kata-containers#9988

[huoqifeng]

Create a peerpod against pod yaml as below:

apiVersion: v1
kind: Pod
metadata:
  labels:
    run: busybox
  name: busybox
  annotations:
    io.katacontainers.config.runtime.cc_init_data: ewogICAgImFsZ29yaXRobSI6ICJzaGEzODQiLAogICAgInZlcnNpb24iOiAiMC4xLjAiLAogICAgImRhdGEiOiB7CiAgICAgICJhYS50b21sIjogIlt0b2tlbl9jb25maWdzXVxuW3Rva2VuX2NvbmZpZ3MuY29jb19hc11cbnVybCA9IFwiaHR0cDovLzEyNy4wLjAuMTo4MDgwXCJcblxuW3Rva2VuX2NvbmZpZ3Mua2JzXVxudXJsID0gXCJodHRwOi8vMTI3LjAuMC4xOjgwODBcIlxuIiwKICAgICAgImNkaC50b21sIjogInNvY2tldCA9IFxuY3JlZGVudGlhbHMgPSBbXVxuW2tiY11cbm5hbWUgPSBcImNjX2tiY1wiXG51cmwgPSBcImh0dHA6Ly8xLjIuMy40OjgwODBcIiIsCiAgICAgICJwb2xpY3kucmVnbyI6ICJwYWNrYWdlIGFnZW50X3BvbGljeVxuaW1wb3J0IGZ1dHVyZS5rZXl3b3Jkcy5pblxuaW1wb3J0IGZ1dHVyZS5rZXl3b3Jkcy5ldmVyeVxuaW1wb3J0IGlucHV0XG5cbiMgRGVmYXVsdCB2YWx1ZXMsIHJldHVybmVkIGJ5IE9QQSB3aGVuIHJ1bGVzIGNhbm5vdCBiZSBldmFsdWF0ZWQgdG8gdHJ1ZS5cbmRlZmF1bHQgQ29weUZpbGVSZXF1ZXN0IDo9IGZhbHNlXG5kZWZhdWx0IENyZWF0ZUNvbnRhaW5lclJlcXVlc3QgOj0gZmFsc2VcbmRlZmF1bHQgQ3JlYXRlU2FuZGJveFJlcXVlc3QgOj0gdHJ1ZVxuZGVmYXVsdCBEZXN0cm95U2FuZGJveFJlcXVlc3QgOj0gdHJ1ZVxuZGVmYXVsdCBFeGVjUHJvY2Vzc1JlcXVlc3QgOj0gZmFsc2VcbmRlZmF1bHQgR2V0T09NRXZlbnRSZXF1ZXN0IDo9IHRydWVcbmRlZmF1bHQgR3Vlc3REZXRhaWxzUmVxdWVzdCA6PSB0cnVlXG5kZWZhdWx0IE9ubGluZUNQVU1lbVJlcXVlc3QgOj0gdHJ1ZVxuZGVmYXVsdCBQdWxsSW1hZ2VSZXF1ZXN0IDo9IHRydWVcbmRlZmF1bHQgUmVhZFN0cmVhbVJlcXVlc3QgOj0gZmFsc2VcbmRlZmF1bHQgUmVtb3ZlQ29udGFpbmVyUmVxdWVzdCA6PSB0cnVlXG5kZWZhdWx0IFJlbW92ZVN0YWxlVmlydGlvZnNTaGFyZU1vdW50c1JlcXVlc3QgOj0gdHJ1ZVxuZGVmYXVsdCBTaWduYWxQcm9jZXNzUmVxdWVzdCA6PSB0cnVlXG5kZWZhdWx0IFN0YXJ0Q29udGFpbmVyUmVxdWVzdCA6PSB0cnVlXG5kZWZhdWx0IFN0YXRzQ29udGFpbmVyUmVxdWVzdCA6PSB0cnVlXG5kZWZhdWx0IFR0eVdpblJlc2l6ZVJlcXVlc3QgOj0gdHJ1ZVxuZGVmYXVsdCBVcGRhdGVFcGhlbWVyYWxNb3VudHNSZXF1ZXN0IDo9IHRydWVcbmRlZmF1bHQgVXBkYXRlSW50ZXJmYWNlUmVxdWVzdCA6PSB0cnVlXG5kZWZhdWx0IFVwZGF0ZVJvdXRlc1JlcXVlc3QgOj0gdHJ1ZVxuZGVmYXVsdCBXYWl0UHJvY2Vzc1JlcXVlc3QgOj0gdHJ1ZVxuZGVmYXVsdCBXcml0ZVN0cmVhbVJlcXVlc3QgOj0gZmFsc2UiCiAgICB9Cn0=
spec:
  containers:
  - image: quay.io/prometheus/busybox
    name: busybox
    resources: {}
  dnsPolicy: ClusterFirst
  restartPolicy: Never
  runtimeClassName: kata-remote

Where io.katacontainers.config.runtime.cc_init_data comes from base64 string of:

{
    "algorithm": "sha384",
    "version": "0.1.0",
    "data": {
      "aa.toml": "[token_configs]\n[token_configs.coco_as]\nurl = \"http://127.0.0.1:8080\"\n\n[token_configs.kbs]\nurl = \"http://127.0.0.1:8080\"\n",
      "cdh.toml": "socket = \ncredentials = []\n[kbc]\nname = \"cc_kbc\"\nurl = \"http://1.2.3.4:8080\"",
      "policy.rego": "package agent_policy\nimport future.keywords.in\nimport future.keywords.every\nimport input\n\n# Default values, returned by OPA when rules cannot be evaluated to true.\ndefault CopyFileRequest := false\ndefault CreateContainerRequest := false\ndefault CreateSandboxRequest := true\ndefault DestroySandboxRequest := true\ndefault ExecProcessRequest := false\ndefault GetOOMEventRequest := true\ndefault GuestDetailsRequest := true\ndefault OnlineCPUMemRequest := true\ndefault PullImageRequest := true\ndefault ReadStreamRequest := false\ndefault RemoveContainerRequest := true\ndefault RemoveStaleVirtiofsShareMountsRequest := true\ndefault SignalProcessRequest := true\ndefault StartContainerRequest := true\ndefault StatsContainerRequest := true\ndefault TtyWinResizeRequest := true\ndefault UpdateEphemeralMountsRequest := true\ndefault UpdateInterfaceRequest := true\ndefault UpdateRoutesRequest := true\ndefault WaitProcessRequest := true\ndefault WriteStreamRequest := false"
    }
}

Check it on s390x for libvirt provider, in created PeerPod, check the files under /run/peerpod:

# ls /run/peerpod/
aa.toml  agent-config.toml  cdh.toml  checksum.txt  daemon.json  initdata.json  policy.rego

Make it ready to review and I'll add unit test code according to review comments.

[huoqifeng]

Use pod yaml:

apiVersion: v1
kind: Pod
metadata:
  labels:
    run: busybox
  name: busybox
  annotations:
    io.katacontainers.config.runtime.cc_init_data: YWxnb3JpdGhtID0gInNoYTM4NCIKdmVyc2lvbiA9ICIwLjEuMCIKCltkYXRhXQoiYWEudG9tbCIgPSAnJycKW3Rva2VuX2NvbmZpZ3NdClt0b2tlbl9jb25maWdzLmNvY29fYXNdCnVybCA9ICdodHRwOi8vMTI3LjAuMC4xOjgwODAnCgpbdG9rZW5fY29uZmlncy5rYnNdCnVybCA9ICdodHRwOi8vMTI3LjAuMC4xOjgwODAnCicnJwoKImNkaC50b21sIiAgPSAnJycKc29ja2V0ID0gJ3VuaXg6Ly8vcnVuL2NvbmZpZGVudGlhbC1jb250YWluZXJzL2NkaC5zb2NrJwpjcmVkZW50aWFscyA9IFtdCgpba2JjXQpuYW1lID0gJ2NjX2tiYycKdXJsID0gJ2h0dHA6Ly8xLjIuMy40OjgwODAnCicnJwoKInBvbGljeS5yZWdvIiA9ICcnJwpwYWNrYWdlIGFnZW50X3BvbGljeQoKaW1wb3J0IGZ1dHVyZS5rZXl3b3Jkcy5pbgppbXBvcnQgZnV0dXJlLmtleXdvcmRzLmV2ZXJ5CgppbXBvcnQgaW5wdXQKCiMgRGVmYXVsdCB2YWx1ZXMsIHJldHVybmVkIGJ5IE9QQSB3aGVuIHJ1bGVzIGNhbm5vdCBiZSBldmFsdWF0ZWQgdG8gdHJ1ZS4KZGVmYXVsdCBDb3B5RmlsZVJlcXVlc3QgOj0gZmFsc2UKZGVmYXVsdCBDcmVhdGVDb250YWluZXJSZXF1ZXN0IDo9IGZhbHNlCmRlZmF1bHQgQ3JlYXRlU2FuZGJveFJlcXVlc3QgOj0gdHJ1ZQpkZWZhdWx0IERlc3Ryb3lTYW5kYm94UmVxdWVzdCA6PSB0cnVlCmRlZmF1bHQgRXhlY1Byb2Nlc3NSZXF1ZXN0IDo9IGZhbHNlCmRlZmF1bHQgR2V0T09NRXZlbnRSZXF1ZXN0IDo9IHRydWUKZGVmYXVsdCBHdWVzdERldGFpbHNSZXF1ZXN0IDo9IHRydWUKZGVmYXVsdCBPbmxpbmVDUFVNZW1SZXF1ZXN0IDo9IHRydWUKZGVmYXVsdCBQdWxsSW1hZ2VSZXF1ZXN0IDo9IHRydWUKZGVmYXVsdCBSZWFkU3RyZWFtUmVxdWVzdCA6PSBmYWxzZQpkZWZhdWx0IFJlbW92ZUNvbnRhaW5lclJlcXVlc3QgOj0gdHJ1ZQpkZWZhdWx0IFJlbW92ZVN0YWxlVmlydGlvZnNTaGFyZU1vdW50c1JlcXVlc3QgOj0gdHJ1ZQpkZWZhdWx0IFNpZ25hbFByb2Nlc3NSZXF1ZXN0IDo9IHRydWUKZGVmYXVsdCBTdGFydENvbnRhaW5lclJlcXVlc3QgOj0gdHJ1ZQpkZWZhdWx0IFN0YXRzQ29udGFpbmVyUmVxdWVzdCA6PSB0cnVlCmRlZmF1bHQgVHR5V2luUmVzaXplUmVxdWVzdCA6PSB0cnVlCmRlZmF1bHQgVXBkYXRlRXBoZW1lcmFsTW91bnRzUmVxdWVzdCA6PSB0cnVlCmRlZmF1bHQgVXBkYXRlSW50ZXJmYWNlUmVxdWVzdCA6PSB0cnVlCmRlZmF1bHQgVXBkYXRlUm91dGVzUmVxdWVzdCA6PSB0cnVlCmRlZmF1bHQgV2FpdFByb2Nlc3NSZXF1ZXN0IDo9IHRydWUKZGVmYXVsdCBXcml0ZVN0cmVhbVJlcXVlc3QgOj0gZmFsc2UKJycn
spec:
  containers:
  - image: quay.io/prometheus/busybox
    name: busybox
    resources: {}
  dnsPolicy: ClusterFirst
  restartPolicy: Never
  runtimeClassName: kata-remote

Where io.katacontainers.config.runtime.cc_init_data comes from base64 string of toml:

algorithm = "sha384"
version = "0.1.0"

[data]
"aa.toml" = '''
[token_configs]
[token_configs.coco_as]
url = 'http://127.0.0.1:8080'

[token_configs.kbs]
url = 'http://127.0.0.1:8080'
'''

"cdh.toml"  = '''
socket = 'unix:///run/confidential-containers/cdh.sock'
credentials = []

[kbc]
name = 'cc_kbc'
url = 'http://1.2.3.4:8080'
'''

"policy.rego" = '''
package agent_policy

import future.keywords.in
import future.keywords.every

import input

# Default values, returned by OPA when rules cannot be evaluated to true.
default CopyFileRequest := false
default CreateContainerRequest := false
default CreateSandboxRequest := true
default DestroySandboxRequest := true
default ExecProcessRequest := false
default GetOOMEventRequest := true
default GuestDetailsRequest := true
default OnlineCPUMemRequest := true
default PullImageRequest := true
default ReadStreamRequest := false
default RemoveContainerRequest := true
default RemoveStaleVirtiofsShareMountsRequest := true
default SignalProcessRequest := true
default StartContainerRequest := true
default StatsContainerRequest := true
default TtyWinResizeRequest := true
default UpdateEphemeralMountsRequest := true
default UpdateInterfaceRequest := true
default UpdateRoutesRequest := true
default WaitProcessRequest := true
default WriteStreamRequest := false
'''

Check PeerPod:

# ls /run/peerpod/
aa.toml  agent-config.toml  cdh.toml  checksum.txt  daemon.json  initdata.meta  policy.rego

[mkulke]

I think we want to bump the kata containers revision a commit that works?

I think we should remove AA_KBC_PARAMS support together with this change, otherwise it would be weird to have per-deployment and per-pod aa/cdh configuration overlapping.

[huoqifeng]

I think we want to bump the kata containers revision a commit that works?

I think we should remove AA_KBC_PARAMS support together with this change, otherwise it would be weird to have per-deployment and per-pod aa/cdh configuration overlapping.

Thanks @mkulke for remind it, I just realized KBS is deployed dynamically in e2e test and using AA_KBC_PARAMS here https://github.com/confidential-containers/cloud-api-adaptor/blob/main/src/cloud-api-adaptor/test/e2e/main_test.go#L189, Maybe we need keep it before change the KBS deployment in e2e test.

[mkulke]

looks good! a couple of nits

[bpradipt]

Few things are unclear to me (most likely I missed) and hence asking question here

1. Will initData support via annotation be the only way to configure aa and cdh?
2. If the answer to 1 is yes, will AA_KBC_PARAMS still be used to set the annotation in the pod spec ?

Also, I think if some commits are squashed and cleaned up it'll be easier to review :-)

[huoqifeng]

2. AA_KBC_PARAMS

@bpradipt AA_KBC_PARAMS is not removed is because e2e test deploy KBS dynamically and the test cases leverage it. I'll considering drop AA_KBC_PARAMS after revise the related test cases. And I'll squash the commits later.

[mkulke]

@bpradipt AA_KBC_PARAMS is not removed is because e2e test deploy KBS dynamically and the test cases leverage it. I'll considering drop AA_KBC_PARAMS after revise the related test cases. And I'll squash the commits later.

I think there are 2 issues we have to consider before merging init-data

1. It is a kind of undefined/undocumented breaking change. If I deploy a pod with initdata annotation my AA_KBC_PARAMs will be overwritten (because write_files is processed first, then initdata overwrites the same files). I would avoid that.
2. There is no e2e tests covering this feature, which generally is something that we should avoid.
3. The initdata feature is not 100% ready, we still need to write code that is consuming initdata.digest.

So, one idea to still get this merged in this state would be:

• Comment out the initdata file-provisioning code, only calculate/write initdata.digest. We can start to write code that is working with initdata.digest.
• The AA_KBC_PARAMS mechanism will stay in place and work as expected in the meantime.
• In a (atomic) successor PR we:
  • Remove the AA_KBC_PARAMS logic
  • Adjust documentation
  • Enable file provisioning from initdata
  • Convert the kbs e2e tests to use initdata. Ideally those would use KBS cert + measurements/runtime_data checks for initdata.digest.

btw, I don't know how the remote attestation verification of initdata would look like in the sample attester, it probably won't work unless we extend the sample attester to keep some in-memory log that is attached to the evidence?

[huoqifeng]

@bpradipt AA_KBC_PARAMS is not removed is because e2e test deploy KBS dynamically and the test cases leverage it. I'll considering drop AA_KBC_PARAMS after revise the related test cases. And I'll squash the commits later.

I think there are 2 issues we have to consider before merging init-data

1. It is a kind of undefined/undocumented breaking change. If I deploy a pod with initdata annotation my AA_KBC_PARAMs will be overwritten (because write_files is processed first, then initdata overwrites the same files). I would avoid that.

2. There is no e2e tests covering this feature, which generally is something that we should avoid.

3. The initdata feature is not 100% ready, we still need to write code that is consuming initdata.digest.

So, one idea to still get this merged in this state would be:

* Comment out the initdata file-provisioning code, only calculate/write initdata.digest. We can start to write code that is working with initdata.digest.

* The AA_KBC_PARAMS mechanism will stay in place and work as expected in the meantime.

* In a (atomic) successor PR we:
  
  * Remove the AA_KBC_PARAMS logic
  * Adjust documentation
  * Enable file provisioning from initdata
  * Convert the kbs e2e tests to use initdata. Ideally those would use KBS cert + measurements/runtime_data checks for initdata.digest.

btw, I don't know how the remote attestation verification of initdata would look like in the sample attester, it probably won't work unless we extend the sample attester to keep some in-memory log that is attached to the evidence?

The key problem to remove AA_KBC_PARAMs and not be able to add e2e test cases for initdata is because the KBS deployment in our e2e test, which is dynamic and requires to update initdata in test case, it's doable but need more times. It's simpler in a real deployment or production env. Maybe, we can keep AA_KBC_PARAMs as-is and use initdata to provision all the configures only if the annotation provided to make sure not breaking existing algorithm. It's experimental phase. We then add test cases for initdata and drop AA_KBC_PARAMs in successor PRs. @bpradipt @mkulke what do you think?

[mkulke]

Maybe, we can keep AA_KBC_PARAMs as-is and use initdata to provision all the configures only if the annotation provided to make sure not breaking existing algorithm

Not sure I understand fully. Do you mean if AA_KBC_PARAMS is set we ignore the initdata annotation? if that's the case: yes, that would work, I think. It would remove the ambiguities of who is actually writing cdh.toml + aa.toml. I woulld add a note in the log that we ignore initdata annotation, though.

[bpradipt]

Maybe, we can keep AA_KBC_PARAMs as-is and use initdata to provision all the configures only if the annotation provided to make sure not breaking existing algorithm

Not sure I understand fully. Do you mean if AA_KBC_PARAMS is set we ignore the initdata annotation? if that's the case: yes, that would work, I think. It would remove the ambiguities of who is actually writing cdh.toml + aa.toml. I woulld add a note in the log that we ignore initdata annotation, though.

This makes sense. If AA_KBC_PARAMS is set CAA can ignore the initdata annotation and log it.

[mkulke]

thanks for taking this on - and accommodating all the suggestions!