From 675c1dd318e7b9f18b60d9badaefb3d99af26d36 Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Wed, 4 Dec 2024 15:45:36 +0500 Subject: [PATCH 01/14] Implementation according to SRS --- external-table/src/libchurl.c | 6 +-- fdw/libchurl.c | 45 ++++++++++++++++--- fdw/libchurl.h | 21 +++++++-- fdw/pxf_bridge.c | 85 ++++++++++++++++++++++++++++++++--- fdw/pxf_bridge.h | 1 + fdw/pxf_option.c | 18 ++++++++ fdw/pxf_option.h | 8 ++++ 7 files changed, 165 insertions(+), 19 deletions(-) diff --git a/external-table/src/libchurl.c b/external-table/src/libchurl.c index efc890c31a..1c0a2374e3 100644 --- a/external-table/src/libchurl.c +++ b/external-table/src/libchurl.c @@ -382,7 +382,7 @@ log_curl_debug(CURL *handle, curl_infotype type, char *data, size_t size, void * } static CHURL_HANDLE -churl_init(const char *url, CHURL_HEADERS headers) +churl_init(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options /* = NULL */) { churl_context *context = churl_new_context(); @@ -446,9 +446,9 @@ churl_init_upload_timeout(const char *url, CHURL_HEADERS headers, long timeout) } CHURL_HANDLE -churl_init_download(const char *url, CHURL_HEADERS headers) +churl_init_download(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options) { - churl_context *context = churl_init(url, headers); + churl_context *context = churl_init(url, headers, ssl_options); context->upload = false; diff --git a/fdw/libchurl.c b/fdw/libchurl.c index 6a5d4fcfad..ca87ef8818 100644 --- a/fdw/libchurl.c +++ b/fdw/libchurl.c @@ -114,6 +114,7 @@ static JsonSemAction nullSemAction = churl_context *churl_new_context(void); static void create_curl_handle(churl_context *context); static void set_curl_option(churl_context *context, CURLoption option, const void *data); +static void set_curl_ssl_options(churl_context *context, churl_ssl_options *ssl_options); static size_t read_callback(void *ptr, size_t size, size_t nmemb, void *userdata); static void setup_multi_handle(churl_context *context); static void multi_perform(churl_context *context); @@ -388,7 +389,7 @@ log_curl_debug(CURL *handle, curl_infotype type, char *data, size_t size, void * } static CHURL_HANDLE -churl_init(const char *url, CHURL_HEADERS headers) +churl_init(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options /* = NULL*/) { churl_context *context = churl_new_context(); @@ -422,21 +423,51 @@ churl_init(const char *url, CHURL_HEADERS headers) set_curl_option(context, CURLOPT_WRITEDATA, context); set_curl_option(context, CURLOPT_HEADERFUNCTION, header_callback); set_curl_option(context, CURLOPT_HEADERDATA, context); + + set_curl_ssl_options(context, ssl_options); + churl_headers_set(context, headers); return (CHURL_HANDLE) context; } +static void +set_curl_ssl_options(churl_context *context, churl_ssl_options *ssl_options) +{ + if (ssl_options != NULL) { + const char *cacert = ssl_options->pxf_ssl_cacert; // PXF_SSL_CACERT + + if (ssl_options->pxf_ssl_cert) + set_curl_option(context, CURLOPT_SSLCERT, ssl_options->pxf_ssl_cert); + + if (ssl_options->pxf_ssl_key) + set_curl_option(context, CURLOPT_SSLKEY, ssl_options->pxf_ssl_key); + + if (ssl_options->pxf_ssl_cert_type) + set_curl_option(context, CURLOPT_SSLCERTTYPE, ssl_options->pxf_ssl_cert_type); + + if (ssl_options->pxf_ssl_keypasswd != NULL) { + set_curl_option(context, CURLOPT_SSLKEYPASSWD, ssl_options->pxf_ssl_keypasswd); + } + + if (cacert != NULL && cacert[0] != '\0') { + set_curl_option(context, CURLOPT_CAINFO, cacert); + } + + set_curl_option(context, CURLOPT_SSL_VERIFYPEER, (const void *)ssl_options->pxf_ssl_verify_peer); + } +} + CHURL_HANDLE -churl_init_upload(const char *url, CHURL_HEADERS headers) +churl_init_upload(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options) { - return churl_init_upload_timeout(url, headers, 0); + return churl_init_upload_timeout(url, headers, ssl_options, 0); } CHURL_HANDLE -churl_init_upload_timeout(const char *url, CHURL_HEADERS headers, long timeout) +churl_init_upload_timeout(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options, long timeout) { - churl_context *context = churl_init(url, headers); + churl_context *context = churl_init(url, headers, ssl_options); context->upload = true; @@ -453,9 +484,9 @@ churl_init_upload_timeout(const char *url, CHURL_HEADERS headers, long timeout) } CHURL_HANDLE -churl_init_download(const char *url, CHURL_HEADERS headers) +churl_init_download(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options) { - churl_context *context = churl_init(url, headers); + churl_context *context = churl_init(url, headers, ssl_options); context->upload = false; diff --git a/fdw/libchurl.h b/fdw/libchurl.h index 9f372047f5..cd1960ec54 100644 --- a/fdw/libchurl.h +++ b/fdw/libchurl.h @@ -31,6 +31,18 @@ typedef void *CHURL_HEADERS; typedef void *CHURL_HANDLE; +/* + SSL options +*/ +typedef struct { + char *pxf_ssl_cacert; + char *pxf_ssl_cert; + char *pxf_ssl_cert_type; + char *pxf_ssl_key; + char *pxf_ssl_keypasswd; + long pxf_ssl_verify_peer; +} churl_ssl_options; + /* * PUT example * ----------- @@ -104,8 +116,10 @@ void churl_headers_cleanup(CHURL_HEADERS headers); * Start an upload to url * returns a handle to churl transfer */ -CHURL_HANDLE churl_init_upload(const char *url, CHURL_HEADERS headers); -CHURL_HANDLE churl_init_upload_timeout(const char *url, CHURL_HEADERS headers, long timeout); +CHURL_HANDLE churl_init_upload(const char *url, CHURL_HEADERS headers, + churl_ssl_options *ssl_options); +CHURL_HANDLE churl_init_upload_timeout(const char *url, CHURL_HEADERS headers, + churl_ssl_options *ssl_options, long timeout); /* * Returns local port of connected handle or 0 @@ -116,7 +130,8 @@ int churl_get_local_port(CHURL_HANDLE handle); * Start a download to url * returns a handle to churl transfer */ -CHURL_HANDLE churl_init_download(const char *url, CHURL_HEADERS headers); +CHURL_HANDLE churl_init_download(const char *url, CHURL_HEADERS headers, + churl_ssl_options *ssl_options); /* * Restart a session to a new URL diff --git a/fdw/pxf_bridge.c b/fdw/pxf_bridge.c index b009dd842e..c6950ac0a8 100644 --- a/fdw/pxf_bridge.c +++ b/fdw/pxf_bridge.c @@ -32,8 +32,10 @@ typedef struct PxfFdwCancelState CHURL_HANDLE churl_handle; ResourceOwner owner; StringInfoData uri; + char *pxf_protocol; int pxf_port; /* port number for the PXF Service */ char *pxf_host; /* hostname for the PXF Service */ + churl_ssl_options *ssl_options; /* NULL if SSL not configured */ } PxfFdwCancelState; /* helper function declarations */ @@ -47,6 +49,50 @@ static size_t FillBuffer(PxfFdwScanState *pxfsstate, char *start, int minlen, in static size_t FillBuffer(PxfFdwScanState *pxfsstate, char *start, size_t size); #endif +static churl_ssl_options *churl_make_ssl_options(PxfOptions *options) +{ + churl_ssl_options *ssl_options = palloc0(sizeof(churl_ssl_options)); + + if (options->pxf_ssl_cacert) + ssl_options->pxf_ssl_cacert = pstrdup(options->pxf_ssl_cacert); + + if (options->pxf_ssl_cert) + ssl_options->pxf_ssl_cert = pstrdup(options->pxf_ssl_cert); + + if (options->pxf_ssl_cert_type) + ssl_options->pxf_ssl_cert_type = pstrdup(options->pxf_ssl_cert_type); + + if (options->pxf_ssl_key) + ssl_options->pxf_ssl_key = pstrdup(options->pxf_ssl_key); + + if (options->pxf_ssl_keypasswd) + ssl_options->pxf_ssl_keypasswd = pstrdup(options->pxf_ssl_keypasswd); + + ssl_options->pxf_ssl_verify_peer = options->pxf_ssl_verify_peer; + + return ssl_options; +} + +static void free_churl_ssl_options(churl_ssl_options *ssl_options) +{ + if (ssl_options->pxf_ssl_cacert) + pfree(ssl_options->pxf_ssl_cacert); + + if (ssl_options->pxf_ssl_cert) + pfree(ssl_options->pxf_ssl_cert); + + if (ssl_options->pxf_ssl_cert_type) + pfree(ssl_options->pxf_ssl_cert_type); + + if (ssl_options->pxf_ssl_key) + pfree(ssl_options->pxf_ssl_key); + + if (ssl_options->pxf_ssl_keypasswd) + pfree(ssl_options->pxf_ssl_keypasswd); + + pfree(ssl_options); +} + static void PxfBridgeAbortCallback(ResourceReleasePhase phase, bool isCommit, @@ -84,7 +130,7 @@ PxfBridgeCancel(PxfFdwCancelState *pxfcstate) initStringInfo(&pxfcstate->uri); BuildUriForCancel(pxfcstate); - churl_handle = churl_init_upload_timeout(pxfcstate->uri.data, pxfcstate->churl_headers, 1L); + churl_handle = churl_init_upload_timeout(pxfcstate->uri.data, pxfcstate->churl_headers, pxfcstate->ssl_options, 1L); churl_cleanup(churl_handle, false); } @@ -112,9 +158,16 @@ PxfBridgeCancelCleanup(PxfFdwCancelState *pxfcstate) if (IsAbortInProgress()) PxfBridgeCancel(pxfcstate); + if (pxfcstate->pxf_protocol) + pfree(pxfcstate->pxf_protocol); + if (pxfcstate->pxf_host) pfree(pxfcstate->pxf_host); + if (pxfcstate->ssl_options) { + free_churl_ssl_options(pxfcstate->ssl_options); + } + pfree(pxfcstate); } @@ -181,6 +234,7 @@ PxfBridgeImportStart(PxfFdwScanState *pxfsstate) { MemoryContext oldcontext; PxfFdwCancelState *pxfcstate; + churl_ssl_options *ssl_options = NULL; pxfsstate->churl_headers = churl_headers_init(); @@ -192,11 +246,23 @@ PxfBridgeImportStart(PxfFdwScanState *pxfsstate) pxfsstate->retrieved_attrs, pxfsstate->projectionInfo); - pxfsstate->churl_handle = churl_init_download(pxfsstate->uri.data, pxfsstate->churl_headers); + if (strcmp("https", pxfsstate->options->pxf_protocol) == 0) { + ssl_options = churl_make_ssl_options(pxfsstate->options); + } + + pxfsstate->churl_handle = churl_init_download(pxfsstate->uri.data, pxfsstate->churl_headers, ssl_options); + if (ssl_options != NULL) { + free_churl_ssl_options(ssl_options); + } oldcontext = MemoryContextSwitchTo(CurTransactionContext); pxfcstate = palloc0(sizeof(PxfFdwCancelState)); + pxfcstate->pxf_protocol = pstrdup(pxfsstate->options->pxf_protocol); pxfcstate->pxf_host = pstrdup(pxfsstate->options->pxf_host); + if (ssl_options != 0) { + /* make another copy for transaction context */ + pxfcstate->ssl_options = churl_make_ssl_options(pxfsstate->options); + } MemoryContextSwitchTo(oldcontext); pxfsstate->pxfcstate = pxfcstate; pxfcstate->churl_headers = pxfsstate->churl_headers; @@ -223,7 +289,7 @@ PxfBridgeExportStart(PxfFdwModifyState *pxfmstate) NULL, NULL, NULL); - pxfmstate->churl_handle = churl_init_upload(pxfmstate->uri.data, pxfmstate->churl_headers); + pxfmstate->churl_handle = churl_init_upload(pxfmstate->uri.data, pxfmstate->churl_headers, pxfmstate->ssl_options); } /* @@ -280,8 +346,11 @@ PxfBridgeWrite(PxfFdwModifyState *pxfmstate, char *databuf, int datalen) static void BuildUriForCancel(PxfFdwCancelState *pxfcstate) { + const char *proto = strcmp("https", pxfcstate->pxf_protocol) == 0 ? "https" : "http"; + resetStringInfo(&pxfcstate->uri); - appendStringInfo(&pxfcstate->uri, "http://%s:%d/%s/cancel", pxfcstate->pxf_host, pxfcstate->pxf_port, PXF_SERVICE_PREFIX); + appendStringInfo(&pxfcstate->uri, "%s://%s:%d/%s/cancel", + proto, pxfcstate->pxf_host, pxfcstate->pxf_port, PXF_SERVICE_PREFIX); elog(DEBUG2, "pxf_fdw: uri %s for cancel", pxfcstate->uri.data); } @@ -292,9 +361,11 @@ static void BuildUriForRead(PxfFdwScanState *pxfsstate) { PxfOptions *options = pxfsstate->options; + const char *proto = strcmp("https", options->pxf_protocol) == 0 ? "https" : "http"; resetStringInfo(&pxfsstate->uri); - appendStringInfo(&pxfsstate->uri, "http://%s:%d/%s/read", options->pxf_host, options->pxf_port, PXF_SERVICE_PREFIX); + appendStringInfo(&pxfsstate->uri, "%s://%s:%d/%s/read", + proto, options->pxf_host, options->pxf_port, PXF_SERVICE_PREFIX); elog(DEBUG2, "pxf_fdw: uri %s for read", pxfsstate->uri.data); } @@ -305,9 +376,11 @@ static void BuildUriForWrite(PxfFdwModifyState *pxfmstate) { PxfOptions *options = pxfmstate->options; + const char *proto = strcmp("https", options->pxf_protocol) == 0 ? "https" : "http"; resetStringInfo(&pxfmstate->uri); - appendStringInfo(&pxfmstate->uri, "http://%s:%d/%s/write", options->pxf_host, options->pxf_port, PXF_SERVICE_PREFIX); + appendStringInfo(&pxfmstate->uri, "%s://%s:%d/%s/write", + proto, options->pxf_host, options->pxf_port, PXF_SERVICE_PREFIX); elog(DEBUG2, "pxf_fdw: uri %s with file name for write: %s", pxfmstate->uri.data, options->resource); } diff --git a/fdw/pxf_bridge.h b/fdw/pxf_bridge.h index def672c696..623d5cd591 100644 --- a/fdw/pxf_bridge.h +++ b/fdw/pxf_bridge.h @@ -65,6 +65,7 @@ typedef struct PxfFdwModifyState CHURL_HANDLE churl_handle; /* curl handle */ CHURL_HEADERS churl_headers; /* curl headers */ + churl_ssl_options *ssl_options; /* NULL if SSL not used */ StringInfoData uri; /* rest endpoint URI for modify */ Relation relation; PxfOptions *options; /* FDW options */ diff --git a/fdw/pxf_option.c b/fdw/pxf_option.c index a0d6205524..c600c11db8 100644 --- a/fdw/pxf_option.c +++ b/fdw/pxf_option.c @@ -36,6 +36,12 @@ #define FDW_OPTION_PXF_HOST "pxf_host" #define FDW_OPTION_PXF_PORT "pxf_port" #define FDW_OPTION_PXF_PROTOCOL "pxf_protocol" +#define FDW_OPTION_SSL_CACERT "pxf_ssl_cacert" +#define FDW_OPTION_SSL_CERT "pxf_ssl_cert" +#define FDW_OPTION_SSL_CERT_TYPE "pxf_ssl_cert_type" +#define FDW_OPTION_SSL_KEY "pxf_ssl_key" +#define FDW_OPTION_SSL_KEYPASSWD "pxf_ssl_keypasswd" +#define FDW_OPTION_SSL_VERIFY_PEER "pxf_ssl_verify_peer" #define FDW_OPTION_REJECT_LIMIT "reject_limit" #define FDW_OPTION_REJECT_LIMIT_TYPE "reject_limit_type" #define FDW_OPTION_RESOURCE "resource" @@ -446,6 +452,18 @@ PxfGetOptions(Oid foreigntableid) opt->pxf_port = atoi(defGetString(def)); else if (strcmp(def->defname, FDW_OPTION_PXF_PROTOCOL) == 0) opt->pxf_protocol = defGetString(def); + else if (strcmp(def->defname, FDW_OPTION_SSL_CACERT) == 0) + opt->pxf_ssl_cacert = defGetString(def); + else if (strcmp(def->defname, FDW_OPTION_SSL_CERT) == 0) + opt->pxf_ssl_cert = defGetString(def); + else if (strcmp(def->defname, FDW_OPTION_SSL_CERT_TYPE) == 0) + opt->pxf_ssl_cert_type = defGetString(def); + else if (strcmp(def->defname, FDW_OPTION_SSL_KEY) == 0) + opt->pxf_ssl_key = defGetString(def); + else if (strcmp(def->defname, FDW_OPTION_SSL_KEYPASSWD) == 0) + opt->pxf_ssl_keypasswd = defGetString(def); + else if (strcmp(def->defname, FDW_OPTION_SSL_VERIFY_PEER) == 0) + opt->pxf_ssl_verify_peer = atol(defGetString(def)); else if (strcmp(def->defname, FDW_OPTION_PROTOCOL) == 0) opt->protocol = defGetString(def); else if (strcmp(def->defname, FDW_OPTION_RESOURCE) == 0) diff --git a/fdw/pxf_option.h b/fdw/pxf_option.h index 8b8fc0804d..d57109931e 100644 --- a/fdw/pxf_option.h +++ b/fdw/pxf_option.h @@ -27,6 +27,14 @@ typedef struct PxfOptions char *pxf_protocol; /* protocol for the PXF Service (i.e HTTP or * HTTPS) */ + /* SSL options, used when protocol is HTTPS */ + const char *pxf_ssl_cacert; + const char *pxf_ssl_cert; + const char *pxf_ssl_cert_type; + const char *pxf_ssl_key; + const char *pxf_ssl_keypasswd; + long pxf_ssl_verify_peer; + /* Server doesn't come from options, it is the actual SERVER name */ char *server; /* the name of the external server */ From 0592f3c69e5472ef2cc1661ae51e196bc1ea4c13 Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Mon, 16 Dec 2024 08:42:41 +0500 Subject: [PATCH 02/14] Revert unwanted change --- external-table/src/libchurl.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/external-table/src/libchurl.c b/external-table/src/libchurl.c index 1c0a2374e3..f7932f6c80 100644 --- a/external-table/src/libchurl.c +++ b/external-table/src/libchurl.c @@ -382,7 +382,7 @@ log_curl_debug(CURL *handle, curl_infotype type, char *data, size_t size, void * } static CHURL_HANDLE -churl_init(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options /* = NULL */) +churl_init(const char *url, CHURL_HEADERS headers) { churl_context *context = churl_new_context(); @@ -446,9 +446,8 @@ churl_init_upload_timeout(const char *url, CHURL_HEADERS headers, long timeout) } CHURL_HANDLE -churl_init_download(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options) +churl_init_download(const char *url, CHURL_HEADERS headers) { - churl_context *context = churl_init(url, headers, ssl_options); context->upload = false; From eadd22ec27e8b3146309386b4852896c78fae2fb Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Mon, 16 Dec 2024 08:44:24 +0500 Subject: [PATCH 03/14] Revert unwanted change --- external-table/src/libchurl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/external-table/src/libchurl.c b/external-table/src/libchurl.c index f7932f6c80..efc890c31a 100644 --- a/external-table/src/libchurl.c +++ b/external-table/src/libchurl.c @@ -448,6 +448,7 @@ churl_init_upload_timeout(const char *url, CHURL_HEADERS headers, long timeout) CHURL_HANDLE churl_init_download(const char *url, CHURL_HEADERS headers) { + churl_context *context = churl_init(url, headers); context->upload = false; From 3386d35e74bbb8a043f6c78e6f6d872a49857760 Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Mon, 16 Dec 2024 17:43:49 +0500 Subject: [PATCH 04/14] Implement validation --- fdw/pxf_option.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/fdw/pxf_option.c b/fdw/pxf_option.c index c600c11db8..6980ca8fab 100644 --- a/fdw/pxf_option.c +++ b/fdw/pxf_option.c @@ -248,6 +248,30 @@ pxf_fdw_validator(PG_FUNCTION_ARGS) errmsg("the %s option cannot be defined at the foreign-data wrapper level", FDW_OPTION_DISABLE_PPD))); } + else if (strcmp(def->defname, FDW_OPTION_SSL_CACERT) == 0 || + strcmp(def->defname, FDW_OPTION_SSL_CERT) == 0 || + strcmp(def->defname, FDW_OPTION_SSL_CERT_TYPE) == 0 || + strcmp(def->defname, FDW_OPTION_SSL_KEY) == 0 || + strcmp(def->defname, FDW_OPTION_SSL_KEYPASSWD) == 0) + { + (void) defGetString(def); /* call is required for validation */ + + if (catalog != ForeignDataWrapperRelationId) + ereport(ERROR, + (errcode(ERRCODE_FDW_INVALID_OPTION_NAME), + errmsg("the %s option can be defined only at the wrapper level", + def->defname))); + } + else if (strcmp(def->defname, FDW_OPTION_SSL_VERIFY_PEER) == 0) + { + (void) defGetBoolean(def); /* call is required for validation */ + + if (catalog != ForeignDataWrapperRelationId) + ereport(ERROR, + (errcode(ERRCODE_FDW_INVALID_OPTION_NAME), + errmsg("the %s option can be defined only at the wrapper level", + def->defname))); + } else if (IsCopyOption(def->defname)) copy_options = lappend(copy_options, def); } @@ -463,7 +487,7 @@ PxfGetOptions(Oid foreigntableid) else if (strcmp(def->defname, FDW_OPTION_SSL_KEYPASSWD) == 0) opt->pxf_ssl_keypasswd = defGetString(def); else if (strcmp(def->defname, FDW_OPTION_SSL_VERIFY_PEER) == 0) - opt->pxf_ssl_verify_peer = atol(defGetString(def)); + opt->pxf_ssl_verify_peer = defGetBoolean(def); else if (strcmp(def->defname, FDW_OPTION_PROTOCOL) == 0) opt->protocol = defGetString(def); else if (strcmp(def->defname, FDW_OPTION_RESOURCE) == 0) From e82be596c36826cb3ba375a6d7617f06c622d630 Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Tue, 17 Dec 2024 20:00:20 +0500 Subject: [PATCH 05/14] Rework ssl options initialization for Pxf bridge --- fdw/pxf_bridge.c | 12 +++++++++++- fdw/pxf_bridge.h | 1 - 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/fdw/pxf_bridge.c b/fdw/pxf_bridge.c index c6950ac0a8..c48919c7cb 100644 --- a/fdw/pxf_bridge.c +++ b/fdw/pxf_bridge.c @@ -281,6 +281,7 @@ PxfBridgeImportStart(PxfFdwScanState *pxfsstate) void PxfBridgeExportStart(PxfFdwModifyState *pxfmstate) { + churl_ssl_options *ssl_options = NULL; /* NULL if SSL not used */ BuildUriForWrite(pxfmstate); pxfmstate->churl_headers = churl_headers_init(); BuildHttpHeaders(pxfmstate->churl_headers, @@ -289,7 +290,16 @@ PxfBridgeExportStart(PxfFdwModifyState *pxfmstate) NULL, NULL, NULL); - pxfmstate->churl_handle = churl_init_upload(pxfmstate->uri.data, pxfmstate->churl_headers, pxfmstate->ssl_options); + + if (strcmp("https", pxfmstate->options->pxf_protocol) == 0) { + ssl_options = churl_make_ssl_options(pxfmstate->options); + } + + pxfmstate->churl_handle = churl_init_upload(pxfmstate->uri.data, pxfmstate->churl_headers, ssl_options); + + if (ssl_options != NULL) { + free_churl_ssl_options(ssl_options); + } } /* diff --git a/fdw/pxf_bridge.h b/fdw/pxf_bridge.h index 623d5cd591..def672c696 100644 --- a/fdw/pxf_bridge.h +++ b/fdw/pxf_bridge.h @@ -65,7 +65,6 @@ typedef struct PxfFdwModifyState CHURL_HANDLE churl_handle; /* curl handle */ CHURL_HEADERS churl_headers; /* curl headers */ - churl_ssl_options *ssl_options; /* NULL if SSL not used */ StringInfoData uri; /* rest endpoint URI for modify */ Relation relation; PxfOptions *options; /* FDW options */ From 97b5a3a216236346819c394ed67691e19407fc4d Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Mon, 23 Dec 2024 09:57:21 +0500 Subject: [PATCH 06/14] Adjust indentation --- fdw/pxf_option.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fdw/pxf_option.h b/fdw/pxf_option.h index d57109931e..53947355a2 100644 --- a/fdw/pxf_option.h +++ b/fdw/pxf_option.h @@ -28,12 +28,12 @@ typedef struct PxfOptions * HTTPS) */ /* SSL options, used when protocol is HTTPS */ - const char *pxf_ssl_cacert; - const char *pxf_ssl_cert; - const char *pxf_ssl_cert_type; - const char *pxf_ssl_key; - const char *pxf_ssl_keypasswd; - long pxf_ssl_verify_peer; + const char *pxf_ssl_cacert; + const char *pxf_ssl_cert; + const char *pxf_ssl_cert_type; + const char *pxf_ssl_key; + const char *pxf_ssl_keypasswd; + long pxf_ssl_verify_peer; /* Server doesn't come from options, it is the actual SERVER name */ char *server; /* the name of the external server */ From 74f2f3d98f1de010cb1f3c6ddfd92a70ebb7a55f Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Mon, 23 Dec 2024 10:08:22 +0500 Subject: [PATCH 07/14] Move SSL options validation to the ValidateOption --- fdw/pxf_option.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/fdw/pxf_option.c b/fdw/pxf_option.c index 6980ca8fab..f3d51ef63c 100644 --- a/fdw/pxf_option.c +++ b/fdw/pxf_option.c @@ -78,6 +78,13 @@ static const struct PxfFdwOption valid_options[] = { {FDW_OPTION_REJECT_LIMIT_TYPE, ForeignTableRelationId}, {FDW_OPTION_LOG_ERRORS, ForeignTableRelationId}, + /* SSL related options */ + {FDW_OPTION_SSL_CACERT, ForeignDataWrapperRelationId}, + {FDW_OPTION_SSL_CERT, ForeignDataWrapperRelationId}, + {FDW_OPTION_SSL_CERT_TYPE, ForeignDataWrapperRelationId}, + {FDW_OPTION_SSL_KEY, ForeignDataWrapperRelationId}, + {FDW_OPTION_SSL_KEYPASSWD, ForeignDataWrapperRelationId}, + /* Sentinel */ {NULL, InvalidOid} }; @@ -248,20 +255,6 @@ pxf_fdw_validator(PG_FUNCTION_ARGS) errmsg("the %s option cannot be defined at the foreign-data wrapper level", FDW_OPTION_DISABLE_PPD))); } - else if (strcmp(def->defname, FDW_OPTION_SSL_CACERT) == 0 || - strcmp(def->defname, FDW_OPTION_SSL_CERT) == 0 || - strcmp(def->defname, FDW_OPTION_SSL_CERT_TYPE) == 0 || - strcmp(def->defname, FDW_OPTION_SSL_KEY) == 0 || - strcmp(def->defname, FDW_OPTION_SSL_KEYPASSWD) == 0) - { - (void) defGetString(def); /* call is required for validation */ - - if (catalog != ForeignDataWrapperRelationId) - ereport(ERROR, - (errcode(ERRCODE_FDW_INVALID_OPTION_NAME), - errmsg("the %s option can be defined only at the wrapper level", - def->defname))); - } else if (strcmp(def->defname, FDW_OPTION_SSL_VERIFY_PEER) == 0) { (void) defGetBoolean(def); /* call is required for validation */ From 607a381e77d040e99e7e3e00e2214ba3464e756c Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Mon, 23 Dec 2024 10:13:45 +0500 Subject: [PATCH 08/14] Use NULL instead of 0 --- fdw/pxf_bridge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdw/pxf_bridge.c b/fdw/pxf_bridge.c index c48919c7cb..ec75de4e8c 100644 --- a/fdw/pxf_bridge.c +++ b/fdw/pxf_bridge.c @@ -259,7 +259,7 @@ PxfBridgeImportStart(PxfFdwScanState *pxfsstate) pxfcstate = palloc0(sizeof(PxfFdwCancelState)); pxfcstate->pxf_protocol = pstrdup(pxfsstate->options->pxf_protocol); pxfcstate->pxf_host = pstrdup(pxfsstate->options->pxf_host); - if (ssl_options != 0) { + if (ssl_options != NULL) { /* make another copy for transaction context */ pxfcstate->ssl_options = churl_make_ssl_options(pxfsstate->options); } From c5d24f71e5e44d92d1e19248a7efdc3199a5254a Mon Sep 17 00:00:00 2001 From: Andrey Sokolov Date: Tue, 24 Dec 2024 17:18:24 +0300 Subject: [PATCH 09/14] Implement TLS options for PXF external tables (#141) TLS options implemented for PXF external tables The requirement for using TLS is implemented for PXF external tables. Communication takes place between the segment node and the PXF server and is pointless without the PXF server. Testing should be done in conjunction with PXF server changes and is out of scope for this commit. PXF external tables are controlled by environment variables. The following options are affected by this commit: PXF_PROTOCOL: Type string, default http. Defines the protocol for communication with the PXF server. Possible values are http and https. PXF_SSL_CACERT: Type string, default /home/gpadmin/arenadata_configs/cacert.pem. Specifies the absolute path to the CA certificate that signed the server certificate. Refers to CURLOPT_CAINFO. PXF_SSL_CERT: Type string, default client.pem. Specifies the path to the client certificate. Refers to CURLOPT_SSLCERT. PXF_SSL_CERT_TYPE: Type string, default pem. Defines the type of the client certificate. Refers to CURLOPT_SSLCERTTYPE. PXF_SSL_KEY: Type string, default empty. Defines the path to the client's SSL private key. Refers to CURLOPT_SSLKEY. PXF_SSL_KEYPASSWD: Type string, default empty. Defines the password for the client SSL private key. Refers to CURLOPT_KEYPASSWD. PXF_SSL_VERIFY_PEER: Type long, default 1. Defines whether the client should verify the server certificate. Refers to CURLOPT_SSL_VERIFYPEER. --------- Co-authored-by: Denis Kovalev --- external-table/src/libchurl.c | 28 ++++++++++++++++ external-table/src/pxfbridge.c | 12 +++---- external-table/src/pxfutils.c | 61 ++++++++++++++++++++++++++++++++++ external-table/src/pxfutils.h | 42 ++++++++++++++++++----- 4 files changed, 128 insertions(+), 15 deletions(-) diff --git a/external-table/src/libchurl.c b/external-table/src/libchurl.c index efc890c31a..14c8a77b46 100644 --- a/external-table/src/libchurl.c +++ b/external-table/src/libchurl.c @@ -108,6 +108,7 @@ static JsonSemAction nullSemAction = churl_context *churl_new_context(void); static void create_curl_handle(churl_context *context); static void set_curl_option(churl_context *context, CURLoption option, const void *data); +static void set_curl_ssl_options(churl_context *context); static size_t read_callback(void *ptr, size_t size, size_t nmemb, void *userdata); static void setup_multi_handle(churl_context *context); static void multi_perform(churl_context *context); @@ -415,11 +416,38 @@ churl_init(const char *url, CHURL_HEADERS headers) set_curl_option(context, CURLOPT_WRITEDATA, context); set_curl_option(context, CURLOPT_HEADERFUNCTION, header_callback); set_curl_option(context, CURLOPT_HEADERDATA, context); + + set_curl_ssl_options(context); + churl_headers_set(context, headers); return (CHURL_HANDLE) context; } +static void +set_curl_ssl_options(churl_context *context) +{ + const char *proto = get_pxf_protocol(); + + if (proto && strcmp(proto, "https") == 0) + { + const char *cacert = get_pxf_ssl_cacert(); + + set_curl_option(context, CURLOPT_SSLCERT, get_pxf_ssl_cert()); + set_curl_option(context, CURLOPT_SSLKEY, get_pxf_ssl_key()); + + set_curl_option(context, CURLOPT_SSLCERTTYPE, get_pxf_ssl_certtype()); + set_curl_option(context, CURLOPT_KEYPASSWD, get_pxf_ssl_keypasswd()); + + if (cacert != NULL && cacert[0] != '\0') + { + set_curl_option(context, CURLOPT_CAINFO, cacert); + } + + set_curl_option(context, CURLOPT_SSL_VERIFYPEER, (const void *) get_pxf_ssl_verifypeer()); + } +} + CHURL_HANDLE churl_init_upload(const char *url, CHURL_HEADERS headers) { diff --git a/external-table/src/pxfbridge.c b/external-table/src/pxfbridge.c index 80267a04a5..65eda8ea1e 100644 --- a/external-table/src/pxfbridge.c +++ b/external-table/src/pxfbridge.c @@ -230,8 +230,8 @@ static void build_uri_for_cancel(pxfbridge_cancel *cancel) { resetStringInfo(&cancel->uri); - appendStringInfo(&cancel->uri, "http://%s/%s/cancel", - get_authority(), PXF_SERVICE_PREFIX); + appendStringInfo(&cancel->uri, "%s://%s/%s/cancel", + get_pxf_protocol(), get_authority(), PXF_SERVICE_PREFIX); if ((LOG >= log_min_messages) || (LOG >= client_min_messages)) { @@ -248,8 +248,8 @@ static void build_uri_for_read(gphadoop_context *context) { resetStringInfo(&context->uri); - appendStringInfo(&context->uri, "http://%s/%s/read", - get_authority(), PXF_SERVICE_PREFIX); + appendStringInfo(&context->uri, "%s://%s/%s/read", + get_pxf_protocol(), get_authority(), PXF_SERVICE_PREFIX); if ((LOG >= log_min_messages) || (LOG >= client_min_messages)) { @@ -265,8 +265,8 @@ build_uri_for_read(gphadoop_context *context) static void build_uri_for_write(gphadoop_context *context) { - appendStringInfo(&context->uri, "http://%s/%s/write", - get_authority(), PXF_SERVICE_PREFIX); + appendStringInfo(&context->uri, "%s://%s/%s/write", + get_pxf_protocol(), get_authority(), PXF_SERVICE_PREFIX); if ((LOG >= log_min_messages) || (LOG >= client_min_messages)) { diff --git a/external-table/src/pxfutils.c b/external-table/src/pxfutils.c index c6a01e6ca1..37f59bd8f7 100644 --- a/external-table/src/pxfutils.c +++ b/external-table/src/pxfutils.c @@ -8,6 +8,9 @@ #include "utils/formatting.h" #include "utils/syscache.h" +static const char *getenv_char(const char *name, const char *default_value); +static long getenv_long(const char *name, long default_value); + /* * Full name of the HEADER KEY expected by the PXF service * Converts input string to upper case and prepends "X-GP-OPTIONS-" string @@ -69,6 +72,22 @@ concat(int num_args,...) return str.data; } +static const char* +getenv_char(const char *name, const char *default_value) +{ + const char *value = getenv(name); + + return value ? value : default_value; +} + +static long +getenv_long(const char *name, long default_value) +{ + const char *value = getenv(name); + + return value ? atol(value) : default_value; +} + /* Get authority (host:port) for the PXF server URL */ char * get_authority(void) @@ -76,6 +95,12 @@ get_authority(void) return psprintf("%s:%d", get_pxf_host(), get_pxf_port()); } +const char * +get_pxf_protocol(void) +{ + return getenv_char(ENV_PXF_PROTOCOL, PXF_DEFAULT_PROTOCOL); +} + /* Returns the PXF Host defined in the PXF_HOST * environment variable or the default when undefined */ @@ -116,6 +141,42 @@ get_pxf_port(void) return port; } +const char * +get_pxf_ssl_keypasswd(void) +{ + return getenv_char(ENV_PXF_SSL_KEYPASSWD, PXF_DEFAULT_SSL_KEYPASSWD); +} + +const char * +get_pxf_ssl_cacert(void) +{ + return getenv_char(ENV_PXF_SSL_CACERT, PXF_DEFAULT_SSL_CACERT); +} + +const char * +get_pxf_ssl_cert(void) +{ + return getenv_char(ENV_PXF_SSL_CERT, PXF_DEFAULT_SSL_CERT); +} + +const char * +get_pxf_ssl_key(void) +{ + return getenv_char(ENV_PXF_SSL_KEY, PXF_DEFAULT_SSL_KEY); +} + +const char * +get_pxf_ssl_certtype(void) +{ + return getenv_char(ENV_PXF_SSL_CERT_TYPE, PXF_DEFAULT_SSL_CERT_TYPE); +} + +long +get_pxf_ssl_verifypeer(void) +{ + return getenv_long(ENV_PXF_SSL_VERIFY_PEER, PXF_DEFAULT_SSL_VERIFY_PEER); +} + /* Returns the namespace (schema) name for a given namespace oid */ char * GetNamespaceName(Oid nsp_oid) diff --git a/external-table/src/pxfutils.h b/external-table/src/pxfutils.h index db930e5fb3..a622964f1d 100644 --- a/external-table/src/pxfutils.h +++ b/external-table/src/pxfutils.h @@ -12,6 +12,9 @@ char *TypeOidGetTypename(Oid typid); /* Concatenate multiple literal strings using stringinfo */ char *concat(int num_args,...); +/* Get protocol for the PXF server URL */ +const char *get_pxf_protocol(void); + /* Get authority (host:port) for the PXF server URL */ char *get_authority(void); @@ -25,17 +28,38 @@ const char *get_pxf_host(void); */ const int get_pxf_port(void); +const char *get_pxf_ssl_keypasswd(void); +const char *get_pxf_ssl_cacert(void); +const char *get_pxf_ssl_cert(void); +const char *get_pxf_ssl_key(void); +const char *get_pxf_ssl_certtype(void); +long get_pxf_ssl_verifypeer(void); + /* Returns the namespace (schema) name for a given namespace oid */ char *GetNamespaceName(Oid nsp_oid); -#define PXF_PROFILE "PROFILE" -#define FRAGMENTER "FRAGMENTER" -#define ACCESSOR "ACCESSOR" -#define RESOLVER "RESOLVER" -#define ANALYZER "ANALYZER" -#define ENV_PXF_HOST "PXF_HOST" -#define ENV_PXF_PORT "PXF_PORT" -#define PXF_DEFAULT_HOST "localhost" -#define PXF_DEFAULT_PORT 5888 +#define PXF_PROFILE "PROFILE" +#define FRAGMENTER "FRAGMENTER" +#define ACCESSOR "ACCESSOR" +#define RESOLVER "RESOLVER" +#define ANALYZER "ANALYZER" +#define ENV_PXF_HOST "PXF_HOST" +#define ENV_PXF_PORT "PXF_PORT" +#define ENV_PXF_PROTOCOL "PXF_PROTOCOL" +#define ENV_PXF_SSL_CACERT "PXF_SSL_CACERT" +#define ENV_PXF_SSL_CERT "PXF_SSL_CERT" +#define ENV_PXF_SSL_CERT_TYPE "PXF_SSL_CERT_TYPE" +#define ENV_PXF_SSL_KEY "PXF_SSL_KEY" +#define ENV_PXF_SSL_KEYPASSWD "PXF_SSL_KEYPASSWD" +#define ENV_PXF_SSL_VERIFY_PEER "PXF_SSL_VERIFY_PEER" +#define PXF_DEFAULT_HOST "localhost" +#define PXF_DEFAULT_PORT 5888 +#define PXF_DEFAULT_PROTOCOL "http" +#define PXF_DEFAULT_SSL_CACERT "/home/gpadmin/arenadata_configs/cacert.pem" +#define PXF_DEFAULT_SSL_CERT "client.pem" +#define PXF_DEFAULT_SSL_CERT_TYPE "pem" +#define PXF_DEFAULT_SSL_KEY "" +#define PXF_DEFAULT_SSL_KEYPASSWD "" +#define PXF_DEFAULT_SSL_VERIFY_PEER 1 #endif /* _PXFUTILS_H_ */ From 3f89be8d80ce8ca227feff4a8056a5c05097a0cf Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Thu, 9 Jan 2025 08:48:22 +0300 Subject: [PATCH 10/14] Whitespace changes --- fdw/libchurl.h | 6 +++--- fdw/pxf_bridge.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fdw/libchurl.h b/fdw/libchurl.h index cd1960ec54..f8d756d209 100644 --- a/fdw/libchurl.h +++ b/fdw/libchurl.h @@ -116,9 +116,9 @@ void churl_headers_cleanup(CHURL_HEADERS headers); * Start an upload to url * returns a handle to churl transfer */ -CHURL_HANDLE churl_init_upload(const char *url, CHURL_HEADERS headers, +CHURL_HANDLE churl_init_upload(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options); -CHURL_HANDLE churl_init_upload_timeout(const char *url, CHURL_HEADERS headers, +CHURL_HANDLE churl_init_upload_timeout(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options, long timeout); /* @@ -130,7 +130,7 @@ int churl_get_local_port(CHURL_HANDLE handle); * Start a download to url * returns a handle to churl transfer */ -CHURL_HANDLE churl_init_download(const char *url, CHURL_HEADERS headers, +CHURL_HANDLE churl_init_download(const char *url, CHURL_HEADERS headers, churl_ssl_options *ssl_options); /* diff --git a/fdw/pxf_bridge.c b/fdw/pxf_bridge.c index ec75de4e8c..017295d00e 100644 --- a/fdw/pxf_bridge.c +++ b/fdw/pxf_bridge.c @@ -262,7 +262,7 @@ PxfBridgeImportStart(PxfFdwScanState *pxfsstate) if (ssl_options != NULL) { /* make another copy for transaction context */ pxfcstate->ssl_options = churl_make_ssl_options(pxfsstate->options); - } + } MemoryContextSwitchTo(oldcontext); pxfsstate->pxfcstate = pxfcstate; pxfcstate->churl_headers = pxfsstate->churl_headers; From 5ec9b9d3e1cc280fc54d2b20ee4f4f82f28679fd Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Fri, 10 Jan 2025 08:41:47 +0300 Subject: [PATCH 11/14] remove extra tabs --- fdw/libchurl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdw/libchurl.c b/fdw/libchurl.c index ca87ef8818..e86c9fd65e 100644 --- a/fdw/libchurl.c +++ b/fdw/libchurl.c @@ -450,7 +450,7 @@ set_curl_ssl_options(churl_context *context, churl_ssl_options *ssl_options) set_curl_option(context, CURLOPT_SSLKEYPASSWD, ssl_options->pxf_ssl_keypasswd); } - if (cacert != NULL && cacert[0] != '\0') { + if (cacert != NULL && cacert[0] != '\0') { set_curl_option(context, CURLOPT_CAINFO, cacert); } From f0a5d5bbe06a1094de735d2ba8cc7e83dbcbb4da Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Wed, 15 Jan 2025 10:43:15 +0500 Subject: [PATCH 12/14] Implement FDW SSL automated test in container --- arenadata/Dockerfile | 32 +++++++++++++++ arenadata/test_in_docker.sh | 11 +++++ dev/set_up_gpadmin_user.bash | 8 ++++ fdw/Makefile | 5 ++- fdw/expected/pxf_fdw_ssl.out | 78 ++++++++++++++++++++++++++++++++++++ fdw/sql/pxf_fdw_ssl.sql | 74 ++++++++++++++++++++++++++++++++++ 6 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 fdw/expected/pxf_fdw_ssl.out create mode 100644 fdw/sql/pxf_fdw_ssl.sql diff --git a/arenadata/Dockerfile b/arenadata/Dockerfile index 062dabb9a3..d10004bf84 100644 --- a/arenadata/Dockerfile +++ b/arenadata/Dockerfile @@ -66,5 +66,37 @@ RUN source gpdb_src/concourse/scripts/common.bash && \ # create test image which prepares base image and keeps only pxf artifacts from build image FROM base as test +ENV GPHOME=/usr/local/greenplum-db-devel +ENV PXF_HOME=/usr/local/greenplum-db-devel/pxf + COPY --from=build /tmp/build/${OUTPUT_ARTIFACT_DIR}/pxf.tar.gz /tmp/build/${OUTPUT_ARTIFACT_DIR}/ COPY --from=build /tmp/build/pxf_src /tmp/build/pxf_src + +COPY --from=build /tmp/build/pxf_src/automation/arenadata/conf/ssl/certs /opt/ssl/certs + +RUN mkdir -p ${GPHOME} && tar -xzf /tmp/build/${OUTPUT_ARTIFACT_DIR}/pxf.tar.gz -C ${GPHOME} + +# Copy pxf-application.properties +COPY --from=build /tmp/build/pxf_src/automation/arenadata/conf/pxf-application.properties ${PXF_HOME}/conf/pxf-application.properties + +ENV HOSTNAME=mdw +ENV DOCKER_GP_CLUSTER_HOSTS=mdw,sdw1,sdw2 +ENV DOCKER_GP_MASTER_SERVER=mdw +ENV DOCKER_GP_SEGMENT_SERVERS=sdw1,sdw2 +ENV DOCKER_GP_PRIMARY_SEGMENTS_PER_HOST=3 +ENV DOCKER_GP_WITH_MIRROR=false +ENV PXF_PROTOCOL=https +ENV PXF_HOST=mdw +ENV PXF_PORT=5888 +ENV PXF_VAULT_ENABLED=true +ENV PXF_VAULT_SECRET_PATH=adb/adb-it/service/pxf +ENV PXF_SSL_ENABLED=true +ENV PXF_SSL_KEY_STORE_PATH=/opt/ssl/certs/pxf.jks +ENV PXF_SSL_TRUST_STORE_PATH=/opt/ssl/certs/pxf.jks +ENV PXF_SSL_CLIENT_AUTH=NEED +ENV PXF_SSL_CACERT=/opt/ssl/certs/ca-cert +ENV PXF_SSL_CERT=/opt/ssl/certs/pxf-client.pem +ENV PXF_SSL_KEY=/opt/ssl/certs/pxf-client.key +ENV PXF_SSL_CERT_TYPE=PEMCOPY +ENV PXF_SSL_KEY_STORE_PASSWORD=123456 +ENV PXF_SSL_TRUST_STORE_PASSWORD=123456 diff --git a/arenadata/test_in_docker.sh b/arenadata/test_in_docker.sh index eb0a90e868..86c876376a 100755 --- a/arenadata/test_in_docker.sh +++ b/arenadata/test_in_docker.sh @@ -4,6 +4,17 @@ set -exo pipefail # manually prepare gpadmin user; test_pxf.bash doesn't tweak gpadmin folder permissions and ssh keys ./gpdb_src/concourse/scripts/setup_gpadmin_user.bash + +hostname mdw + +if [[ "$PXF_PROTOCOL" = "https" ]]; then + echo "--------------------------------------" + echo "Init SSL env variables for PXF service" + echo "--------------------------------------" + env | grep -E 'PXF_SSL|PXF_HOST|PXF_PROTOCOL' | sed 's/^/export /' >> /home/gpadmin/.bash_profile + env | grep -E 'PXF_SSL|PXF_HOST|PXF_PROTOCOL' | sed 's/^/export /' >> /home/gpadmin/.bashrc +fi + # unpack gpdb and pxf; run gpdb cluster and pxf server /tmp/build/pxf_src/concourse/scripts/test_pxf.bash # tweak necessary folders to run regression tests later diff --git a/dev/set_up_gpadmin_user.bash b/dev/set_up_gpadmin_user.bash index 8d27a10710..487caa64eb 100755 --- a/dev/set_up_gpadmin_user.bash +++ b/dev/set_up_gpadmin_user.bash @@ -23,3 +23,11 @@ export SLAVES=1 export GOPATH=/opt/go export PATH=\${PXF_HOME}/bin:\${GPHD_ROOT}/hadoop/bin:\${GOPATH}/bin:/usr/local/go/bin:\$PATH EOF + +if [[ "$PXF_PROTOCOL" = "https" ]]; then + echo "--------------------------------------" + echo "Init SSL env variables for PXF service" + echo "--------------------------------------" + env | grep -E 'PXF_SSL|PXF_HOST|PXF_PROTOCOL' | sed 's/^/export /' >> /home/gpadmin/.bash_profile + env | grep -E 'PXF_SSL|PXF_HOST|PXF_PROTOCOL' | sed 's/^/export /' >> /home/gpadmin/.bashrc +fi diff --git a/fdw/Makefile b/fdw/Makefile index 8ea1b6c669..03644e5d05 100644 --- a/fdw/Makefile +++ b/fdw/Makefile @@ -2,7 +2,10 @@ EXTENSION = pxf_fdw DATA = pxf_fdw--1.0.sql MODULE_big = pxf_fdw OBJS = pxf_fdw.o pxf_bridge.o pxf_deparse.o pxf_filter.o pxf_header.o pxf_option.o libchurl.o -REGRESS = pxf_fdw_wrapper pxf_fdw_server pxf_fdw_user_mapping pxf_fdw_foreign_table + +is_ssl_supported :=$(filter https,$(PXF_PROTOCOL)) +REGRESS_SSL := $(if $(is_ssl_supported), pxf_fdw_ssl, ) +REGRESS = pxf_fdw_wrapper pxf_fdw_server pxf_fdw_user_mapping pxf_fdw_foreign_table $(REGRESS_SSL) SHLIB_LINK += -lcurl PXF_API_VERSION := $(shell cat ../api_version) diff --git a/fdw/expected/pxf_fdw_ssl.out b/fdw/expected/pxf_fdw_ssl.out new file mode 100644 index 0000000000..595d9d5fcd --- /dev/null +++ b/fdw/expected/pxf_fdw_ssl.out @@ -0,0 +1,78 @@ +-- =================================================================== +-- Validation for SSL options +-- =================================================================== +-- This is expected to be run in the container +-- +CREATE FOREIGN DATA WRAPPER pxf_filter_push_down_fdw_ssl + HANDLER pxf_fdw_handler + VALIDATOR pxf_fdw_validator + OPTIONS (protocol 'system', mpp_execute 'all segments', + pxf_protocol 'https', + pxf_ssl_cacert '/opt/ssl/certs/ca-cert', + pxf_ssl_cert '/opt/ssl/certs/pxf-client.pem', + pxf_ssl_cert_type 'PEM', + pxf_ssl_key '/opt/ssl/certs/pxf-client.key' + ); +CREATE SERVER pxf_filter_push_down_server_ssl + FOREIGN DATA WRAPPER pxf_filter_push_down_fdw_ssl; +CREATE USER MAPPING FOR CURRENT_USER SERVER pxf_filter_push_down_server_ssl; +CREATE FOREIGN TABLE test_filter ( + t0 text, + a1 integer, + b2 boolean, + c3 numeric, + d4 char(2), + e5 varchar(2), + x1 bpchar(2), + x2 smallint, + x3 bigint, + x4 real, + x5 float8, + x6 bytea, + x7 date, + x8 time, + x9 timestamp, + x10 timestamp with time zone, + x11 interval, + x12 uuid, + x13 json, + x14 jsonb, + x15 int2[], + x16 int4[], + x17 int8[], + x18 bool[], + x19 text[], + x20 float4[], + x21 float8[], + x22 bytea[], + x23 bpchar[], + x24 varchar(2)[], + x25 date[], + x26 uuid[], + x27 numeric[], + x28 time[], + x29 timestamp[], + x30 timestamp with time zone[], + x31 interval[], + x32 json[], + x33 jsonb[], + filterValue text) + SERVER pxf_filter_push_down_server_ssl + OPTIONS (resource 'dummy_path', format 'filter', delimiter ','); +SELECT t0, a1 FROM test_filter; + t0 | a1 +----+---- + | 0 + B | + C | 2 + D | 3 + E | 4 + F | 5 + G | 6 + H | 7 + I | 8 + J | 9 +(10 rows) + +ALTER FOREIGN DATA WRAPPER pxf_filter_push_down_fdw_ssl + OPTIONS ( ADD pxf_ssl_verify_peer 'false' ); diff --git a/fdw/sql/pxf_fdw_ssl.sql b/fdw/sql/pxf_fdw_ssl.sql new file mode 100644 index 0000000000..5e171a7807 --- /dev/null +++ b/fdw/sql/pxf_fdw_ssl.sql @@ -0,0 +1,74 @@ +-- =================================================================== +-- Validation for SSL options +-- =================================================================== +-- This is expected to be run in the container +-- + +CREATE FOREIGN DATA WRAPPER pxf_filter_push_down_fdw_ssl + HANDLER pxf_fdw_handler + VALIDATOR pxf_fdw_validator + OPTIONS (protocol 'system', mpp_execute 'all segments', + pxf_protocol 'https', + pxf_ssl_cacert '/opt/ssl/certs/ca-cert', + pxf_ssl_cert '/opt/ssl/certs/pxf-client.pem', + pxf_ssl_cert_type 'PEM', + pxf_ssl_key '/opt/ssl/certs/pxf-client.key' + ); + +CREATE SERVER pxf_filter_push_down_server_ssl + FOREIGN DATA WRAPPER pxf_filter_push_down_fdw_ssl; + +CREATE USER MAPPING FOR CURRENT_USER SERVER pxf_filter_push_down_server_ssl; + +CREATE FOREIGN TABLE test_filter ( + t0 text, + a1 integer, + b2 boolean, + c3 numeric, + d4 char(2), + e5 varchar(2), + x1 bpchar(2), + x2 smallint, + x3 bigint, + x4 real, + x5 float8, + x6 bytea, + x7 date, + x8 time, + x9 timestamp, + x10 timestamp with time zone, + x11 interval, + x12 uuid, + x13 json, + x14 jsonb, + x15 int2[], + x16 int4[], + x17 int8[], + x18 bool[], + x19 text[], + x20 float4[], + x21 float8[], + x22 bytea[], + x23 bpchar[], + x24 varchar(2)[], + x25 date[], + x26 uuid[], + x27 numeric[], + x28 time[], + x29 timestamp[], + x30 timestamp with time zone[], + x31 interval[], + x32 json[], + x33 jsonb[], + filterValue text) + SERVER pxf_filter_push_down_server_ssl + OPTIONS (resource 'dummy_path', format 'filter', delimiter ','); + +SELECT t0, a1 FROM test_filter; + + +ALTER FOREIGN DATA WRAPPER pxf_filter_push_down_fdw_ssl + OPTIONS ( ADD pxf_ssl_verify_peer 'false' ); + + + From d638c692a7082fda367fe1586e6fdafe715a43ed Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Thu, 16 Jan 2025 00:15:02 +0500 Subject: [PATCH 13/14] Add writable test --- arenadata/test_in_docker.sh | 7 +++++++ fdw/expected/pxf_fdw_ssl.out | 28 ++++++++++++++++++++++++++++ fdw/sql/pxf_fdw_ssl.sql | 28 ++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+) diff --git a/arenadata/test_in_docker.sh b/arenadata/test_in_docker.sh index 86c876376a..b91db81f91 100755 --- a/arenadata/test_in_docker.sh +++ b/arenadata/test_in_docker.sh @@ -7,6 +7,7 @@ set -exo pipefail hostname mdw +# Pass throw PXF environment variables to gpadmin user if [[ "$PXF_PROTOCOL" = "https" ]]; then echo "--------------------------------------" echo "Init SSL env variables for PXF service" @@ -17,6 +18,12 @@ fi # unpack gpdb and pxf; run gpdb cluster and pxf server /tmp/build/pxf_src/concourse/scripts/test_pxf.bash + +# Enable basePath in the PXF server to allow writable tables test +sed -i \ + -e 's||pxf.fs.basePath/tmp/pxf/|g' \ + ${PXF_HOME}/servers/default/pxf-site.xml + # tweak necessary folders to run regression tests later chown gpadmin:gpadmin -R /usr/local/greenplum-db-devel chown gpadmin:gpadmin -R /tmp/build/pxf_src diff --git a/fdw/expected/pxf_fdw_ssl.out b/fdw/expected/pxf_fdw_ssl.out index 595d9d5fcd..be3b8c4e60 100644 --- a/fdw/expected/pxf_fdw_ssl.out +++ b/fdw/expected/pxf_fdw_ssl.out @@ -76,3 +76,31 @@ SELECT t0, a1 FROM test_filter; ALTER FOREIGN DATA WRAPPER pxf_filter_push_down_fdw_ssl OPTIONS ( ADD pxf_ssl_verify_peer 'false' ); +-- Check writable table +CREATE FOREIGN DATA WRAPPER file_pxf_fdw_ssl + HANDLER pxf_fdw_handler + VALIDATOR pxf_fdw_validator + OPTIONS (protocol 'file', mpp_execute 'all segments', + pxf_protocol 'https', + pxf_ssl_cacert '/opt/ssl/certs/ca-cert', + pxf_ssl_cert '/opt/ssl/certs/pxf-client.pem', + pxf_ssl_cert_type 'PEM', + pxf_ssl_key '/opt/ssl/certs/pxf-client.key' +); +CREATE SERVER pxf_file_server_ssl + FOREIGN DATA WRAPPER file_pxf_fdw_ssl OPTIONS( + config 'default'); +CREATE USER MAPPING FOR CURRENT_USER SERVER pxf_file_server_ssl; +CREATE FOREIGN TABLE test_file2 ( + id int, + descr text +) +SERVER pxf_file_server_ssl +OPTIONS (resource 'fdw_file', format 'csv', delimiter ','); +INSERT INTO test_file2 SELECT generate_series(1,10) AS id, md5(random()::text) AS descr; +WARNING: skipping "test_file2" --- cannot analyze this foreign table +\! ls -1s /tmp/pxf/fdw_file +total 12 +4 43-0000000022_0 +4 43-0000000022_1 +4 43-0000000022_2 diff --git a/fdw/sql/pxf_fdw_ssl.sql b/fdw/sql/pxf_fdw_ssl.sql index 5e171a7807..ee4b5c1302 100644 --- a/fdw/sql/pxf_fdw_ssl.sql +++ b/fdw/sql/pxf_fdw_ssl.sql @@ -71,4 +71,32 @@ ALTER FOREIGN DATA WRAPPER pxf_filter_push_down_fdw_ssl OPTIONS ( ADD pxf_ssl_verify_peer 'false' ); +-- Check writable table +CREATE FOREIGN DATA WRAPPER file_pxf_fdw_ssl + HANDLER pxf_fdw_handler + VALIDATOR pxf_fdw_validator + OPTIONS (protocol 'file', mpp_execute 'all segments', + pxf_protocol 'https', + pxf_ssl_cacert '/opt/ssl/certs/ca-cert', + pxf_ssl_cert '/opt/ssl/certs/pxf-client.pem', + pxf_ssl_cert_type 'PEM', + pxf_ssl_key '/opt/ssl/certs/pxf-client.key' +); + +CREATE SERVER pxf_file_server_ssl + FOREIGN DATA WRAPPER file_pxf_fdw_ssl OPTIONS( + config 'default'); + +CREATE USER MAPPING FOR CURRENT_USER SERVER pxf_file_server_ssl; + +CREATE FOREIGN TABLE test_file2 ( + id int, + descr text +) +SERVER pxf_file_server_ssl +OPTIONS (resource 'fdw_file', format 'csv', delimiter ','); + +INSERT INTO test_file2 SELECT generate_series(1,10) AS id, md5(random()::text) AS descr; + +\! ls -1s /tmp/pxf/fdw_file From 54981e43c81691bd30785f39d26274a9e45e467c Mon Sep 17 00:00:00 2001 From: Denis Kovalev Date: Thu, 16 Jan 2025 11:27:34 +0500 Subject: [PATCH 14/14] Move SSL related testing to separated container --- arenadata/Dockerfile | 32 ----------- arenadata/Dockerfile_ssl | 102 ++++++++++++++++++++++++++++++++++++ arenadata/README.md | 13 +++++ arenadata/test_in_docker.sh | 10 ++-- 4 files changed, 121 insertions(+), 36 deletions(-) create mode 100644 arenadata/Dockerfile_ssl diff --git a/arenadata/Dockerfile b/arenadata/Dockerfile index d10004bf84..062dabb9a3 100644 --- a/arenadata/Dockerfile +++ b/arenadata/Dockerfile @@ -66,37 +66,5 @@ RUN source gpdb_src/concourse/scripts/common.bash && \ # create test image which prepares base image and keeps only pxf artifacts from build image FROM base as test -ENV GPHOME=/usr/local/greenplum-db-devel -ENV PXF_HOME=/usr/local/greenplum-db-devel/pxf - COPY --from=build /tmp/build/${OUTPUT_ARTIFACT_DIR}/pxf.tar.gz /tmp/build/${OUTPUT_ARTIFACT_DIR}/ COPY --from=build /tmp/build/pxf_src /tmp/build/pxf_src - -COPY --from=build /tmp/build/pxf_src/automation/arenadata/conf/ssl/certs /opt/ssl/certs - -RUN mkdir -p ${GPHOME} && tar -xzf /tmp/build/${OUTPUT_ARTIFACT_DIR}/pxf.tar.gz -C ${GPHOME} - -# Copy pxf-application.properties -COPY --from=build /tmp/build/pxf_src/automation/arenadata/conf/pxf-application.properties ${PXF_HOME}/conf/pxf-application.properties - -ENV HOSTNAME=mdw -ENV DOCKER_GP_CLUSTER_HOSTS=mdw,sdw1,sdw2 -ENV DOCKER_GP_MASTER_SERVER=mdw -ENV DOCKER_GP_SEGMENT_SERVERS=sdw1,sdw2 -ENV DOCKER_GP_PRIMARY_SEGMENTS_PER_HOST=3 -ENV DOCKER_GP_WITH_MIRROR=false -ENV PXF_PROTOCOL=https -ENV PXF_HOST=mdw -ENV PXF_PORT=5888 -ENV PXF_VAULT_ENABLED=true -ENV PXF_VAULT_SECRET_PATH=adb/adb-it/service/pxf -ENV PXF_SSL_ENABLED=true -ENV PXF_SSL_KEY_STORE_PATH=/opt/ssl/certs/pxf.jks -ENV PXF_SSL_TRUST_STORE_PATH=/opt/ssl/certs/pxf.jks -ENV PXF_SSL_CLIENT_AUTH=NEED -ENV PXF_SSL_CACERT=/opt/ssl/certs/ca-cert -ENV PXF_SSL_CERT=/opt/ssl/certs/pxf-client.pem -ENV PXF_SSL_KEY=/opt/ssl/certs/pxf-client.key -ENV PXF_SSL_CERT_TYPE=PEMCOPY -ENV PXF_SSL_KEY_STORE_PASSWORD=123456 -ENV PXF_SSL_TRUST_STORE_PASSWORD=123456 diff --git a/arenadata/Dockerfile_ssl b/arenadata/Dockerfile_ssl new file mode 100644 index 0000000000..d10004bf84 --- /dev/null +++ b/arenadata/Dockerfile_ssl @@ -0,0 +1,102 @@ +ARG GPDB_IMAGE=hub.adsw.io/library/gpdb6_regress:adb-6.x-dev +FROM $GPDB_IMAGE as base + +# install go, ginkgo and keep env variables which may be used as a part of base image +RUN set -eux; \ + ARCH="$(uname -m)"; \ + case "${ARCH}" in \ + aarch64|armhf|armv7l|amd64|x86_64) \ + wget https://go.dev/dl/go1.21.3.linux-amd64.tar.gz \ + && rm -rf /usr/local/go && tar -C /usr/local -xzf go1.21.3.linux-amd64.tar.gz && rm go1.21.3.linux-amd64.tar.gz \ + ;; \ + arm64) \ + wget https://go.dev/dl/go1.21.3.linux-arm64.tar.gz \ + && rm -rf /usr/local/go && tar -C /usr/local -xzf go1.21.3.linux-arm64.tar.gz && rm go1.21.3.linux-arm64.tar.gz \ + ;; \ + ppc64el|ppc64le) \ + wget https://go.dev/dl/go1.21.3.linux-ppc64le.tar.gz \ + && rm -rf /usr/local/go && tar -C /usr/local -xzf go1.21.3.linux-ppc64le.tar.gz && rm go1.21.3.linux-ppc64le.tar.gz \ + ;; \ + *) \ + echo "Unsupported arch: ${ARCH}"; \ + exit 1; \ + ;; \ + esac; \ + . /etc/os-release; \ + case "$ID" in \ + centos*) \ + yum-config-manager --disable epel && yum-config-manager --add-repo 'http://archives.fedoraproject.org/pub/archive/epel/7/$basearch'; \ + curl -fSL https://download.oracle.com/java/17/archive/jdk-17.0.12_linux-x64_bin.rpm -o /tmp/jdk-17.0.12_linux-x64_bin.rpm \ + && yum -y install /tmp/jdk-17.0.12_linux-x64_bin.rpm \ + && rm -rf /tmp/jdk-17.0.12_linux-x64_bin.rpm; \ + sed -i "s/JAVA_HOME=.*/JAVA_HOME=\$(readlink -f \/usr\/bin\/java | sed 's:bin\/java::')/g" /etc/profile.d/jdk_home.sh; \ + ;; \ + ubuntu*) \ + apt-get install -y openjdk-17-jdk; \ + ;; \ + esac; +ENV GOPATH=$HOME/go +ENV PATH=$PATH:/usr/local/go/bin:$GOPATH/bin +RUN go install github.com/onsi/ginkgo/ginkgo@latest \ + && go install github.com/onsi/ginkgo/v2/ginkgo@latest + +# leave pxf artifacts dir env also +ENV OUTPUT_ARTIFACT_DIR="pxf_tarball" + +# remove unnecessary artifacts and create symlinks +# concource scripts expects gpdb and pxf placed in the same folder +RUN rm /home/gpadmin/bin_gpdb/server-*.tar.gz && \ + mkdir /tmp/build && \ + ln -s /home/gpadmin/gpdb_src /tmp/build/gpdb_src && \ + ln -s /home/gpadmin/bin_gpdb /tmp/build/bin_gpdb +# default working dir - the place where all sources and artifacts are placed +WORKDIR /tmp/build + +# create separate image with files we don't want to keep in base image +FROM base as build +COPY . /tmp/build/pxf_src +ENV JAVA_TOOL_OPTIONS=-Dfile.encoding=UTF8 +SHELL ["bash", "-c"] +RUN source gpdb_src/concourse/scripts/common.bash && \ + install_gpdb && \ + source '/usr/local/greenplum-db-devel/greenplum_path.sh' && \ + mkdir ${OUTPUT_ARTIFACT_DIR} && \ + export SKIP_FDW_BUILD_REASON=0 && \ + pxf_src/concourse/scripts/compile_pxf.bash + +# create test image which prepares base image and keeps only pxf artifacts from build image +FROM base as test +ENV GPHOME=/usr/local/greenplum-db-devel +ENV PXF_HOME=/usr/local/greenplum-db-devel/pxf + +COPY --from=build /tmp/build/${OUTPUT_ARTIFACT_DIR}/pxf.tar.gz /tmp/build/${OUTPUT_ARTIFACT_DIR}/ +COPY --from=build /tmp/build/pxf_src /tmp/build/pxf_src + +COPY --from=build /tmp/build/pxf_src/automation/arenadata/conf/ssl/certs /opt/ssl/certs + +RUN mkdir -p ${GPHOME} && tar -xzf /tmp/build/${OUTPUT_ARTIFACT_DIR}/pxf.tar.gz -C ${GPHOME} + +# Copy pxf-application.properties +COPY --from=build /tmp/build/pxf_src/automation/arenadata/conf/pxf-application.properties ${PXF_HOME}/conf/pxf-application.properties + +ENV HOSTNAME=mdw +ENV DOCKER_GP_CLUSTER_HOSTS=mdw,sdw1,sdw2 +ENV DOCKER_GP_MASTER_SERVER=mdw +ENV DOCKER_GP_SEGMENT_SERVERS=sdw1,sdw2 +ENV DOCKER_GP_PRIMARY_SEGMENTS_PER_HOST=3 +ENV DOCKER_GP_WITH_MIRROR=false +ENV PXF_PROTOCOL=https +ENV PXF_HOST=mdw +ENV PXF_PORT=5888 +ENV PXF_VAULT_ENABLED=true +ENV PXF_VAULT_SECRET_PATH=adb/adb-it/service/pxf +ENV PXF_SSL_ENABLED=true +ENV PXF_SSL_KEY_STORE_PATH=/opt/ssl/certs/pxf.jks +ENV PXF_SSL_TRUST_STORE_PATH=/opt/ssl/certs/pxf.jks +ENV PXF_SSL_CLIENT_AUTH=NEED +ENV PXF_SSL_CACERT=/opt/ssl/certs/ca-cert +ENV PXF_SSL_CERT=/opt/ssl/certs/pxf-client.pem +ENV PXF_SSL_KEY=/opt/ssl/certs/pxf-client.key +ENV PXF_SSL_CERT_TYPE=PEMCOPY +ENV PXF_SSL_KEY_STORE_PASSWORD=123456 +ENV PXF_SSL_TRUST_STORE_PASSWORD=123456 diff --git a/arenadata/README.md b/arenadata/README.md index 067d25bb4b..0385419491 100644 --- a/arenadata/README.md +++ b/arenadata/README.md @@ -17,3 +17,16 @@ docker run --rm -it \ --privileged --sysctl kernel.sem="500 1024000 200 4096" \ gpdb6_pxf_regress:latest /tmp/build/pxf_src/arenadata/test_in_docker.sh ``` + +## How to test PXF with TLS support +To test TLS with TLS we build PXF with Dockerfile_ssl wich has PXF set up with SSL support: +```bash +docker build -t gpdb6_pxf_regress_ssl:latest -f arenadata/Dockerfile_ssl . +``` + +To additionally test `fdw` and `external-table` parts you may call: +```bash +docker run --rm -it \ + --privileged --sysctl kernel.sem="500 1024000 200 4096" \ + gpdb6_pxf_regress_ssl:latest /tmp/build/pxf_src/arenadata/test_in_docker.sh +``` diff --git a/arenadata/test_in_docker.sh b/arenadata/test_in_docker.sh index b91db81f91..16260764ef 100755 --- a/arenadata/test_in_docker.sh +++ b/arenadata/test_in_docker.sh @@ -19,10 +19,12 @@ fi # unpack gpdb and pxf; run gpdb cluster and pxf server /tmp/build/pxf_src/concourse/scripts/test_pxf.bash -# Enable basePath in the PXF server to allow writable tables test -sed -i \ - -e 's||pxf.fs.basePath/tmp/pxf/|g' \ - ${PXF_HOME}/servers/default/pxf-site.xml +# Enable basePath in the PXF server to allow writable tables SSL test +if [[ "$PXF_PROTOCOL" = "https" ]]; then + sed -i \ + -e 's||pxf.fs.basePath/tmp/pxf/|g' \ + ${PXF_HOME}/servers/default/pxf-site.xml +fi # tweak necessary folders to run regression tests later chown gpadmin:gpadmin -R /usr/local/greenplum-db-devel