Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/ledgerctl/auth/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ func syncProfile(cmd *cobra.Command, server string) error {

insecure, _ := cmd.Flags().GetBool("insecure")
tlsCaCert, _ := cmd.Flags().GetString("tls-ca-cert")
tlsServerName, _ := cmd.Flags().GetString("tls-server-name")
signingKey, _ := cmd.Flags().GetString("signing-key")
responseVerifyKey, _ := cmd.Flags().GetString("response-verify-key")

Expand All @@ -190,6 +191,7 @@ func syncProfile(cmd *cobra.Command, server string) error {
Server: server,
Insecure: insecure,
TLSCaCert: tlsCaCert,
TLSServerName: tlsServerName,
Comment thread
NumaryBot marked this conversation as resolved.
SigningKey: signingKey,
SigningKeyID: signingKeyID,
ResponseVerifyKey: responseVerifyKey,
Expand Down
32 changes: 26 additions & 6 deletions cmd/ledgerctl/cmdutil/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,47 @@ const (

// GetClientTransportCredentials returns the transport credentials based on CLI flags.
// If --insecure is set, returns insecure credentials.
// Otherwise, returns TLS credentials, optionally using a custom CA from --tls-ca-cert.
// Otherwise, returns TLS credentials, optionally using a custom CA from --tls-ca-cert
// and a custom verification hostname from --tls-server-name.
//
// Setting --insecure together with --tls-ca-cert is rejected: those flags
// encode conflicting intent (no TLS vs. verify with this CA), and silently
// preferring one of them — as the older code did — masks env-var leakage like
// a stray LEDGERCTL_INSECURE=true in a container image (the original cause of
// the "error reading server preface: EOF" production incident).
// Setting --insecure together with --tls-ca-cert or --tls-server-name is
// rejected: those flags encode conflicting intent (no TLS vs. verify with this
// CA / against this hostname), and silently preferring one of them — as the
// older code did — masks env-var leakage like a stray LEDGERCTL_INSECURE=true
// in a container image (the original cause of the "error reading server
// preface: EOF" production incident).
//
// --tls-server-name overrides the hostname the certificate is verified against
// (SNI + SAN match), decoupling it from the dial address in --server. This is
// what lets a client dial by IP (e.g. 127.0.0.1:8888 from inside a pod, or a
// LoadBalancer VIP) while still validating an operator-issued certificate whose
// SANs only cover the in-cluster DNS names (e.g.
// <pod>.<cluster>-headless.<ns>.svc.cluster.local), never localhost/127.0.0.1.
func GetClientTransportCredentials(cmd *cobra.Command) (credentials.TransportCredentials, error) {
insecureMode, _ := cmd.Flags().GetBool("insecure")
caCertPath, _ := cmd.Flags().GetString("tls-ca-cert")
serverName, _ := cmd.Flags().GetString("tls-server-name")

if insecureMode && caCertPath != "" {
return nil, errors.New("--insecure and --tls-ca-cert are mutually exclusive (--insecure may also come from the LEDGERCTL_INSECURE env var; unset it to use TLS)")
}

if insecureMode && serverName != "" {
return nil, errors.New("--insecure and --tls-server-name are mutually exclusive (--insecure may also come from the LEDGERCTL_INSECURE env var; unset it to use TLS)")
}

if insecureMode {
return insecure.NewCredentials(), nil
}

tlsConfig := &tls.Config{MinVersion: tls.VersionTLS12}

// Override the name verified against the server certificate's SANs, so the
// dial address (--server) and the verification identity can differ.
if serverName != "" {
tlsConfig.ServerName = serverName
}

if caCertPath != "" {
caPEM, err := os.ReadFile(caCertPath)
if err != nil {
Expand Down
31 changes: 31 additions & 0 deletions cmd/ledgerctl/cmdutil/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func newTLSFlagCommand() *cobra.Command {
cmd := &cobra.Command{Use: "test"}
cmd.Flags().Bool("insecure", false, "")
cmd.Flags().String("tls-ca-cert", "", "")
cmd.Flags().String("tls-server-name", "", "")

return cmd
}
Expand Down Expand Up @@ -57,6 +58,36 @@ func TestGetClientTransportCredentials_InsecureOnly(t *testing.T) {
require.NotNil(t, creds)
}

// --tls-server-name only makes sense when TLS is active; pairing it with
// --insecure is rejected for the same reason as --tls-ca-cert — it catches a
// stray LEDGERCTL_INSECURE leaking in and silently disabling verification.
func TestGetClientTransportCredentials_InsecureWithServerNameIsRejected(t *testing.T) {
t.Parallel()

cmd := newTLSFlagCommand()
require.NoError(t, cmd.Flags().Set("insecure", "true"))
require.NoError(t, cmd.Flags().Set("tls-server-name", "ledger.svc.cluster.local"))

_, err := GetClientTransportCredentials(cmd)
require.Error(t, err)
require.Contains(t, err.Error(), "--insecure and --tls-server-name are mutually exclusive")
}

// A --tls-server-name without --insecure is the supported case: dial by IP,
// verify against the cert SANs by name. This is what unblocks ledgerctl from
// inside a TLS cluster pod, where the operator-issued cert covers the in-cluster
// DNS names but never 127.0.0.1/localhost.
func TestGetClientTransportCredentials_TLSWithServerName(t *testing.T) {
t.Parallel()

cmd := newTLSFlagCommand()
require.NoError(t, cmd.Flags().Set("tls-server-name", "ledger.svc.cluster.local"))

creds, err := GetClientTransportCredentials(cmd)
require.NoError(t, err)
require.NotNil(t, creds)
}

func TestGetClientTransportCredentials_TLSWithCustomCA(t *testing.T) {
t.Parallel()

Expand Down
3 changes: 3 additions & 0 deletions cmd/ledgerctl/cmdutil/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type Profile struct {
Server string `json:"server"`
Insecure bool `json:"insecure,omitempty"`
TLSCaCert string `json:"tlsCaCert,omitempty"`
TLSServerName string `json:"tlsServerName,omitempty"`
SigningKey string `json:"signingKey,omitempty"`
SigningKeyID string `json:"signingKeyId,omitempty"`
ResponseVerifyKey string `json:"responseVerifyKey,omitempty"`
Expand Down Expand Up @@ -160,6 +161,8 @@ func ProfileFlagValue(p *Profile, flagName string) string {
return ""
case "tls-ca-cert":
return p.TLSCaCert
case "tls-server-name":
return p.TLSServerName
case "signing-key":
return p.SigningKey
case "signing-key-id":
Expand Down
1 change: 1 addition & 0 deletions cmd/ledgerctl/cmdutil/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func ResolveConnectionFlags(cmd *cobra.Command) error {
resolveFlag(cmd, "server", "LEDGERCTL_SERVER", ProfileFlagValue(p, "server"))
resolveFlag(cmd, "insecure", "LEDGERCTL_INSECURE", ProfileFlagValue(p, "insecure"))
resolveFlag(cmd, "tls-ca-cert", "LEDGERCTL_TLS_CA_CERT", ProfileFlagValue(p, "tls-ca-cert"))
resolveFlag(cmd, "tls-server-name", "LEDGERCTL_TLS_SERVER_NAME", ProfileFlagValue(p, "tls-server-name"))
resolveFlag(cmd, "consistency", "LEDGERCTL_CONSISTENCY", "")
resolveFlag(cmd, "auth-token", "LEDGERCTL_AUTH_TOKEN", "")
resolveFlag(cmd, "signing-key", "LEDGERCTL_SIGNING_KEY", ProfileFlagValue(p, "signing-key"))
Expand Down
25 changes: 25 additions & 0 deletions cmd/ledgerctl/cmdutil/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func newConnCmd() *cobra.Command {
cmd.Flags().String("server", "localhost:8888", "")
cmd.Flags().Bool("insecure", false, "")
cmd.Flags().String("tls-ca-cert", "", "")
cmd.Flags().String("tls-server-name", "", "")
cmd.Flags().String("consistency", "", "")
cmd.Flags().String("auth-token", "", "")
cmd.Flags().String("signing-key", "", "")
Expand Down Expand Up @@ -63,6 +64,30 @@ func TestResolveConnectionFlagsAppliesActiveProfile(t *testing.T) {
require.True(t, insecure, "insecure must come from the active profile")
}

// TestResolveConnectionFlagsAppliesServerName asserts the profile's
// tlsServerName reaches the --tls-server-name flag through the shared resolver,
// so a profile can pin the verification hostname without repeating it on every
// invocation.
func TestResolveConnectionFlagsAppliesServerName(t *testing.T) {
isolateConfigDir(t)
t.Setenv("LEDGERCTL_PROFILE", "")
t.Setenv("LEDGERCTL_TLS_SERVER_NAME", "")

require.NoError(t, SaveConfig(Config{
Profiles: map[string]Profile{
"prod": {Server: "10.0.0.5:8888", TLSServerName: "ledger.svc.cluster.local"},
},
}))

cmd := newConnCmd()
require.NoError(t, cmd.Flags().Set("profile", "prod"))

require.NoError(t, ResolveConnectionFlags(cmd))

serverName, _ := cmd.Flags().GetString("tls-server-name")
require.Equal(t, "ledger.svc.cluster.local", serverName, "tls-server-name must come from the active profile")
}

// TestResolveConnectionFlagsExplicitFlagWins guards the precedence: an explicit
// --server on the command line is never overwritten by the profile value.
func TestResolveConnectionFlagsExplicitFlagWins(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions cmd/ledgerctl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func newRootCommand() *cobra.Command {
rootCmd.PersistentFlags().String("server", "localhost:8888", "gRPC server address (env: LEDGERCTL_SERVER)")
rootCmd.PersistentFlags().Bool("insecure", false, "Use insecure connection (no TLS) (env: LEDGERCTL_INSECURE)")
rootCmd.PersistentFlags().String("tls-ca-cert", "", "Path to CA certificate file (PEM) for server verification (env: LEDGERCTL_TLS_CA_CERT)")
rootCmd.PersistentFlags().String("tls-server-name", "", "Hostname to verify against the server certificate SANs, overriding the --server host (use when dialing by IP or a name not covered by the cert, e.g. from inside a TLS cluster pod) (env: LEDGERCTL_TLS_SERVER_NAME)")

// Add persistent flags for request signing.
rootCmd.PersistentFlags().String("signing-key", "", "Path to Ed25519 private key file (seed: 32 bytes raw or hex-encoded) (env: LEDGERCTL_SIGNING_KEY)")
Expand Down
3 changes: 3 additions & 0 deletions cmd/ledgerctl/profile/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Use --use to activate it immediately even when other profiles already exist.`,
cmd.Flags().String("server", "", "gRPC server address (required)")
cmd.Flags().Bool("insecure", false, "Use insecure connection (no TLS)")
cmd.Flags().String("tls-ca-cert", "", "Path to CA certificate file (PEM)")
cmd.Flags().String("tls-server-name", "", "Hostname to verify against the server certificate SANs, overriding the --server host")
Comment thread
NumaryBot marked this conversation as resolved.
Comment thread
NumaryBot marked this conversation as resolved.

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.

🟡 [minor] LEDGERCTL_TLS_SERVER_NAME not applied when creating a profile via environment variable

When profile create is invoked without an explicit --tls-server-name flag but with LEDGERCTL_TLS_SERVER_NAME set, the owned-flag exclusion in bindSubcommandEnv means the env var is not read for the local flag, and ResolveConnectionFlags is not called for profile subcommands. The profile is therefore saved without tlsServerName, so subsequent commands that rely on the persisted value fail unless the user repeats --tls-server-name explicitly every time.

Suggestion: Explicitly read LEDGERCTL_TLS_SERVER_NAME (and similarly LEDGERCTL_TLS_CA_CERT) in the profile-create path before persisting the profile, either by calling a shared resolver or by directly checking the prefixed env var and populating the flag default.

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.

Working as intended — declining this one. ResolveConnectionFlags deliberately skips profile-management commands (see resolve.go: "they define local flags with the same names and must not be contaminated by the active profile or environment variables"), so profile create never inherits any LEDGERCTL_* value — not just LEDGERCTL_TLS_SERVER_NAME but also LEDGERCTL_SERVER, LEDGERCTL_INSECURE, LEDGERCTL_TLS_CA_CERT, etc. --tls-server-name is fully consistent with every other connection flag here. Auto-baking an ambient env var into a newly-created profile would break that contract and risk silent cross-contamination between profiles (an env set for cluster A leaking into a profile meant for cluster B). Users who want env-driven values use them at command time (where ResolveConnectionFlags does apply them), not at profile-create time. No change.

cmd.Flags().String("signing-key", "", "Path to Ed25519 private key file for request signing")
cmd.Flags().String("signing-key-id", "", "Key ID for request signatures")
cmd.Flags().String("response-verify-key", "", "Path to Ed25519 seed file for verifying server response signatures")
Expand All @@ -52,6 +53,7 @@ func runCreate(cmd *cobra.Command, args []string) error {
server, _ := cmd.Flags().GetString("server")
insecure, _ := cmd.Flags().GetBool("insecure")
tlsCaCert, _ := cmd.Flags().GetString("tls-ca-cert")
tlsServerName, _ := cmd.Flags().GetString("tls-server-name")
signingKey, _ := cmd.Flags().GetString("signing-key")
signingKeyID, _ := cmd.Flags().GetString("signing-key-id")
responseVerifyKey, _ := cmd.Flags().GetString("response-verify-key")
Expand All @@ -61,6 +63,7 @@ func runCreate(cmd *cobra.Command, args []string) error {
Server: server,
Insecure: insecure,
TLSCaCert: tlsCaCert,
TLSServerName: tlsServerName,
Comment thread
NumaryBot marked this conversation as resolved.
SigningKey: signingKey,
SigningKeyID: signingKeyID,
ResponseVerifyKey: responseVerifyKey,
Expand Down
6 changes: 6 additions & 0 deletions cmd/ledgerctl/profile/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ func runShow(cmd *cobra.Command, args []string) error {
tlsCaCert = p.TLSCaCert
}

tlsServerName := "(none)"
if p.TLSServerName != "" {
tlsServerName = p.TLSServerName
}

kr := cmdutil.GetKeyring(cmd)

authStatus := pterm.Yellow("no token stored")
Expand Down Expand Up @@ -93,6 +98,7 @@ func runShow(cmd *cobra.Command, args []string) error {
{"Server", p.Server},
{"Insecure", insecure},
{"TLS CA Cert", tlsCaCert},
{"TLS Server Name", tlsServerName},
{"Signing Key", signingKey},
{"Signing Key ID", signingKeyID},
{"Response Verify Key", responseVerifyKey},
Expand Down
36 changes: 36 additions & 0 deletions docs/ops/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,47 @@ These flags are available for all commands:
| `--server` | `localhost:8888` | gRPC server address |
| `--insecure` | `false` | Use insecure connection (no TLS) |
| `--tls-ca-cert` | | Path to CA certificate file (PEM) for server verification |
| `--tls-server-name` | | Hostname to verify against the server certificate SANs, overriding the `--server` host. Use when dialing by IP or a name the certificate does not cover (env: `LEDGERCTL_TLS_SERVER_NAME`) |
| `--signing-key` | | Path to Ed25519 private key file (seed: 32 bytes raw or hex-encoded) |
| `--signing-key-id` | `default` | Key ID for request signatures |
| `--response-verify-key` | | Path to Ed25519 public key file for verifying server response signatures |
| `--consistency` | | Read consistency level: `stale`, `leader`, or `linearizable` (default) |
| `--auth-token` | | Bearer token for authentication (JWT string or `@path-to-file`) |

### TLS server name (verifying by name while dialing by IP)

TLS verification matches the hostname in `--server` against the server
certificate's Subject Alternative Names (SANs). When the two cannot line up, use
`--tls-server-name` to override the verification hostname independently of the
dial address.

The common case is running `ledgerctl` **from inside a TLS cluster pod**. The
operator-issued certificate covers the in-cluster DNS names only
(`<pod>.<cluster>-headless.<namespace>.svc.cluster.local`,
`<cluster>.<namespace>.svc.cluster.local`) and deliberately never `localhost`
or `127.0.0.1`. Dialing the default `localhost:8888` therefore fails with a
certificate name mismatch. Two ways to fix it:

```bash
# Option A — dial the in-cluster FQDN directly (matches the cert SANs)
ledgerctl \
--server "$HOSTNAME.ledger-<cluster>-headless.$POD_NAMESPACE.svc.cluster.local:8888" \
--tls-ca-cert "$TLS_CA_CERT_FILE" \
ledgers list

# Option B — dial by IP/loopback, verify against the cert name
ledgerctl \
--server 127.0.0.1:8888 \
--tls-server-name "ledger-<cluster>.<namespace>.svc.cluster.local" \
--tls-ca-cert "$TLS_CA_CERT_FILE" \
ledgers list
```

`--tls-server-name` is rejected together with `--insecure` (conflicting intent —
no TLS vs. verify against this name), matching the `--tls-ca-cert` behavior. It
can also be pinned in a profile (`tlsServerName`) so it does not have to be
repeated on every invocation.

### Read Consistency

By default, all read operations use **linearizable** consistency: the node performs a ReadIndex barrier to ensure it has applied all committed entries before reading from the local store. This guarantees that reads always reflect the latest committed state, but it can block during maintenance windows (e.g. mirror sync, snapshot creation) when the FSM is frozen.
Expand Down Expand Up @@ -3000,6 +3035,7 @@ ledgerctl profile create <name> --server <addr> [flags]
| `--server` | *(required)* | gRPC server address |
| `--insecure` | `false` | Use insecure connection (no TLS) |
| `--tls-ca-cert` | | Path to CA certificate file (PEM) |
| `--tls-server-name` | | Hostname to verify against the server certificate SANs, overriding the `--server` host |
| `--use` | `false` | Set this profile as the active profile |

**Behavior:**
Expand Down
Loading