Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cdh: use b64url encoding in sealed-secrets JWS #794

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

mkulke
Copy link
Contributor

@mkulke mkulke commented Nov 8, 2024

The payload of the JWS should be encoded/decoded with b64url and no padding.

@mkulke mkulke requested a review from a team as a code owner November 8, 2024 16:43
Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@Xynnn007 Xynnn007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to change the base64 coding alphabet, we need also change the following things to keep it all consistent. Seems that those are STANDARD for now. Btw is there any reason to change the code alphabet? I am not familiar with the convention of base64.

The base64 encoding and examples in the doc: https://github.com/confidential-containers/guest-components/blob/main/confidential-data-hub/docs/SEALED_SECRET.md
The generation code: https://github.com/confidential-containers/guest-components/blob/main/confidential-data-hub/secret/src/secret/mod.rs#L70

@mkulke
Copy link
Contributor Author

mkulke commented Nov 11, 2024

If we want to change the base64 coding alphabet, we need also change the following things to keep it all consistent. Seems that those are STANDARD for now. Btw is there any reason to change the code alphabet? I am not familiar with the convention of base64.

afaik, all of the JW* family use b64-url encoding. some characters in the b64 set are hard to process (e.g. as query params in urls)

image

The base64 encoding and examples in the doc: https://github.com/confidential-containers/guest-components/blob/main/confidential-data-hub/docs/SEALED_SECRET.md The generation code: https://github.com/confidential-containers/guest-components/blob/main/confidential-data-hub/secret/src/secret/mod.rs#L70

thx. will fix the docs. but the generation code should be fixed in this PR, no?

The payload of the JWS should be encoded/decoded with b64url and no padding.

Signed-off-by: Magnus Kulke <[email protected]>
@mkulke mkulke force-pushed the mkulke/fix-jws-encoding branch from 727d1c5 to 79198ba Compare November 11, 2024 08:20
@mkulke mkulke requested a review from Xynnn007 November 11, 2024 08:22
@Xynnn007
Copy link
Member

thx. will fix the docs. but the generation code should be fixed in this PR, no?

Yes, it should be fixed also in the PR imo

Copy link
Member

@Xynnn007 Xynnn007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mkulke
Copy link
Contributor Author

mkulke commented Nov 11, 2024

thx. will fix the docs. but the generation code should be fixed in this PR, no?

Yes, it should be fixed also in the PR imo

ah, sorry, I was being ambiguous. I meant it is already fixed (b64 is aliased to b64_url_nopad). is there any other place that would need to be adjusted?

image

@Xynnn007
Copy link
Member

@mkulke No else found from myside. Sry too as my first glance did not catch the part that you already had fixed.

@mkulke mkulke merged commit 8749486 into confidential-containers:main Nov 11, 2024
5 checks passed
@mkulke mkulke deleted the mkulke/fix-jws-encoding branch November 11, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants