[#20097] YSQL: Supporting HostSSL hba entry in Ysql Conn Mgr

Summary: This diff adds the support for hostssl type logical connections in ysql conn mgr. To faciliate it, ysql conn mgr is sending additional byte in auth pass through packet which denotes the type of logical connection. This byte can take 3 following values: # 'E' : Denotes TCP/IP SSL encrypted logical conn. # 'U' : Denotes TCP/IP unencrypted logical conn. # 'L' : Denotes Unix socket logical connection - This flag is currently not used as we don't support Unix socket connections between client and ysql conn mgr (tracked by GH #20048). Jira: DB-9040 Test Plan: **Fixing Existing Failed Test with Conn Mgr** ```./yb_build.sh --java-test org.yb.pgsql.TestPgEncryption ``` Above test is now enabled to run by default with both ysql conn mgr and postgres port. **Manual Testing** 1. Download dummy cert files ``` wget https://raw.githubusercontent.com/yugabyte/yugabyte-db/master/test_certs/ca.crt wget -O node.<tserver_addr>.key https://raw.githubusercontent.com/yugabyte/yugabyte-db/master/test_certs/ysql.key wget -O node.<tserver_addr>.crt https://raw.githubusercontent.com/yugabyte/yugabyte-db/master/test_certs/ysql.crt chmod 600 ca.crt node.<tserver_addr>.key node.<tserver_addr>.crt CERTS=`pwd` ENABLE_TLS="use_client_to_server_encryption=true,certs_for_client_dir=$CERTS" ``` 2. Start the cluster: ``` ./bin/yugabyted destroy && ./bin/yugabyted start --tserver_flags "ysql_enable_auth=true,enable_ysql_conn_mgr=true,allowed_preview_flags_csv=enable_ysql_conn_mgr,$ENABLE_TLS" --ui false ``` 3. Should be able to make SSL connections: ``` ./bin/ysqlsh -h <tserver_addr> Password for user yugabyte: ysqlsh (11.2-YB-2.21.0.0-b0) SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384, bits: 256, compression: off) Type "help" for help. yugabyte=# ``` Reviewers: janand, jason Reviewed By: janand, jason Subscribers: jason, nkumar, mihnea, yql, janand Differential Revision: https://phorge.dev.yugabyte.com/D31638
yugabyte · Feb 14, 2024 · 1b784fe · 1b784fe
1 parent b39feda
commit 1b784fe
Show file tree

Hide file tree

Showing 6 changed files with 94 additions and 16 deletions.
diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgEncryption.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgEncryption.java
@@ -19,30 +19,54 @@
 import java.nio.file.FileSystems;
 import java.sql.Connection;
 import java.sql.SQLException;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 
 import org.hamcrest.CoreMatchers;
+import org.junit.Assume;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.junit.runners.Parameterized;
 
 import org.yb.client.TestUtils;
-import org.yb.YBTestRunner;
+import org.yb.minicluster.MiniYBClusterBuilder;
+import org.yb.util.SystemUtil;
+import org.yb.YBParameterizedTestRunner;
 
 import com.google.common.collect.ImmutableMap;
 
 /**
  * Tests for PostgreSQL configuration.
  */
-@RunWith(value = YBTestRunner.class)
+@RunWith(value = YBParameterizedTestRunner.class)
 public class TestPgEncryption extends BasePgSQLTest {
-  private static final Logger LOG = LoggerFactory.getLogger(TestPgEncryption.class);
 
-  public TestPgEncryption() {
+  private final ConnectionEndpoint connectionEndpoint;
+
+  @Parameterized.Parameters
+  public static List<ConnectionEndpoint> parameters() {
+    final String enableYsqlConnMgr = System.getenv("YB_ENABLE_YSQL_CONN_MGR_IN_TESTS");
+    if (enableYsqlConnMgr != null && enableYsqlConnMgr.equalsIgnoreCase("true"))
+      return Arrays.asList(ConnectionEndpoint.YSQL_CONN_MGR);
+    if (SystemUtil.IS_LINUX)
+      return Arrays.asList(ConnectionEndpoint.YSQL_CONN_MGR, ConnectionEndpoint.POSTGRES);
+    else
+      return Arrays.asList(ConnectionEndpoint.POSTGRES);
+  }
+
+  public TestPgEncryption(ConnectionEndpoint connectionEndpoint) {
+    this.connectionEndpoint = connectionEndpoint;
     useIpWithCertificate = true;
   }
 
+  @Override
+  protected void customizeMiniClusterBuilder(MiniYBClusterBuilder builder) {
+    super.customizeMiniClusterBuilder(builder);
+    if (connectionEndpoint == ConnectionEndpoint.YSQL_CONN_MGR)
+      builder.enableYsqlConnMgr(true);
+  }
+
   @Override
   protected Map<String, String> getTServerFlags() {
     Map<String, String> flagMap = super.getTServerFlags();
@@ -59,7 +83,8 @@ private static String certsDir() {
   @Test
   public void testSslNoAuth() throws Exception {
     // Client connection with SSL enabled -- should allow connection with or without pass.
-    ConnectionBuilder tsConnBldr = getConnectionBuilder().withSslMode("require");
+    ConnectionBuilder tsConnBldr = getConnectionBuilder().withSslMode("require")
+                                   .withConnectionEndpoint(connectionEndpoint);
     try (Connection ignored = tsConnBldr.withUser("yugabyte").withPassword("yugabyte").connect()) {
       // No-op.
     }
@@ -68,12 +93,17 @@ public void testSslNoAuth() throws Exception {
     }
 
     // Client connection with SSL disabled -- should *not* allow connection with or without pass.
-    tsConnBldr = getConnectionBuilder().withSslMode("disable");
+    tsConnBldr = getConnectionBuilder().withSslMode("disable")
+                                       .withConnectionEndpoint(connectionEndpoint);
     try (Connection ignored = tsConnBldr.withUser("yugabyte").withPassword("yugabyte").connect()) {
       fail("Expected login attempt to fail");
     } catch (SQLException sqle) {
+      // With ysql conn mgr endpoint, the checking of ssl connectivity with client happens at
+      // ysql conn mgr layer itself and returns required / below error.
       assertThat(
         sqle.getMessage(),
+        connectionEndpoint == ConnectionEndpoint.YSQL_CONN_MGR ?
+        CoreMatchers.containsString("SSL is required") :
         CoreMatchers.containsString("no pg_hba.conf entry for")
       );
     }
@@ -83,6 +113,8 @@ public void testSslNoAuth() throws Exception {
     } catch (SQLException sqle) {
       assertThat(
         sqle.getMessage(),
+        connectionEndpoint == ConnectionEndpoint.YSQL_CONN_MGR ?
+        CoreMatchers.containsString("SSL is required") :
         CoreMatchers.containsString("no pg_hba.conf entry for")
       );
     }
@@ -94,6 +126,7 @@ public void testSslWithAuth() throws Exception {
 
     // Client connection with SSL enabled -- should only allow connection with pass (+SSL).
     ConnectionBuilder tsConnBldr = getConnectionBuilder().withSslMode("require")
+                                                         .withConnectionEndpoint(connectionEndpoint)
                                                          .withTServer(tserver);
     try (Connection ignored = tsConnBldr.withUser("yugabyte").withPassword("yugabyte").connect()) {
       // No-op.
@@ -108,12 +141,15 @@ public void testSslWithAuth() throws Exception {
     }
 
     // Client connection with SSL disabled -- should *not* allow connection with or without pass.
-    tsConnBldr = getConnectionBuilder().withSslMode("disable").withTServer(tserver);
+    tsConnBldr = getConnectionBuilder().withSslMode("disable").withTServer(tserver)
+                                       .withConnectionEndpoint(connectionEndpoint);
     try (Connection ignored = tsConnBldr.withUser("yugabyte").withPassword("yugabyte").connect()) {
       fail("Expected login attempt to fail");
     } catch (SQLException sqle) {
       assertThat(
         sqle.getMessage(),
+        connectionEndpoint == ConnectionEndpoint.YSQL_CONN_MGR ?
+        CoreMatchers.containsString("SSL is required") :
         CoreMatchers.containsString("no pg_hba.conf entry for")
       );
     }
@@ -123,13 +159,19 @@ public void testSslWithAuth() throws Exception {
     } catch (SQLException sqle) {
       assertThat(
         sqle.getMessage(),
+        connectionEndpoint == ConnectionEndpoint.YSQL_CONN_MGR ?
+        CoreMatchers.containsString("SSL is required") :
         CoreMatchers.containsString("no pg_hba.conf entry for")
       );
     }
   }
 
   @Test
   public void testSslWithCustomAuth() throws Exception {
+    Assume.assumeFalse("Skipping this test for Ysql Connection Manager as CERT Authentication " +
+        "is currently not supported in Ysql Connection Manager.",
+        connectionEndpoint == ConnectionEndpoint.YSQL_CONN_MGR);
+
     String sslcertFile = String.format("%s/ysql.crt", certsDir());
     String sslkeyFile = String.format("%s/ysql.key.der", certsDir());
     String sslrootcertFile = String.format("%s/%s", certsDir(), "ca.crt");

diff --git a/src/odyssey/sources/yb_auth_passthrough.c b/src/odyssey/sources/yb_auth_passthrough.c
@@ -91,7 +91,8 @@ static int yb_server_write_auth_passthroug_request_pkt(od_client_t *client,
 
 	msg = kiwi_fe_write_authentication(NULL, client->startup.user.value,
 					   client->startup.database.value,
-					   client_address);
+					   client_address,
+					   client->tls ? YB_LOGICAL_ENCRYPTED_CONN : YB_LOGICAL_UNENCRYPTED_CONN);
 
 	/* Send `Auth Passthrough Request` packet. */
 	rc = od_write(&server->io, msg);

diff --git a/src/odyssey/third_party/kiwi/kiwi/fe_write.h b/src/odyssey/third_party/kiwi/kiwi/fe_write.h
@@ -336,13 +336,36 @@ kiwi_fe_write_prep_stmt(machine_msg_t *msg, char *query, char *param)
 	return msg;
 }
 
+/*
+ * yb_logical_conn_type enum identifies type of connection between client
+ * and ysql conn mgr.
+ * 
+ * Below are different values yb_logical_conn_type enum can have:
+ * 
+ * YB_LOGICAL_ENCRYPTED_CONN denotes SSL encrypted, TCP/IP Connection 
+ * between client & ysql conn mgr
+ * 
+ * YB_LOGICAL_UNENCRYPTED_CONN denotes no SSL, TCP/IP Connection between
+ * client & ysql conn mgr
+ * 
+ * YB_LOGICAL_USD_CONN denotes Unix Socket Connection between client &
+ * ysql conn mgr which is currently not supported.
+ * TODO(mkumar) GH #20048 Support for unix socket connectivity b/w client and
+ * odyssey.
+ */
+typedef enum {
+	YB_LOGICAL_ENCRYPTED_CONN = 'E',
+	YB_LOGICAL_UNENCRYPTED_CONN = 'U',
+	YB_LOGICAL_USD_CONN = 'L'
+} yb_logical_conn_type;
+
 KIWI_API static inline machine_msg_t *
 kiwi_fe_write_authentication(machine_msg_t *msg, char *username, char *database,
-			     char *peer)
+			     char *peer, yb_logical_conn_type logical_conn_type )
 {
 	int size = sizeof(kiwi_header_t) +
 		   sizeof(char) * (strlen(username) + strlen(database) +
-				   strlen(peer) + 3);
+				   strlen(peer) + 3) + sizeof(yb_logical_conn_type);
 
 	int offset = 0;
 	if (msg)
@@ -357,6 +380,7 @@ kiwi_fe_write_authentication(machine_msg_t *msg, char *username, char *database,
 	kiwi_write(&pos, username, strlen(username) + 1); // username
 	kiwi_write(&pos, database, strlen(database) + 1); // database
 	kiwi_write(&pos, peer, strlen(peer) + 1); // host
+	kiwi_write8(&pos, logical_conn_type); // conn type
 	return msg;
 }
 
@@ -442,4 +466,5 @@ KIWI_API static inline machine_msg_t *kiwi_fe_copy_msg(machine_msg_t *msg,
 	return msg;
 }
 
-#endif /* KIWI_FE_WRITE_H */
+#endif /* KIWI_FE_WRITE_H */
+
diff --git a/src/postgres/src/backend/libpq/hba.c b/src/postgres/src/backend/libpq/hba.c
@@ -54,6 +54,7 @@
 
 /* YB includes */
 #include "pg_yb_utils.h"
+#include "yb_ysql_conn_mgr_helper.h"
 
 
 #define MAX_TOKEN	256
@@ -2278,7 +2279,9 @@ check_hba(hbaPort *port)
 				continue;
 
 			/* Check SSL state */
-			if (port->ssl_in_use)
+			if (YbIsClientYsqlConnMgr() && port->yb_is_auth_passthrough_req ?
+					port->yb_is_ssl_enabled_in_logical_conn :
+					port->ssl_in_use)
 			{
 				/* Connection is SSL, match both "host" and "hostssl" */
 				if (hba->conntype == ctHostNoSSL)

diff --git a/src/postgres/src/backend/tcop/postgres.c b/src/postgres/src/backend/tcop/postgres.c
@@ -5836,8 +5836,8 @@ PostgresMain(int argc, char *argv[],
 					// HARD Code connection type between client and ysql_conn_mgr to AF_INET (only supported)
 					// for authentication
 					MyProcPort->raddr.addr.ss_family = AF_INET;
-					// TODO(mkumar) GH #20097 Add support for connection type hostssl/hostnossl
-					// in  ysql conn mgr
+					MyProcPort->yb_is_ssl_enabled_in_logical_conn = 
+						pq_getmsgbyte(&input_message) == 'E' ? true : false;
 
 					/* Update the `remote_host` */
 					struct sockaddr_in *ip_address_1;
@@ -5854,6 +5854,7 @@ PostgresMain(int argc, char *argv[],
 
 					/* Place back the old context */
 					MyProcPort->yb_is_auth_passthrough_req = false;
+					MyProcPort->yb_is_ssl_enabled_in_logical_conn = false;
 					MyProcPort->user_name = user_name;
 					MyProcPort->database_name = db_name;
 					MyProcPort->remote_host = host;

diff --git a/src/postgres/src/include/libpq/libpq-be.h b/src/postgres/src/include/libpq/libpq-be.h
@@ -155,6 +155,12 @@ typedef struct Port
 	 */
 	bool 		yb_is_auth_passthrough_req;
 
+	/*
+	 * To be used during Authentication Passthrough (authentication of logical connections),
+	 * it identifies whether the logical connection is encrypted.
+	 */
+	bool 		yb_is_ssl_enabled_in_logical_conn;
+
 	/*
 	 * Information that really has no business at all being in struct Port,
 	 * but since it gets used by elog.c in the same way as database_name and