Skip to content

go/proxyd: Add support for additional SSL certs#1818

Merged
mslipper merged 1 commit intoethereum-optimism:developfrom
mslipper:feat/additional-certs
Nov 29, 2021
Merged

go/proxyd: Add support for additional SSL certs#1818
mslipper merged 1 commit intoethereum-optimism:developfrom
mslipper:feat/additional-certs

Conversation

@mslipper
Copy link
Contributor

@mslipper mslipper commented Nov 29, 2021

Fixes ENG-1704

@changeset-bot
Copy link

changeset-bot bot commented Nov 29, 2021

🦋 Changeset detected

Latest commit: 6c7f483

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/proxyd Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mslipper mslipper changed the title go/proxyd: ENG-1704 add support for additional SSL certs go/proxyd: Add support for additional SSL certs Nov 29, 2021
@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2021

Codecov Report

Merging #1818 (00f9c86) into develop (24cc244) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1818   +/-   ##
========================================
  Coverage    72.00%   72.00%           
========================================
  Files           70       70           
  Lines         2318     2318           
  Branches       345      345           
========================================
  Hits          1669     1669           
  Misses         649      649           
Flag Coverage Δ
batch-submitter 62.07% <ø> (ø)
contracts 87.96% <ø> (ø)
core-utils 57.50% <ø> (ø)
data-transport-layer 38.23% <ø> (ø)
message-relayer 70.86% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24cc244...00f9c86. Read the comment docs.

@optimisticben
Copy link
Contributor

I was think more along the lines of adding a TLS config which defines the CA certs to use explicitly, like https://gist.github.com/xjdrew/97be3811966c8300b724deabc10e38e2#file-client-go-L15-L35

This would allow custom CA certs to be loaded from file, as well as prepare for client cert auth.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this step happen at build time to make it reproducible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

update-ca-certificates needs to be run whenever the supplied certificates change. Since we support bind-mounting the certificates at runtime, this needs to happen before executing the proxyd binary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updating certs from the internet at runtime is asking for trouble.

Can we default to the container CA certs, but allow specifying a CA file at runtime (end drop the entrypoint script)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

update-ca-certificates doesn't update certs from the internet, it just appends all certs in the ca-certificates directory into one big file in /etc/ssl: https://gitlab-test.alpinelinux.org/alpine/ca-certificates/-/blob/master/update-ca.c.

That said, I think it's also useful to specify a CA file at runtime for things like client certificates, so I've added that functionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, I thought that update-ca-certificates called out to the network and downloaded the latest certs from the alpine registry

@mslipper mslipper force-pushed the feat/additional-certs branch from 00f9c86 to 3881a30 Compare November 29, 2021 21:15
@mslipper
Copy link
Contributor Author

Done, updated to support custom CAs via the following backend config:

# Path to a custom root CA.
ca_file = ""
# Path to a custom client cert file.
client_cert_file = ""
# Path to a custom client key file.
client_key_file = ""

@mslipper mslipper force-pushed the feat/additional-certs branch from 3881a30 to 6c7f483 Compare November 29, 2021 21:50
@mslipper mslipper merged commit 066ab89 into ethereum-optimism:develop Nov 29, 2021
theochap added a commit that referenced this pull request Dec 10, 2025
## Overview

Stops the safe head walkback within sync start at genesis, in case there
aren't enough L2 blocks to cover a full sequence window.

Co-authored-by: theo <80177219+theochap@users.noreply.github.com>
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.

4 participants