Cloud-Api-Adaptor: PP Secure Comms #1776
Conversation
davidhadas
force-pushed
the
PpSecureComms
branch
2 times, most recently
from
b57eab5
to
0cda75a
Compare
davidhadas
changed the title
davidhadas
force-pushed
the
PpSecureComms
branch
from
58e0a7f
to
412662d
Compare
davidhadas
changed the title
davidhadas
force-pushed
the
PpSecureComms
branch
2 times, most recently
from
f5c47fb
to
478d901
Compare
bpradipt
requested review from
yoheiueda,
mkulke,
snir911,
huoqifeng and
stevenhorsman
davidhadas
force-pushed
the
PpSecureComms
branch
2 times, most recently
from
8786e5d
to
c4c6223
Compare
|
I added standalone testing - you may try out the Secure Comms feature outside of a CoCo environment using this standalone test. |
davidhadas
force-pushed
the
PpSecureComms
branch
6 times, most recently
from
0862ea0
to
322be5c
Compare
| - apiGroups: [""] | ||
| resources: ["secrets"] | ||
| verbs: ["create", "patch", "update", "get", "watch", "list"] | ||
| --- |
There was a problem hiding this comment.
Why it is needed? can access be limited to specific secret?
resourceNames: ["my-configmap"]
There was a problem hiding this comment.
The implementation is creating secrets named pp-<peer pod SID> under the namespace. These secrets include a key pair for each PP. This will be used to regain PP keys following CAA Pod restart.
See src/cloud-api-adaptor/docs/SecureComms - Is this enough? |
davidhadas
force-pushed
the
PpSecureComms
branch
from
322be5c
to
d12ec61
Compare
davidhadas
force-pushed
the
PpSecureComms
branch
from
d12ec61
to
f2e80b4
Compare
Resolved |
|
@davidhadas overall the code looks good to me. I only had some comments w.r.to better readability. |
|
I tested this on my setup using the docker provider. I would have expected the sample attester to work. Need to check my AA build. |
@yoheiueda for secure-comms CAA is creating a key pair for the PP, using peer-pod controller will not be feasible as I see it. |
|
I'm able to run it successfully using docker provider. My issue was KBS and I had to use the following images kubectl set image -n kbs-operator-system deployment/kbs-deployment kbs=ghcr.io/confidential-containers/staged-images/kbs-grpc-as:latest as=ghcr.io/confidential-containers/staged-images/coco-as-grpc:latest rvps=ghcr.io/confidential-containers/staged-images/rvps:latest Here are the relevant cloud-api-adaptor logs: Here are the entries in kbs |
davidhadas
force-pushed
the
PpSecureComms
branch
3 times, most recently
from
0affa49
to
d09c78f
Compare
There was a problem hiding this comment.
/lgtm
Thanks @davidhadas for your patience and addressing the review comments.
|
|
||
| ## Testing | ||
|
|
||
| Testing securecomms as a standalone can be done by using: |
There was a problem hiding this comment.
Just to note that with the cluster, kbs and secure comms set up I got this working:
# go run ./test/securecomms/double/main.go
2024/05/02 12:35:54 [secure-comms] Using PP SecureComms: InitSshServer version v0.2
2024/05/02 12:35:54 [secure-comms] Inbound listening to port 7000
2024/05/02 12:35:54 [secure-comms] Inbound listening to port 16443
2024/05/02 12:35:54 [secure-comms] Inbound listening to port 9053
...
2024/05/02 12:36:02 [secure-comms] Attestation phase: starting
2024/05/02 12:36:02 [secure-comms] Attestation phase: client connected
2024/05/02 12:36:02 [secure-comms] Attestation phase: connected
2024/05/02 12:36:02 [secure-comms] Attestation phase: ssh connected - 127.0.0.1:2222
2024/05/02 12:36:03 [secure-comms] Attestation phase: SSH server initialized keys
2024/05/02 12:36:03 [secure-comms] Attestation phase: SSH server initialized with NoClientAuth
2024/05/02 12:36:03 [secure-comms] Attestation phase: ssh skipped validating server's host key (type ssh-rsa) during attestation
2024/05/02 12:36:03 [secure-comms] Attestation phase: logged-in without key
2024/05/02 12:36:03 [secure-comms] Attestation phase: peer reported phase Attestation
2024/05/02 12:36:03 [secure-comms] Attestation phase: peer reported phase Attestation
...
2024/05/02 12:36:03 [secure-comms] Attestation phase: done
HttpClient start : http://127.0.0.1:45561
HttpClient sending req: http://127.0.0.1:45561
2024/05/02 12:36:03 [secure-comms] Kubernetes phase: starting (number of restarts 0)
2024/05/02 12:36:03 [secure-comms] Kubernetes phase: ssh connected - 127.0.0.1:2222
2024/05/02 12:36:03 [secure-comms] Kubernetes client connected
2024/05/02 12:36:03 [secure-comms] Kubernetes phase: connected
...
2024/05/02 12:36:04 [secure-comms] Kubernetes phase: done
2024/05/02 12:36:04 [secure-comms] SshClientInstance DisconnectPP success
2024/05/02 12:36:04 [secure-comms] DeleteSecret 'pp-sid'
*** SUCCESS ***
Maybe not what I'd describe as standalone as so, but it's good to get it working :)
|
|
||
| ## Future Plans | ||
|
|
||
| - Add DeleteResource() support in KBS, KBC, api-server-rest, than cleanup resources added by Secure Comms to KBS whenever a Peer Pod fail to be created or when a Peer Pod is terminated. |
There was a problem hiding this comment.
I would like to see Pradipta's suggestion of moving the agent-protocol-forwarder config to a file and provide it via user-data which process-user-data can read and create the config, such that we don't need a separate podvm build to use this feature, on the future plans list as that would help enable much easier testing & release of this feature.
There was a problem hiding this comment.
Since this doc is specific for the secure comms and using a config file for agent-protocol-forwarder is not really specific to secure comms, my suggestion will be to create a tracker issue in this repo.
There was a problem hiding this comment.
There is a lot of code here and the networking is not much speciality, so I've not done the most in-depth code review, but I've tried following the docs and got the tests passing and given that this is an optional feature that is off by default, and this is the first step on the journey, then I don't see an issue with merging it personally. I would like to hear from some of the other reviewers who gave comments and offer more value than I do! Thanks for the updates and patience @davidhadas
Secure Comms feature to secure communication between the cluster and the Peer Pods. Rely on KBS keys to establish an ssh channel between the cluster WN and the PP. The ssh channel can then be used to tunnel different communications to/from the PP. Use peer-pods-cm to initialize adaptor's secure-comms Use agent-protocol-forwarder.service to initialize forwarder's secure-comms Includes testing for Secure-Comms running as a stand-alone Includes unit testing for Secure-Comms Signed-off-by: David Hadas <david.hadas@gmail.com>
|
One of the libvirt e2e is failing. |
Does not seem related - tests passed yesterday afaik and the only changed file is a readme file. I also suggest that PR owners will be granted some more privileges such as enabling them to re-run tests :) |
|
The |
TestLibvirtCreatePeerPodAndCheckEnvVariableLogsWithImageAndDeployment Failed for a second time |
|
Since the e2e finally passed, I'm merging this. |
Resolves: #1770
Secure Comms feature to secure communication between the cluster and the Peer Pods. Rely on KBS keys to establish an ssh channel between the cluster WN and the PP. The ssh channel can then be used to tunnel different communications to/from the PP.