agent: alternative implementation for sealed_secret as volume #10674

bpradipt · 2024-12-18T10:24:25Z

The earlier implementation relied on using a specific mount-path prefix - /sealed to determine that the referenced secret is a sealed secret. However that was restrictive for certain use cases as it forced the user to always use a specific mountpath naming convention.

This commit introduces an alternative implementation to relax the restriction. A sealed secret can be mounted in any mount-path. However it comes with a potential performance penality. The implementation loops through all volume mounts and reads the file to determine if it's a sealed secret or not.

Fixes: #10398

bpradipt · 2024-12-18T10:27:21Z

@stevenhorsman @fitzthum @Xynnn007 @ChengyuZhu6 @sprt I made an attempt to remove the mount path prefix restriction for sealed_secret as volume. Raising a draft PR to get your feedback on the approach and if is a better alternative to try out.

Apokleos

Thx @bpradipt I just have an idea that about introducing a method to handle this checking logic. but It almost looks good to me.

src/agent/src/rpc.rs

ChengyuZhu6

Thanks @bpradipt ! I think this approach is likely to improve usability and looks good to me. Just two minor comments:

The unrelated file to be unsealed may lead to confusion in logging.
We may need to update the test case of sealedsecret in ci at the same time.

src/agent/src/rpc.rs

bpradipt · 2025-01-08T14:48:37Z

2. We may need to update the test case of sealedsecret in ci at the same time.

The example test pods uses the following mountpath and I don't think we need to change this. Let me know if you think otherwise and I'll change it to some other path..

        - name: sealed-secret-volume
           mountPath: "/sealed/secret-value"
        - name: not-sealed-secret-volume
           mountPath: "/sealed/not-sealed-secret-value"

fitzthum

I think this approach is fine. It's a little weird to check each file to see if it is sealed, but I guess it only scales with the total number of mounts.

src/agent/src/rpc.rs

fitzthum

LGTM

bpradipt · 2025-01-11T10:49:40Z

The sealed secret CI tests for both env and file are failing.
The error points to issues with base64 decoding of the secret

Caused by: parse SealedSecret failed: base64 decode Secret body

This error comes from unseal_secret function in confidential-data-hub/secret/src/lib.rs to validate the input.

I checked the sealed secret input in the CI and it's proper.

sealed.fakejwsheader.ewogICAgInZlcnNpb24iOiAiMC4xLjAiLAogICAgInR5cGUiOiAidmF1bHQiLAogICAgIm5hbWUiOiAia2JzOi8vL2RlZmF1bHQvc2VhbGVkLXNlY3JldC90ZXN0IiwKICAgICJwcm92aWRlciI6ICJrYnMiLAogICAgInByb3ZpZGVyX3NldHRpbmdzIjoge30sCiAgICAiYW5ub3RhdGlvbnMiOiB7fQp9Cg==.fakesignature

Looking for ideas where to look at next to root cause this issue..

@stevenhorsman @fitzthum @ChengyuZhu6 or anyone ..

The earlier implementation relied on using a specific mount-path prefix - `/sealed` to determine that the referenced secret is a sealed secret. However that was restrictive for certain use cases as it forced the user to always use a specific mountpath naming convention. This commit introduces an alternative implementation to relax the restriction. A sealed secret can be mounted in any mount-path. However it comes with a potential performance penality. The implementation loops through all volume mounts and reads the file to determine if it's a sealed secret or not. Fixes: kata-containers#10398 Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>

stevenhorsman · 2025-01-13T10:26:57Z

Sorry for the stupid question here, but just thinking about the env test that is failing first:

In the CI we are seeing:

vmconsole="{\"msg\":\"Calling unseal_file for - source: \\\"/run/kata-containers/shared/containers/6c40bfce284a4d90d56f17edbac493372881accc5d2647ee491a1b4bf3c0daff-c1c16822d73eeab8-secret-value\\\" destination: \\\"/sealed/secret-value\\\"\",\"level\":\"DEBG\",\"ts\":\"2025-01-13T06:21:46.818201552Z\",\"subsystem\":\"rpc\",\"source\":\"agent\",\"name\":\"kata-agent\",\"version\":\"0.1.0\",\"pid\":\"143\"}"
 vmconsole="{\"msg\":\"skipping sealed source entry DirEntry(\\\"/run/kata-containers/shared/containers/6c40bfce284a4d90d56f17edbac493372881accc5d2647ee491a1b4bf3c0daff-c1c16822d73eeab8-secret-value/..2025_01_13_06_21_43.1841179162\\\") because its file type is FileType(FileType { mode: 16384 })\",\"level\":\"DEBG\",\"ts\":\"2025-01-13T06:21:46.818264576Z\",\"source\":\"agent\",\"version\":\"0.1.0\",\"pid\":\"143\",\"name\":\"kata-agent\",\"subsystem\":\"cdh\"}"
vmconsole="{\"msg\":\"sealed source entry target path: \\\"/run/kata-containers/shared/containers/6c40bfce284a4d90d56f17edbac493372881accc5d2647ee491a1b4bf3c0daff-c1c16822d73eeab8-secret-value/..2025_01_13_06_21_43.1841179162\\\"\",\"level\":\"INFO\",\"ts\":\"2025-01-13T06:21:46.818320456Z\",\"name\":\"kata-agent\",\"source\":\"agent\",\"pid\":\"143\",\"subsystem\":\"cdh\",\"version\":\"0.1.0\"}"
vmconsole="{\"msg\":\"sealed source is not a file: \\\"/run/kata-containers/shared/containers/6c40bfce284a4d90d56f17edbac493372881accc5d2647ee491a1b4bf3c0daff-c1c16822d73eeab8-secret-value/..2025_01_13_06_21_43.1841179162\\\"\",\"level\":\"DEBG\",\"ts\":\"2025-01-13T06:21:46.818629772Z\",\"source\":\"agent\",\"pid\":\"143\",\"name\":\"kata-agent\",\"subsystem\":\"cdh\",\"version\":\"0.1.0\"}"
vmconsole="{\"msg\":\"sealed source entry target path: \\\"/run/kata-containers/shared/containers/6c40bfce284a4d90d56f17edbac493372881accc5d2647ee491a1b4bf3c0daff-c1c16822d73eeab8-secret-value/..2025_01_13_06_21_43.1841179162/secret\\\"\",\"level\":\"INFO\",\"ts\":\"2025-01-13T06:21:46.818658179Z\",\"source\":\"agent\",\"subsystem\":\"cdh\",\"name\":\"kata-agent\",\"version\":\"0.1.0\",\"pid\":\"143\"}"
vmconsole="\x1b[90m[\x1b[0m2025-01-13T06:21:46Z \x1b[32mINFO \x1b[0m confidential_data_hub::hub\x1b[90m]\x1b[0m unseal secret called"
vmconsole="\x1b[90m[\x1b[0m2025-01-13T06:21:46Z \x1b[1m\x1b[31mERROR\x1b[0m ttrpc_cdh::ttrpc_server\x1b[90m]\x1b[0m [ttRPC CDH] UnsealSecret :"
vmconsole="    unseal secret failed"
vmconsole="    Caused by: parse SealedSecret failed: base64 decode Secret body"

But your new debug code is in the unseal_file path, not the unsealed_env, so I'm not sure if we'd expect this to work?

For the sealed file test, we seem to get the same response:

The sealed_secret looks like sealed.fakejwsheader.ewogICAgInZlcnNpb24iOiAiMC4xLjAiLAogICAgInR5cGUiOiAidmF1bHQiLAogICAgIm5hbWUiOiAia2JzOi8vL2RlZmF1bHQvc2VhbGVkLXNlY3JldC90ZXN0IiwKICAgICJwcm92aWRlciI6ICJrYnMiLAogICAgInByb3ZpZGVyX3NldHRpbmdzIjoge30sCiAgICAiYW5ub3RhdGlvbnMiOiB7fQp9Cg==.fakesignature
The logs show:

vmconsole="{\"msg\":\"Calling unseal_file for - source: \\\"/run/kata-containers/shared/containers/6c40bfce284a4d90d56f17edbac493372881accc5d2647ee491a1b4bf3c0daff-c1c16822d73eeab8-secret-value\\\" destination: \\\"/sealed/secret-value\\\"\",\"level\":\"DEBG\",\"ts\":\"2025-01-13T06:21:46.818201552Z\",\"subsystem\":\"rpc\",\"source\":\"agent\",\"name\":\"kata-agent\",\"version\":\"0.1.0\",\"pid\":\"143\"}"
vmconsole="{\"msg\":\"skipping sealed source entry DirEntry(\\\"/run/kata-containers/shared/containers/6c40bfce284a4d90d56f17edbac493372881accc5d2647ee491a1b4bf3c0daff-c1c16822d73eeab8-secret-value/..2025_01_13_06_21_43.1841179162\\\") because its file type is FileType(FileType { mode: 16384 })\",\"level\":\"DEBG\",\"ts\":\"2025-01-13T06:21:46.818264576Z\",\"source\":\"agent\",\"version\":\"0.1.0\",\"pid\":\"143\",\"name\":\"kata-agent\",\"subsystem\":\"cdh\"}"
vmconsole="{\"msg\":\"sealed source entry target path: \\\"/run/kata-containers/shared/containers/6c40bfce284a4d90d56f17edbac493372881accc5d2647ee491a1b4bf3c0daff-c1c16822d73eeab8-secret-value/..2025_01_13_06_21_43.1841179162\\\"\",\"level\":\"INFO\",\"ts\":\"2025-01-13T06:21:46.818320456Z\",\"name\":\"kata-agent\",\"source\":\"agent\",\"pid\":\"143\",\"subsystem\":\"cdh\",\"version\":\"0.1.0\"}"
vmconsole="{\"msg\":\"sealed source is not a file: \\\"/run/kata-containers/shared/containers/6c40bfce284a4d90d56f17edbac493372881accc5d2647ee491a1b4bf3c0daff-c1c16822d73eeab8-secret-value/..2025_01_13_06_21_43.1841179162\\\"\",\"level\":\"DEBG\",\"ts\":\"2025-01-13T06:21:46.818629772Z\",\"source\":\"agent\",\"pid\":\"143\",\"name\":\"kata-agent\",\"subsystem\":\"cdh\",\"version\":\"0.1.0\"}"
vmconsole="{\"msg\":\"sealed source entry target path: \\\"/run/kata-containers/shared/containers/6c40bfce284a4d90d56f17edbac493372881accc5d2647ee491a1b4bf3c0daff-c1c16822d73eeab8-secret-value/..2025_01_13_06_21_43.1841179162/secret\\\"\",\"level\":\"INFO\",\"ts\":\"2025-01-13T06:21:46.818658179Z\",\"source\":\"agent\",\"subsystem\":\"cdh\",\"name\":\"kata-agent\",\"version\":\"0.1.0\",\"pid\":\"143\"}"
vmconsole="\x1b[90m[\x1b[0m2025-01-13T06:21:46Z \x1b[32mINFO \x1b[0m confidential_data_hub::hub\x1b[90m]\x1b[0m unseal secret called"
vmconsole="\x1b[90m[\x1b[0m2025-01-13T06:21:46Z \x1b[1m\x1b[31mERROR\x1b[0m ttrpc_cdh::ttrpc_server\x1b[90m]\x1b[0m [ttRPC CDH] UnsealSecret :"
vmconsole="    unseal secret failed"
vmconsole="    Caused by: parse SealedSecret failed: base64 decode Secret body"

From the limited debug info we have here it looks like it's failing in the CDH:

        let sections: Vec<_> = secret.split('.').collect();

        if sections.len() != 4 {
            return Err(SecretError::ParseFailed("malformed input sealed secret"));
        }

        let secret_json = STANDARD
            .decode(sections[2])
            .map_err(|_| SecretError::ParseFailed("base64 decode Secret body"))?;

but if it's using the secret we'd expect, then the 3rd section (ewogICAgInZlcnNpb24iOiAiMC4xLjAiLAogICAgInR5cGUiOiAidmF1bHQiLAogICAgIm5hbWUiOiAia2JzOi8vL2RlZmF1bHQvc2VhbGVkLXNlY3JldC90ZXN0IiwKICAgICJwcm92aWRlciI6ICJrYnMiLAogICAgInByb3ZpZGVyX3NldHRpbmdzIjoge30sCiAgICAiYW5ub3RhdGlvbnMiOiB7fQp9Cg==.), does decode to

{
    "version": "0.1.0",
    "type": "vault",
    "name": "kbs:///default/sealed-secret/test",
    "provider": "kbs",
    "provider_settings": {},
    "annotations": {}
}

So I guess my conclusion is that we must be acting on a different secret then? I'm not sure how to get further with debug though - debugging in the guest-components has always been difficult/impossible :(

bpradipt · 2025-01-13T11:09:10Z

@stevenhorsman from the logs - https://github.com/kata-containers/kata-containers/actions/runs/12726288825/job/35508106381?pr=10674 both the unsealed_env and unsealed_file tests are failing

not ok 2 Unseal Env Secrets with CDH
not ok 3 Unseal File Secrets with CDH

They both are failing.

From the logs at least it looks like the pod is using the correct secrets. But I can't be sure :-(

stevenhorsman · 2025-01-13T11:15:07Z

@stevenhorsman from the logs - https://github.com/kata-containers/kata-containers/actions/runs/12726288825/job/35508106381?pr=10674 both the unsealed_env and unsealed_file tests are failing

not ok 2 Unseal Env Secrets with CDH not ok 3 Unseal File Secrets with CDH

They both are failing.

From the logs at least it looks like the pod is using the correct secrets. But I can't be sure :-(

Yeah my point was - should the unseal_file code be running on the env test?

ChengyuZhu6 · 2025-01-13T11:22:36Z

@bpradipt I think this issue might stem from the patch applied from CDH, as seen in this commit: confidential-containers/guest-components@8749486?

ChengyuZhu6 · 2025-01-13T11:24:26Z

I think we can resolve the issue by updating the encoded string of sealed secret in the ci.

bpradipt · 2025-01-13T11:27:19Z

@bpradipt I think this issue might stem from the patch applied from CDH, as seen in this commit: confidential-containers/guest-components@8749486?

Great find @ChengyuZhu6 .. Let me try updating the encoded string values in ci

stevenhorsman · 2025-01-13T12:22:16Z

Thanks for the help @ChengyuZhu6 - I'm still not clear why Pradipta's code changes would be the first thing to hit it? Any ideas why the nightly isn't failing as soon as we bumped the guest-components to pick up that patch?

bpradipt · 2025-01-13T12:28:40Z

Thanks for the help @ChengyuZhu6 - I'm still not clear why Pradipta's code changes would be the first thing to hit it? Any ideas why the nightly isn't failing as soon as we bumped the guest-components to pick up that patch?

any likelihood of cached artefacts being used for the test cluster?

The existing encoding was base64 and it fails due to confidential-containers/guest-components@8749486 Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>

ChengyuZhu6

LGTM thanks @bpradipt

stevenhorsman

I would still like to understand why we weren't getting failures with the guest-components bump PR (or nightly) if the encoding was the problem, but I don't think that should block this PR now the tests are passing

katacontainersbot added the size/small Small and simple task label Dec 18, 2024

bpradipt marked this pull request as ready for review December 18, 2024 16:18

Apokleos reviewed Dec 19, 2024

View reviewed changes

src/agent/src/rpc.rs Outdated Show resolved Hide resolved

bpradipt self-assigned this Dec 19, 2024

bpradipt force-pushed the fix-10398 branch from b1eefd3 to 74ee473 Compare December 22, 2024 06:16

katacontainersbot added size/large Task of significant size and removed size/small Small and simple task labels Dec 22, 2024

bpradipt requested review from fitzthum, stevenhorsman and ChengyuZhu6 December 22, 2024 06:17

bpradipt added the ok-to-test label Dec 22, 2024

bpradipt requested a review from Apokleos December 23, 2024 12:25

ChengyuZhu6 reviewed Jan 8, 2025

View reviewed changes

bpradipt commented Jan 8, 2025

View reviewed changes

src/agent/src/rpc.rs Outdated Show resolved Hide resolved

fitzthum reviewed Jan 8, 2025

View reviewed changes

src/agent/src/rpc.rs Outdated Show resolved Hide resolved

src/agent/src/rpc.rs Outdated Show resolved Hide resolved

src/agent/src/rpc.rs Outdated Show resolved Hide resolved

bpradipt force-pushed the fix-10398 branch from 74ee473 to 17f6a66 Compare January 9, 2025 04:02

bpradipt requested review from fitzthum and ChengyuZhu6 January 9, 2025 04:05

fitzthum approved these changes Jan 9, 2025

View reviewed changes

bpradipt force-pushed the fix-10398 branch from 17f6a66 to 5218345 Compare January 11, 2025 17:38

tests: Update sealed secret CI value to base64url

36580bb

The existing encoding was base64 and it fails due to confidential-containers/guest-components@8749486 Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>

bpradipt force-pushed the fix-10398 branch from 8e02a35 to 36580bb Compare January 13, 2025 14:37

ChengyuZhu6 approved these changes Jan 14, 2025

View reviewed changes

stevenhorsman approved these changes Jan 14, 2025

View reviewed changes

ChengyuZhu6 merged commit 7d34ca4 into kata-containers:main Jan 14, 2025
291 of 299 checks passed

agent: alternative implementation for sealed_secret as volume #10674

agent: alternative implementation for sealed_secret as volume #10674

Uh oh!

Conversation

bpradipt commented Dec 18, 2024

Uh oh!

bpradipt commented Dec 18, 2024

Uh oh!

Apokleos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChengyuZhu6 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bpradipt commented Jan 8, 2025

Uh oh!

fitzthum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fitzthum left a comment

Choose a reason for hiding this comment

Uh oh!

bpradipt commented Jan 11, 2025

Uh oh!

stevenhorsman commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bpradipt commented Jan 13, 2025

Uh oh!

stevenhorsman commented Jan 13, 2025

Uh oh!

ChengyuZhu6 commented Jan 13, 2025

Uh oh!

ChengyuZhu6 commented Jan 13, 2025

Uh oh!

bpradipt commented Jan 13, 2025

Uh oh!

stevenhorsman commented Jan 13, 2025

Uh oh!

bpradipt commented Jan 13, 2025

Uh oh!

ChengyuZhu6 left a comment

Choose a reason for hiding this comment

Uh oh!

stevenhorsman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stevenhorsman commented Jan 13, 2025 •

edited

Loading