diff --git a/compute_tools/src/compute.rs b/compute_tools/src/compute.rs index 8ceef44d61..09272262de 100644 --- a/compute_tools/src/compute.rs +++ b/compute_tools/src/compute.rs @@ -18,6 +18,7 @@ use std::fs; use std::os::unix::fs::PermissionsExt; use std::path::Path; use std::process::{Command, Stdio}; +use std::str::FromStr; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::RwLock; @@ -126,7 +127,22 @@ impl ComputeNode { fn get_basebackup(&self, lsn: &str) -> Result<()> { let start_time = Utc::now(); - let mut client = Client::connect(&self.pageserver_connstr, NoTls)?; + let mut config = postgres::Config::from_str(&self.pageserver_connstr)?; + + // Like in the neon extension, if the $NEON_AUTH_TOKEN env variable is + // set, use it as the password when connecting to pageserver. + // + // Note: this overrides any password set in the connection string. + match std::env::var("NEON_AUTH_TOKEN") { + Ok(val) => { + info!("Got pageserver auth token from NEON_AUTH_TOKEN env variable"); + config.password(val); + } + Err(std::env::VarError::NotPresent) => info!("NEON_AUTH_TOKEN env variable not set"), + Err(e) => info!("could not parse NEON_AUTH_TOKEN env variable: {}", e), + }; + + let mut client = config.connect(NoTls)?; let basebackup_cmd = match lsn { "0/0" => format!("basebackup {} {}", &self.tenant, &self.timeline), // First start of the compute _ => format!("basebackup {} {} {}", &self.tenant, &self.timeline, lsn), diff --git a/control_plane/src/compute.rs b/control_plane/src/compute.rs index 094d2add8d..730cacf40b 100644 --- a/control_plane/src/compute.rs +++ b/control_plane/src/compute.rs @@ -11,7 +11,6 @@ use std::sync::Arc; use std::time::Duration; use anyhow::{Context, Result}; -use postgres_backend::AuthType; use utils::{ id::{TenantId, TimelineId}, lsn::Lsn, @@ -97,7 +96,7 @@ impl ComputeControlPlane { }); node.create_pgdata()?; - node.setup_pg_conf(self.env.pageserver.pg_auth_type)?; + node.setup_pg_conf()?; self.nodes .insert((tenant_id, node.name.clone()), Arc::clone(&node)); @@ -278,7 +277,7 @@ impl PostgresNode { // Write postgresql.conf with default configuration // and PG_VERSION file to the data directory of a new node. - fn setup_pg_conf(&self, auth_type: AuthType) -> Result<()> { + fn setup_pg_conf(&self) -> Result<()> { let mut conf = PostgresConf::new(); conf.append("max_wal_senders", "10"); conf.append("wal_log_hints", "off"); @@ -302,29 +301,12 @@ impl PostgresNode { let config = &self.pageserver.pg_connection_config; let (host, port) = (config.host(), config.port()); - // Set up authentication - // - // $NEON_AUTH_TOKEN will be replaced with value from environment - // variable during compute pg startup. It is done this way because - // otherwise user will be able to retrieve the value using SHOW - // command or pg_settings - let password = if let AuthType::NeonJWT = auth_type { - "$NEON_AUTH_TOKEN" - } else { - "" - }; - // NOTE avoiding spaces in connection string, because it is less error prone if we forward it somewhere. - // Also note that not all parameters are supported here. Because in compute we substitute $NEON_AUTH_TOKEN - // We parse this string and build it back with token from env var, and for simplicity rebuild - // uses only needed variables namely host, port, user, password. - format!("postgresql://no_user:{password}@{host}:{port}") + // NOTE: avoid spaces in connection string, because it is less error prone if we forward it somewhere. + format!("postgresql://no_user@{host}:{port}") }; conf.append("shared_preload_libraries", "neon"); conf.append_line(""); conf.append("neon.pageserver_connstring", &pageserver_connstr); - if let AuthType::NeonJWT = auth_type { - conf.append("neon.safekeeper_token_env", "$NEON_AUTH_TOKEN"); - } conf.append("neon.tenant_id", &self.tenant_id.to_string()); conf.append("neon.timeline_id", &self.timeline_id.to_string()); if let Some(lsn) = self.lsn { @@ -447,6 +429,8 @@ impl PostgresNode { "DYLD_LIBRARY_PATH", self.env.pg_lib_dir(self.pg_version)?.to_str().unwrap(), ); + + // Pass authentication token used for the connections to pageserver and safekeepers if let Some(token) = auth_token { cmd.env("NEON_AUTH_TOKEN", token); } diff --git a/docs/authentication.md b/docs/authentication.md index dc402d1bca..f768b04c5b 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -106,20 +106,22 @@ Their authentication is just plain PostgreSQL authentication and out of scope fo There is no administrative API except those provided by PostgreSQL. #### Outgoing connections -Compute connects to Pageserver for getting pages. -The connection string is configured by the `neon.pageserver_connstring` PostgreSQL GUC, e.g. `postgresql://no_user:$NEON_AUTH_TOKEN@localhost:15028`. -The environment variable inside the connection string is substituted with -the JWT token. +Compute connects to Pageserver for getting pages. The connection string is +configured by the `neon.pageserver_connstring` PostgreSQL GUC, +e.g. `postgresql://no_user@localhost:15028`. If the `$NEON_AUTH_TOKEN` +environment variable is set, it is used as the password for the connection. (The +pageserver uses JWT tokens for authentication, so the password is really a +token.) -Compute connects to Safekeepers to write and commit data. -The token is the same for all safekeepers. -It's stored in an environment variable, whose name is configured -by the `neon.safekeeper_token_env` PostgreSQL GUC. -If the GUC is unset, no token is passed. +Compute connects to Safekeepers to write and commit data. The list of safekeeper +addresses is given in the `neon.safekeepers` GUC. The connections to the +safekeepers take the password from the `$NEON_AUTH_TOKEN` environment +variable, if set. -Note that both tokens can be (and typically are) the same; -the scope is the tenant and the token is usually passed through the -`$NEON_AUTH_TOKEN` environment variable. +The `compute_ctl` binary that runs before the PostgreSQL server, and launches +PostgreSQL, also makes a connection to the pageserver. It uses it to fetch the +initial "base backup" dump, to initialize the PostgreSQL data directory. It also +uses `$NEON_AUTH_TOKEN` as the password for the connection. ### Pageserver #### Overview diff --git a/pgxn/neon/libpagestore.c b/pgxn/neon/libpagestore.c index a3f34247bb..c44e8fcda5 100644 --- a/pgxn/neon/libpagestore.c +++ b/pgxn/neon/libpagestore.c @@ -46,8 +46,12 @@ PGconn *pageserver_conn = NULL; */ WaitEventSet *pageserver_conn_wes = NULL; -char *page_server_connstring_raw; -char *safekeeper_token_env; +/* GUCs */ +char *neon_timeline; +char *neon_tenant; +int32 max_cluster_size; +char *page_server_connstring; +char *neon_auth_token; int n_unflushed_requests = 0; int flush_every_n_requests = 8; @@ -60,10 +64,37 @@ pageserver_connect(int elevel) { char *query; int ret; + const char *keywords[3]; + const char *values[3]; + int n; Assert(!connected); - pageserver_conn = PQconnectdb(page_server_connstring); + /* + * Connect using the connection string we got from the + * neon.pageserver_connstring GUC. If the NEON_AUTH_TOKEN environment + * variable was set, use that as the password. + * + * The connection options are parsed in the order they're given, so + * when we set the password before the connection string, the + * connection string can override the password from the env variable. + * Seems useful, although we don't currently use that capability + * anywhere. + */ + n = 0; + if (neon_auth_token) + { + keywords[n] = "password"; + values[n] = neon_auth_token; + n++; + } + keywords[n] = "dbname"; + values[n] = page_server_connstring; + n++; + keywords[n] = NULL; + values[n] = NULL; + n++; + pageserver_conn = PQconnectdbParams(keywords, values, 1); if (PQstatus(pageserver_conn) == CONNECTION_BAD) { @@ -125,7 +156,7 @@ pageserver_connect(int elevel) } } - neon_log(LOG, "libpagestore: connected to '%s'", page_server_connstring_raw); + neon_log(LOG, "libpagestore: connected to '%s'", page_server_connstring); connected = true; return true; @@ -354,105 +385,6 @@ check_neon_id(char **newval, void **extra, GucSource source) return **newval == '\0' || HexDecodeString(id, *newval, 16); } -static char * -substitute_pageserver_password(const char *page_server_connstring_raw) -{ - char *host = NULL; - char *port = NULL; - char *user = NULL; - char *auth_token = NULL; - char *err = NULL; - char *page_server_connstring = NULL; - PQconninfoOption *conn_options; - PQconninfoOption *conn_option; - MemoryContext oldcontext; - - /* - * Here we substitute password in connection string with an environment - * variable. To simplify things we construct a connection string back with - * only known options. In particular: host port user and password. We do - * not currently use other options and constructing full connstring in an - * URI shape is quite messy. - */ - - if (page_server_connstring_raw == NULL || page_server_connstring_raw[0] == '\0') - return NULL; - - /* extract the auth token from the connection string */ - conn_options = PQconninfoParse(page_server_connstring_raw, &err); - if (conn_options == NULL) - { - /* The error string is malloc'd, so we must free it explicitly */ - char *errcopy = err ? pstrdup(err) : "out of memory"; - - PQfreemem(err); - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("invalid connection string syntax: %s", errcopy))); - } - - /* - * Trying to populate pageserver connection string with auth token from - * environment. We are looking for password in with placeholder value like - * $ENV_VAR_NAME, so if password field is present and starts with $ we try - * to fetch environment variable value and fail loudly if it is not set. - */ - for (conn_option = conn_options; conn_option->keyword != NULL; conn_option++) - { - if (strcmp(conn_option->keyword, "host") == 0) - { - if (conn_option->val != NULL && conn_option->val[0] != '\0') - host = conn_option->val; - } - else if (strcmp(conn_option->keyword, "port") == 0) - { - if (conn_option->val != NULL && conn_option->val[0] != '\0') - port = conn_option->val; - } - else if (strcmp(conn_option->keyword, "user") == 0) - { - if (conn_option->val != NULL && conn_option->val[0] != '\0') - user = conn_option->val; - } - else if (strcmp(conn_option->keyword, "password") == 0) - { - if (conn_option->val != NULL && conn_option->val[0] != '\0') - { - /* ensure that this is a template */ - if (strncmp(conn_option->val, "$", 1) != 0) - ereport(ERROR, - (errcode(ERRCODE_CONNECTION_EXCEPTION), - errmsg("expected placeholder value in pageserver password starting from $ but found: %s", &conn_option->val[1]))); - - neon_log(LOG, "found auth token placeholder in pageserver conn string '%s'", &conn_option->val[1]); - auth_token = getenv(&conn_option->val[1]); - if (!auth_token) - { - ereport(ERROR, - (errcode(ERRCODE_CONNECTION_EXCEPTION), - errmsg("cannot get auth token, environment variable %s is not set", &conn_option->val[1]))); - } - else - { - neon_log(LOG, "using auth token from environment passed via env"); - } - } - } - } - - /* - * allocate connection string in TopMemoryContext to make sure it is not - * freed - */ - oldcontext = CurrentMemoryContext; - MemoryContextSwitchTo(TopMemoryContext); - page_server_connstring = psprintf("postgresql://%s:%s@%s:%s", user, auth_token ? auth_token : "", host, port); - MemoryContextSwitchTo(oldcontext); - - PQconninfoFree(conn_options); - return page_server_connstring; -} - /* * Module initialization function */ @@ -462,21 +394,12 @@ pg_init_libpagestore(void) DefineCustomStringVariable("neon.pageserver_connstring", "connection string to the page server", NULL, - &page_server_connstring_raw, + &page_server_connstring, "", PGC_POSTMASTER, 0, /* no flags required */ NULL, NULL, NULL); - DefineCustomStringVariable("neon.safekeeper_token_env", - "the environment variable containing JWT token for authentication with Safekeepers, the convention is to either unset or set to $NEON_AUTH_TOKEN", - NULL, - &safekeeper_token_env, - NULL, - PGC_POSTMASTER, - 0, /* no flags required */ - NULL, NULL, NULL); - DefineCustomStringVariable("neon.timeline_id", "Neon timeline_id the server is running on", NULL, @@ -533,26 +456,10 @@ pg_init_libpagestore(void) neon_log(PageStoreTrace, "libpagestore already loaded"); page_server = &api; - /* substitute password in pageserver_connstring */ - page_server_connstring = substitute_pageserver_password(page_server_connstring_raw); - - /* retrieve the token for Safekeeper, if present */ - if (safekeeper_token_env != NULL) { - if (safekeeper_token_env[0] != '$') { - ereport(ERROR, - (errcode(ERRCODE_CONNECTION_EXCEPTION), - errmsg("expected safekeeper auth token environment variable's name starting with $ but found: %s", - safekeeper_token_env))); - } - neon_safekeeper_token = getenv(&safekeeper_token_env[1]); - if (!neon_safekeeper_token) { - ereport(ERROR, - (errcode(ERRCODE_CONNECTION_EXCEPTION), - errmsg("cannot get safekeeper auth token, environment variable %s is not set", - &safekeeper_token_env[1]))); - } - neon_log(LOG, "using safekeeper auth token from environment variable"); - } + /* Retrieve the auth token to use when connecting to pageserver and safekeepers */ + neon_auth_token = getenv("NEON_AUTH_TOKEN"); + if (neon_auth_token) + neon_log(LOG, "using storage auth token from NEON_AUTH_TOKEN environment variable"); if (page_server_connstring && page_server_connstring[0]) { diff --git a/pgxn/neon/libpqwalproposer.c b/pgxn/neon/libpqwalproposer.c index 6b1e6a8bcc..9b6175a621 100644 --- a/pgxn/neon/libpqwalproposer.c +++ b/pgxn/neon/libpqwalproposer.c @@ -51,12 +51,39 @@ walprop_status(WalProposerConn *conn) } WalProposerConn * -walprop_connect_start(char *conninfo) +walprop_connect_start(char *conninfo, char *password) { WalProposerConn *conn; PGconn *pg_conn; + const char *keywords[3]; + const char *values[3]; + int n; - pg_conn = PQconnectStart(conninfo); + /* + * Connect using the given connection string. If the + * NEON_AUTH_TOKEN environment variable was set, use that as + * the password. + * + * The connection options are parsed in the order they're given, so + * when we set the password before the connection string, the + * connection string can override the password from the env variable. + * Seems useful, although we don't currently use that capability + * anywhere. + */ + n = 0; + if (password) + { + keywords[n] = "password"; + values[n] = neon_auth_token; + n++; + } + keywords[n] = "dbname"; + values[n] = conninfo; + n++; + keywords[n] = NULL; + values[n] = NULL; + n++; + pg_conn = PQconnectStartParams(keywords, values, 1); /* * Allocation of a PQconn can fail, and will return NULL. We want to fully diff --git a/pgxn/neon/neon.h b/pgxn/neon/neon.h index da441b783d..3eac8f4570 100644 --- a/pgxn/neon/neon.h +++ b/pgxn/neon/neon.h @@ -13,7 +13,7 @@ #define NEON_H /* GUCs */ -extern char *neon_safekeeper_token; +extern char *neon_auth_token; extern char *neon_timeline; extern char *neon_tenant; diff --git a/pgxn/neon/pagestore_smgr.c b/pgxn/neon/pagestore_smgr.c index ca91112195..5b30641856 100644 --- a/pgxn/neon/pagestore_smgr.c +++ b/pgxn/neon/pagestore_smgr.c @@ -92,14 +92,6 @@ const int SmgrTrace = DEBUG5; page_server_api *page_server; -/* GUCs */ -char *page_server_connstring; - -/*with substituted password*/ -char *neon_timeline; -char *neon_tenant; -int32 max_cluster_size; - /* unlogged relation build states */ typedef enum { diff --git a/pgxn/neon/walproposer.c b/pgxn/neon/walproposer.c index aef2465e54..b0b2a23e3c 100644 --- a/pgxn/neon/walproposer.c +++ b/pgxn/neon/walproposer.c @@ -510,17 +510,9 @@ WalProposerInit(XLogRecPtr flushRecPtr, uint64 systemId) Safekeeper *sk = &safekeeper[n_safekeepers]; int written = 0; - if (neon_safekeeper_token != NULL) { - written = snprintf((char *) &sk->conninfo, MAXCONNINFO, - "host=%s port=%s password=%s dbname=replication options='-c timeline_id=%s tenant_id=%s'", - sk->host, sk->port, neon_safekeeper_token, neon_timeline, - neon_tenant); - } else { - written = snprintf((char *) &sk->conninfo, MAXCONNINFO, - "host=%s port=%s dbname=replication options='-c timeline_id=%s tenant_id=%s'", - sk->host, sk->port, neon_timeline, neon_tenant); - } - + written = snprintf((char *) &sk->conninfo, MAXCONNINFO, + "host=%s port=%s dbname=replication options='-c timeline_id=%s tenant_id=%s'", + sk->host, sk->port, neon_timeline, neon_tenant); if (written > MAXCONNINFO || written < 0) elog(FATAL, "could not create connection string for safekeeper %s:%s", sk->host, sk->port); } @@ -696,7 +688,7 @@ ResetConnection(Safekeeper *sk) /* * Try to establish new connection */ - sk->conn = walprop_connect_start((char *) &sk->conninfo); + sk->conn = walprop_connect_start((char *) &sk->conninfo, neon_auth_token); /* * "If the result is null, then libpq has been unable to allocate a new diff --git a/pgxn/neon/walproposer.h b/pgxn/neon/walproposer.h index 357d6378f8..537c733850 100644 --- a/pgxn/neon/walproposer.h +++ b/pgxn/neon/walproposer.h @@ -454,7 +454,7 @@ extern char *walprop_error_message(WalProposerConn *conn); extern WalProposerConnStatusType walprop_status(WalProposerConn *conn); /* Re-exported PQconnectStart */ -extern WalProposerConn * walprop_connect_start(char *conninfo); +extern WalProposerConn * walprop_connect_start(char *conninfo, char *password); /* Re-exported PQconectPoll */ extern WalProposerConnectPollStatusType walprop_connect_poll(WalProposerConn *conn);