SecureComms: Add Support for activating using InitData #2072

davidhadas · 2024-09-30T10:10:41Z

See:

Add apj.json to InitData
apf.josn includes an sc key used to activate secure comms (if not already activated using an agent-protocol-forwarder.service flag)

davidhadas · 2024-10-08T04:25:30Z

cc: @bpradipt

Install KBS and test SecureComms with KBS Based on confidential-containers#2072 which should be merged first Signed-off-by: David Hadas <david.hadas@gmail.com>

stevenhorsman

Some initial comments. It would be good to fix the typos in the commit message too.

stevenhorsman · 2024-10-08T13:18:03Z

src/cloud-api-adaptor/cmd/agent-protocol-forwarder/main.go

@@ -52,6 +54,8 @@ func load(path string, obj interface{}) error {
 		return fmt.Errorf("failed to decode a Agent Protocol Forwarder config file file: %s: %w", path, err)
 	}

+	logger.Printf("succesful loading config from %s\n", path)


Is this supposed to say "successfully loaded config..."?

stevenhorsman · 2024-10-08T13:20:49Z

src/cloud-api-adaptor/cmd/cloud-api-adaptor/main.go

@@ -114,7 +114,7 @@ func (cfg *daemonConfig) Setup() (cmd.Starter, error) {
 		flags.BoolVar(&secureComms, "secure-comms", false, "Use SSH to secure communication between cluster and peer pods")
 		flags.StringVar(&secureCommsInbounds, "secure-comms-inbounds", "", "Inbound tags for secure communication tunnels")
 		flags.StringVar(&secureCommsOutbounds, "secure-comms-outbounds", "", "Outbound tags for secure communication tunnels")
-		flags.StringVar(&secureCommsKbsAddr, "secure-comms-kbs", "kbs-service.kbs-operator-system:8080", "Address of a KBS Service for Secure-Comms")
+		flags.StringVar(&secureCommsKbsAddr, "secure-comms-kbs", "kbs-service.trustee-operator-system:8080", "Address of a KBS Service for Secure-Comms")


The trustee operator namespace changes should be in a separate commit with an explanation that it's triggered by the change in the trustee-operator project

I've just noticed a bunch of these change are in #2073. Is this PR supposed to depend on that one?

Yes - #2073 should be merged first

stevenhorsman · 2024-10-08T13:27:36Z

src/cloud-api-adaptor/docs/SecureComms.md

+```sh
+kubectl get secrets -n trustee-operator-system
+NAME                  TYPE     DATA   AGE
+kbs-auth-public-key   Opaque   1      28h
+kbs-client            Opaque   1      28h
+```


What's the reason for this command being added?

I've just seen this code is under #2065 now. What's going on with these PR and their duplication of code?

This is a slight improvement to the SecureComms doc which shows the correct result after following the instructions trustee operator and following the recommendation: "Make sure to uncomment the secret generation as recommended for both public and private key (kbs-auth-public-key and kbs-client secrets). "

We can add it to #2073 if it will make things clearer

I prefer to leave this extra documentation detail as is, although it also appears in #2065 - unless someone finds this very disturbing.

stevenhorsman · 2024-10-08T13:32:55Z

src/cloud-api-adaptor/docs/SecureComms.md

+kubectl -n confidential-containers-system  get cm peer-pods-cm  -o yaml | sed "s/SECURE_COMMS: \"false\"/SECURE_COMMS: \"true\"/"|kubectl apply -f -
+```
+
+Set InitData to point KBC services to IP address 127.0.0.1 


Should this have a heading like Build a podvm that enforces Secure-Comms (Optional) section does as it's an alternative to it?

Some expansion of the explanation of what this is doing and why would be nice.

I will be adapting the documentation based on this comment. Please reconsider the next version.

stevenhorsman · 2024-10-08T13:33:35Z

src/cloud-api-adaptor/docs/SecureComms.md

+For example:
+```sh
+ExecStart=/usr/local/bin/agent-protocol-forwarder -kata-agent-namespace /run/netns/podns -secure-comms -secure-comms-inbounds KUBERNETES_PHASE:mytunnel:6666 -kata-agent-socket /run/kata-containers/agent.sock $TLS_OPTIONS $OPTIONS
+```

 Once you changed `podvm/files/etc/systemd/system/agent-protocol-forwarder.service`, you will need to [rebuild the podvm](./../podvm/README.md).


 ### Activate CAA Secure-Comms feature


I'm not sure if this heading is helpful as it's needed in both the podvm and initdata approach and it's slightly different and doesn't have a heading there?

stevenhorsman · 2024-10-08T13:35:53Z

src/cloud-api-adaptor/docs/SecureComms.md

+
+## Adding named tunnels to the SSH channel


What's the reason for the move of the newline?

changed to ### Activate the Secure-Comms feature
This will be our main section to show the config changes needed.

The other option of changing the podvm will move to the end. We will use it as an extra step for cases where one doe snot trust the InitData (e.g. InitData is not measured).

stevenhorsman · 2024-10-08T13:44:22Z

src/cloud-api-adaptor/docs/SecureComms.md

+'''
+"apf.json" = '''
+{
+    sc: true


Can we expand sc to give a better explanation please e.g. enable_secure_comms

changed to secure-comms to align with other config parameters enabling Secure-Comms

stevenhorsman · 2024-10-08T13:45:15Z

src/cloud-api-adaptor/docs/SecureComms.md

+"apf.json" = '''
+{
+    sc: true
+}


You aren't closing the apf.json file here?

stevenhorsman · 2024-10-08T13:46:12Z

src/cloud-api-adaptor/cmd/cloud-api-adaptor/main.go

@@ -114,7 +114,7 @@ func (cfg *daemonConfig) Setup() (cmd.Starter, error) {
 		flags.BoolVar(&secureComms, "secure-comms", false, "Use SSH to secure communication between cluster and peer pods")
 		flags.StringVar(&secureCommsInbounds, "secure-comms-inbounds", "", "Inbound tags for secure communication tunnels")
 		flags.StringVar(&secureCommsOutbounds, "secure-comms-outbounds", "", "Outbound tags for secure communication tunnels")
-		flags.StringVar(&secureCommsKbsAddr, "secure-comms-kbs", "kbs-service.kbs-operator-system:8080", "Address of a KBS Service for Secure-Comms")
+		flags.StringVar(&secureCommsKbsAddr, "secure-comms-kbs", "kbs-service.trustee-operator-system:8080", "Address of a KBS Service for Secure-Comms")


I've just noticed a bunch of these change are in #2073. Is this PR supposed to depend on that one?

stevenhorsman · 2024-10-08T14:08:56Z

src/cloud-api-adaptor/docs/SecureComms.md

+```sh
+kubectl get secrets -n trustee-operator-system
+NAME                  TYPE     DATA   AGE
+kbs-auth-public-key   Opaque   1      28h
+kbs-client            Opaque   1      28h
+```


I've just seen this code is under #2065 now. What's going on with these PR and their duplication of code?

Add apj.json to InitData apf.josn includes a secure-comms key used to activate secure comms (if not already activated using an agent-protocol-forwarder.service flag) Signed-off-by: David Hadas <david.hadas@gmail.com>

mkulke

I understand that kata is proceeding with an init-data approach that is passed from the runtime to the agent via a SetInitData() RPC call for the upcoming CoCo release.

This looks like a CAA-specific extension to InitData. If the project adopts kata's init-data approach, would this still work?

davidhadas requested a review from a team as a code owner September 30, 2024 10:10

davidhadas force-pushed the secComms_initData branch 2 times, most recently from 8630009 to 206ce0f Compare October 7, 2024 12:37

davidhadas mentioned this pull request Oct 8, 2024

SecureComms: E2e test SecureComms with KBS #2093

Open

stevenhorsman reviewed Oct 8, 2024

View reviewed changes

SecureComms: Activate PP SC using InitData

1a5d0a9

Add apj.json to InitData apf.josn includes a secure-comms key used to activate secure comms (if not already activated using an agent-protocol-forwarder.service flag) Signed-off-by: David Hadas <david.hadas@gmail.com>

davidhadas force-pushed the secComms_initData branch from 206ce0f to 1a5d0a9 Compare October 9, 2024 08:50

mkulke reviewed Oct 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SecureComms: Add Support for activating using InitData #2072

SecureComms: Add Support for activating using InitData #2072

davidhadas commented Sep 30, 2024 •

edited

Loading

davidhadas commented Oct 8, 2024

stevenhorsman left a comment

stevenhorsman Oct 8, 2024

stevenhorsman Oct 8, 2024

stevenhorsman Oct 8, 2024

davidhadas Oct 8, 2024

stevenhorsman Oct 8, 2024

stevenhorsman Oct 8, 2024

davidhadas Oct 8, 2024

davidhadas Oct 9, 2024

stevenhorsman Oct 8, 2024

stevenhorsman Oct 8, 2024

davidhadas Oct 9, 2024

stevenhorsman Oct 8, 2024

stevenhorsman Oct 8, 2024

davidhadas Oct 9, 2024

stevenhorsman Oct 8, 2024

davidhadas Oct 9, 2024

stevenhorsman Oct 8, 2024

davidhadas Oct 9, 2024

stevenhorsman Oct 8, 2024

stevenhorsman Oct 8, 2024

mkulke left a comment

SecureComms: Add Support for activating using InitData #2072

Are you sure you want to change the base?

SecureComms: Add Support for activating using InitData #2072

Conversation

davidhadas commented Sep 30, 2024 • edited Loading

davidhadas commented Oct 8, 2024

stevenhorsman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkulke left a comment

Choose a reason for hiding this comment

davidhadas commented Sep 30, 2024 •

edited

Loading