Skip to content

feat(op-signer/gen-local-creds): parameterize gen script#265

Merged
edobry merged 6 commits intomainfrom
edobry/op-signer-gen-tls
Apr 2, 2025
Merged

feat(op-signer/gen-local-creds): parameterize gen script#265
edobry merged 6 commits intomainfrom
edobry/op-signer-gen-tls

Conversation

@edobry
Copy link
Copy Markdown
Contributor

@edobry edobry commented Mar 28, 2025

This PR parameterizes and renames the gen-local-tls.sh script to gen-local-creds.sh to enable users to generate a subset of credentials, intended to enable its usage in optimism-package. It adds a CLI parameter taking one of {ca,client_tls,client_signing_key,all}, determining what the script will do. It also enables generating TLS credentials for multiple client hostnames in a single run, which is useful for Kurtosis.

In support of https://github.com/ethereum-optimism/platforms-team/issues/581.

@ddaws
Copy link
Copy Markdown
Contributor

ddaws commented Mar 29, 2025

@edobry have you thought about moving this to the Makefile? I think this is a good candidate because the command generates real targets (files). make build could depend on the keys being generated


process_clients() {
generator="$1"
for hostname in $CLIENT_HOSTNAMES; do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like this will generate a certificate per hostname in the case of process_clients generate_client_tls. Why create a cert per hostname instead of including multiple DNS SAN in the single cert? I think multiple SAN is the real world way a client would be identified by multiple DNS names

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the intention is to create multiple keys to be used by multiple services (see here)

Comment thread op-signer/gen-local-creds.sh Outdated
Comment thread op-signer/gen-local-creds.sh Outdated

generate_ca() {
local force="$1"
[ "$force" = "true" ] || [ ! -f "$CA_CERT" ] || return 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Make does this by default, which is why I was thinking it might make sense to move this to the Makefile

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it would probably be good to move to the Makefile and leverage this feature, however I'm loathe to invest in it as most OP Labs projects have transitioned to using just. could be good to migrate this one as well in a future PR

@edobry edobry force-pushed the edobry/op-signer-gen-tls branch from 5a903a5 to 0e005dc Compare April 2, 2025 15:59
@edobry edobry merged commit da34fb1 into main Apr 2, 2025
6 checks passed
@edobry edobry deleted the edobry/op-signer-gen-tls branch April 2, 2025 21:32
edobry added a commit to ethpandaops/optimism-package that referenced this pull request Apr 3, 2025
This PR provisions an `op-signer` instance per network in order to
support signing batcher/proposer/challenger transactions. It leverages
the local KMS support added in [this
PR](ethereum-optimism/infra#252) in order to
reuse the existing client private keys, which it converts from raw hex
strings to PEM format.

It uses the `op-signer/gen-local-creds.sh` script (updated in [this
PR](ethereum-optimism/infra#265)) to generate
mTLS credentials to authenticate connections between `op-signer` and its
clients.

Resolves https://github.com/ethereum-optimism/platforms-team/issues/581.
raffaele-oplabs pushed a commit that referenced this pull request Aug 3, 2025
* parameterize gen-local-tls further

* add cases to enable constrained generation

* rename to gen-local-creds

* sh compatibility

* don't overwrite TLS_DIR

* rename client_key to client_signing_key
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