Skip to content

Conversation

esosaoh
Copy link
Collaborator

@esosaoh esosaoh commented Aug 18, 2025

Description

Fix #179

This PR implements the v1/is_certificate_trusted host capability as specified in here

Test

To test this pull request, you can run the following commands:

make e2e-tests

@esosaoh esosaoh self-assigned this Aug 18, 2025
@viccuad viccuad moved this to Pending review in Kubewarden Aug 18, 2025
@jvanz jvanz requested a review from a team August 18, 2025 17:54
@esosaoh esosaoh force-pushed the cryptographic branch 2 times, most recently from b3aa638 to 93c28a7 Compare August 19, 2025 12:59
Signed-off-by: Esosa Ohangbon <esosaohangbon68@gmail.com>
Signed-off-by: Esosa Ohangbon <esosaohangbon68@gmail.com>
Signed-off-by: Esosa Ohangbon <esosaohangbon68@gmail.com>
Signed-off-by: Esosa Ohangbon <esosaohangbon68@gmail.com>
Signed-off-by: Esosa Ohangbon <esosaohangbon68@gmail.com>
Signed-off-by: Esosa Ohangbon <esosaohangbon68@gmail.com>
Signed-off-by: Esosa Ohangbon <esosaohangbon68@gmail.com>
Signed-off-by: Esosa Ohangbon <esosaohangbon68@gmail.com>
Signed-off-by: Esosa Ohangbon <esosaohangbon68@gmail.com>
Copy link
Member

@jvanz jvanz left a comment

Choose a reason for hiding this comment

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

@esosaoh check why the tests are failing.

*/
export interface Certificate {
encoding: CertificateEncoding;
data: number[];
Copy link
Member

@viccuad viccuad Aug 26, 2025

Choose a reason for hiding this comment

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

data must always be utf8, we should document it on the type here.

We could provide helper functions, but please skip if it feels too much and not language native:

export namespace CertificateUtils {
  export function fromString(certString: string, encoding: CertificateEncoding): Certificate {
    return {
      encoding,
      data: Array.from(new TextEncoder().encode(certString)), // always UTF-8
    };
  }

  export function toString(cert: Certificate): string {
    return new TextDecoder('utf-8').decode(Uint8Array.from(cert.data));
  }
}

Copy link
Member

Choose a reason for hiding this comment

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

Moreover, I think that number is not a good way to represent a list of u8. I think we should be using Uint8Array instead

* a chain to validate it with
*/
export interface CertificateVerificationRequest {
cert: Certificate;
Copy link
Member

Choose a reason for hiding this comment

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

We should document here that:

cert is the certificate to verify.

cert_chain is a list of certs, ordered by trust usage (intermediates first, root last). If empty or missing, certificate is assumed trusted.

const requestObj: CertificateVerificationRequest = {
cert,
cert_chain: certChain,
not_after: notAfter || '',
Copy link
Member

@viccuad viccuad Aug 26, 2025

Choose a reason for hiding this comment

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

I'm afraid the empty not_after is enough. In the case it is not provided, we need to construct a CertificateVerificationRequest without the not_after key instead, as the spec says "undefined", not "empty".

This is how it is implemented in other SDKs, for example on the Rust one, the field doesn't get serialized if it's a None:
https://github.com/kubewarden/policy-sdk-rust/blob/main/src/host_capabilities/mod.rs#L131

*/
export interface CertificateVerificationRequest {
cert: Certificate;
cert_chain: Certificate[];
Copy link
Member

Choose a reason for hiding this comment

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

Both cert_chain and not_after are optional, hence they may be missing from the JSON that a policy sends to the host.

It seems that the cert_chain may be a None or an empty list, but for the not_after we need to not provide it to skip it.

* CertificateVerificationRequest holds information about a certificate and
* a chain to validate it with
*/
export interface CertificateVerificationRequest {
Copy link
Member

Choose a reason for hiding this comment

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

We don't need to export this, it's a helper internal type.
This also allows us to build the Request as needed, given the optional cert_chain and not_after that can be missing.

@jvanz
Copy link
Member

jvanz commented Aug 27, 2025

This PR is blocked by kubewarden/kwctl#1337

@jvanz jvanz moved this from Pending review to Blocked in Kubewarden Aug 27, 2025
*/
export interface Certificate {
encoding: CertificateEncoding;
data: number[];
Copy link
Member

Choose a reason for hiding this comment

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

Moreover, I think that number is not a good way to represent a list of u8. I think we should be using Uint8Array instead

@jvanz
Copy link
Member

jvanz commented Sep 1, 2025

@esosaoh kwctl with the fix to allowing the record and replay of the crypto host capabilities are release. Can you test it again?

@jvanz jvanz moved this from Blocked to In Progress in Kubewarden Sep 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Feature Request: Implement the v1/is_certificate_trusted host capability
4 participants