Skip to content

Add support for DENO_TLS_CA_STORE env variable#184

Merged
andreespirela merged 1 commit intosupabase:mainfrom
voltade:main
Oct 3, 2023
Merged

Add support for DENO_TLS_CA_STORE env variable#184
andreespirela merged 1 commit intosupabase:mainfrom
voltade:main

Conversation

@mdluo
Copy link
Contributor

@mdluo mdluo commented Sep 22, 2023

What kind of change does this PR introduce?

Deno 1.13 introduced DENO_TLS_CA_STORE environment variable that can be used to switch which certificate authorities Deno trusts for TLS: https://deno.com/blog/v1.13#use-system-certificate-store-for-tls

This PR bring this env variable support to edge-runtime by adapting the source code from deno: https://github.com/denoland/deno/blob/v1.37.0/cli/args/mod.rs#L467

DENO_TLS_CA_STORE allows 2 values: mozilla or system, and default to mozilla, which is the current default behavior as well. Users need to explicitly set it to system to trust the extra root cert stores.

What is the current behavior?

It is not possible to make network request to services with self-signed certificate, even after adding the root-ca to the system with ca-certificates:

FROM supabase/edge-runtime:latest

RUN apt update && apt install -y ca-certificates
COPY ./certs/root-ca.crt /usr/local/share/ca-certificates/root-ca.crt
RUN update-ca-certificates

And you will get an error that looks like:

error sending request for url (https://localhost:3000): error trying to connect: invalid peer certificate: UnknownIssuer

What is the new behavior?

By setting DENO_TLS_CA_STORE to system and rebuild the docker image, the HTTPS requests in the worker function works as intended.

# This is a test image built with the changes of this PR, for testing purpose only. Please wait until the PR get merge and use the latest supabase/edge-runtime
FROM public.ecr.aws/f1a3r3o9/supabase/edge-runtime:latest

RUN apt update && apt install -y ca-certificates
COPY ./certs/root-ca.crt /usr/local/share/ca-certificates/root-ca.crt
RUN update-ca-certificates

ENV DENO_TLS_CA_STORE=system

Additional context

@laktek
Copy link
Contributor

laktek commented Sep 26, 2023

@mdluo Thanks for the contribution. We'll review this in the next 2 days (sorry about the delay)

Copy link
Contributor

@andreespirela andreespirela left a comment

Choose a reason for hiding this comment

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

Can this have a test of some sort?

Copy link
Contributor

@andreespirela andreespirela left a comment

Choose a reason for hiding this comment

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

Lgtm

@andreespirela andreespirela merged commit 1652bcc into supabase:main Oct 3, 2023
@github-actions
Copy link

🎉 This PR is included in version 1.21.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants