Skip to content

Advertise SCRAM-SHA-256-PLUS only on TLS connections (like postgres) #477

Open

Description

@sunng87

Problem

ScramAuth advertises SCRAM-SHA-256-PLUS whenever a certificate is configured, regardless of whether the connection is actually over TLS (rebuild_supported_mechanisms builds a static list). Channel binding is only possible over TLS, so advertising -PLUS on a plaintext connection is wrong:

  • An RFC-correct client that supports channel binding and sees -PLUS offered MUST use the p= gs2 flag (RFC 5802 §6) — but it cannot compute tls-server-end-point data without TLS. Postgres avoids this trap by never offering -PLUS on non-TLS connections:

    // auth-scram.c, scram_get_mechanisms
    /*
     * ... Channel binding is only supported with SSL.
     */
    if (port->ssl_in_use)
        appendStringInfoString(buf, SCRAM_SHA_256_PLUS_NAME);
  • Interaction with fix: enforce RFC5802 channel binding negotiation and proof length in SCRAM #476: the gs2-flag checks added there are static — a y flag is rejected whenever a certificate is configured, even on non-TLS connections, where y (or n) is the only option a client has. Postgres rejects y only when ssl_in_use. After this change both the advertisement and the gs2 checks should key off the connection's TLS state, not just certificate configuration.

Scope

  1. Advertise per connection: offer SCRAM-SHA-256-PLUS only when the connection is secure.
  2. Reject -PLUS selection on non-TLS connections in the mechanism dispatch (postgres does this in scram_init: channel binding cannot be performed without SSL).
  3. Make the gs2-flag checks connection-aware: y should be rejected only when the connection is over TLS and a certificate is configured.

API feasibility — what exists today vs what's missing

The TLS state is available and accurate at auth time:

  • negotiate_tls (src/tokio/server.rs) builds DefaultClient::new(addr, true) right after the TLS handshake and false for plain connections, before the startup/auth flow.
  • ClientInfo::is_secure() (src/api/mod.rs) exposes it.
  • Both AuthMechanism::begin(client) and AuthMechanism::next(client, msg) receive the client, so ScramMechanism/SaslMechanism can see it.

Two public surfaces currently can't use it:

  1. SaslMechanismFactory::sasl_mechanism_names(&self) takes no client — the advertised list (src/api/auth/sasl.rs, SaslMechanism::begin) and the dispatch match are connection-blind. Needs either a client-aware variant (e.g. sasl_mechanism_names_for(&self, client: &dyn ClientInfo) with a default impl) or a signature change.
  2. The gs2-flag checks from fix: enforce RFC5802 channel binding negotiation and proof length in SCRAM #476 live in ScramServerAuth::on_client_first_message_with_secret, which has no connection context. Connection-aware enforcement needs state plumbed in — e.g. a setter on the per-connection clone (set_channel_binding_supported(bool)) or a parameter on the method. Low-level users (greptimedb) own the socket and know the TLS state, so either works for them.

So: feasible without restructuring — TLS info reaches the mechanism layer today; only the two surfaces above need extending.

Sketch

  • ScramAuth: filter -PLUS from the advertised names and from the accepted-mechanism check when !client.is_secure().
  • Handler layer sets the channel-binding capability on the per-connection ScramServerAuth clone from client.is_secure(); low-level default stays strict (strict when a certificate is configured), documented.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions