Re: [HACKERS] Better name for PQsslAttributes()
Thank you for the quick response! Attached the last minute - now or never patch to change the function name. In addition I perceived a small inconsistency with the naming of the SGML id of PQsslAttribute. This is addressed in the second patch file. -- Kind Regards, Lars From 5b7f116de237df5404938fcb3a722c071d0ee120 Mon Sep 17 00:00:00 2001 From: Lars Kanis <l...@greiz-reinsdorf.de> Date: Sat, 7 Nov 2015 21:00:21 +0100 Subject: [PATCH] Lowercase pqsslAttribute in SGML-id for consistency. --- doc/src/sgml/libpq.sgml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 0ee018e..fdb06c3 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1873,7 +1873,7 @@ int PQsslInUse(const PGconn *conn); - + PQsslAttributePQsslAttribute -- 2.1.4 From 2d86b4d785bb5b53c779b79a8010e7d7f6cbb9b5 Mon Sep 17 00:00:00 2001 From: Lars Kanis <l...@greiz-reinsdorf.de> Date: Sat, 7 Nov 2015 20:42:43 +0100 Subject: [PATCH] Rename PQsslAttributes() to PQsslAttributeNames(). The name PQsslAttributes suggested, the function would return attribute values or key/value pairs, but it returns keys only. PQsslAttributeNames makes more clear, what the function is about. --- doc/src/sgml/libpq.sgml | 6 +++--- src/interfaces/libpq/exports.txt | 2 +- src/interfaces/libpq/fe-secure-openssl.c | 2 +- src/interfaces/libpq/fe-secure.c | 2 +- src/interfaces/libpq/libpq-fe.h | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 0ee018e..7c31f42 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1947,13 +1947,13 @@ const char *PQsslAttribute(const PGconn *conn, const char *attribute_name); - - PQsslAttributesPQsslAttributes + + PQsslAttributeNamesPQsslAttributeNames Return an array of SSL attribute names available. The array is terminated by a NULL pointer. -const char **PQsslAttributes(const PGconn *conn); +const char **PQsslAttributeNames(const PGconn *conn); diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt index 0bbcae5..c69a4d5 100644 --- a/src/interfaces/libpq/exports.txt +++ b/src/interfaces/libpq/exports.txt @@ -167,6 +167,6 @@ lo_truncate64 164 PQconninfo165 PQsslInUse166 PQsslStruct 167 -PQsslAttributes 168 +PQsslAttributeNames 168 PQsslAttribute169 PQsetErrorContextVisibility 170 diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index 4b2a324..e88a7ee 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -1533,7 +1533,7 @@ PQsslStruct(PGconn *conn, const char *struct_name) } const char ** -PQsslAttributes(PGconn *conn) +PQsslAttributeNames(PGconn *conn) { static const char *result[] = { "library", diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index db91e52..ed1342e 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -409,7 +409,7 @@ PQsslAttribute(PGconn *conn, const char *attribute_name) } const char ** -PQsslAttributes(PGconn *conn) +PQsslAttributeNames(PGconn *conn) { static const char *result[] = {NULL}; diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index 828c533..eddb817 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -329,7 +329,7 @@ extern int PQsetClientEncoding(PGconn *conn, const char *encoding); extern int PQsslInUse(PGconn *conn); extern void *PQsslStruct(PGconn *conn, const char *struct_name); extern const char *PQsslAttribute(PGconn *conn, const char *attribute_name); -extern const char **PQsslAttributes(PGconn *conn); +extern const char **PQsslAttributeNames(PGconn *conn); /* Get the OpenSSL structure associated with a connection. Returns NULL for * unencrypted connections or if any other TLS library is in use. */ -- 2.1.4 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Better name for PQsslAttributes()
Am 07.11.2015 um 22:14 schrieb Tom Lane: > Pushed. I took the opportunity to fix the const-ness annotation of the > function result type, too. Great, thank you! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Better name for PQsslAttributes()
As a co-maintainer of the PostgreSQL adapter for Ruby, I would like to bridge the new SSL related functions to Ruby methods. However I wonder whether PQsslAttributes() is the best name for the function. Based on this name, I would expect to get key+value pairs instead of only the keys. IMHO PQsslAttributeNames() would express better, what the function does. -- Kind Regards, Lars -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Failing SSL connection due to weird interaction with openssl
Am 06.11.2012 21:40, schrieb Robert Haas: On Tue, Oct 23, 2012 at 4:09 AM, Lars Kanis l...@greiz-reinsdorf.de wrote: While investigating a ruby-pg issue [1], we noticed that a libpq SSL connection can fail, if the running application uses OpenSSL for other work, too. Root cause is the thread local error queue of OpenSSL, that is used to transmit textual error messages to the application after a failed crypto operation. In case that the application leaves errors on the queue, the communication to the PostgreSQL server can fail with a message left from the previous failed OpenSSL operation, in particular when using non-blocking operations on the socket. This issue with openssl is quite old now - see [3]. For [1] it turned out that the issue is subdivided into these three parts: 1. the ruby-openssl binding does not clear the thread local error queue of OpenSSL after a certificate verify 2. OpenSSL makes use of a shared error queue for different crypto contexts. 3. libpq does not ensure a cleared error queue when doing SSL_* calls To 1: Remaining messages on the error queue can generally lead to failing operations, later on. I'd talk to the ruby-openssl developers, to discuss how we can avoid any remaining messages on the queue. To 2: SSL_get_error() inspects the shared error queue under some conditions. It's maybe poor API design, but it's documented behaviour [2]. So we certainly have to get along with it. To 3: To make libpq independent to a previous error state, the error queue might be cleared with a call to ERR_clear_error() prior SSL_connect/read/write as in the attached trivial patch. This would make libpq robust against other uses of openssl within the application. What do you think about clearing the OpenSSL error queue in libpq in that way? Shouldn't it be the job of whatever code is consuming the error to clear the error queue afterwards? Yes, of course. I already filed a bug for ruby-openssl, some weeks ago [1]. But IMHO libpq should be changed too, for the following reasons: 1. The behavior of libpq isn't consistent, since blocking calls are already agnostic to remaining errors in the openssl queue, but non-blocking are not. This is a openssl quirk, that is exposed to the libpq-API, this way. 2. libpq throws wrong errors. The error of libpq isn't Remaining errors in openssl error queue. libpq requires a clear error queue in order to work correctly., but instead it throws arbitrary foreign errors that could relate to or may not relate to the communication of libpq. The documentation for SSL_get_error(3) is pretty unambiguous about the need to clear the error queue first. 3. The sensitivity of libpq to the error queue can lead to bugs that are hard to track down, like this one [2]. This is because a libpq error leads the developer to look for a bug related to the database connection, although the issue is in a very different part of the code. Regards, Lars [1] http://bugs.ruby-lang.org/issues/7215 [2] https://bitbucket.org/ged/ruby-pg/issue/142/async_exec-over-ssl-connection-can-fail-on -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Failing SSL connection due to weird interaction with openssl
While investigating a ruby-pg issue [1], we noticed that a libpq SSL connection can fail, if the running application uses OpenSSL for other work, too. Root cause is the thread local error queue of OpenSSL, that is used to transmit textual error messages to the application after a failed crypto operation. In case that the application leaves errors on the queue, the communication to the PostgreSQL server can fail with a message left from the previous failed OpenSSL operation, in particular when using non-blocking operations on the socket. This issue with openssl is quite old now - see [3]. For [1] it turned out that the issue is subdivided into these three parts: 1. the ruby-openssl binding does not clear the thread local error queue of OpenSSL after a certificate verify 2. OpenSSL makes use of a shared error queue for different crypto contexts. 3. libpq does not ensure a cleared error queue when doing SSL_* calls To 1: Remaining messages on the error queue can generally lead to failing operations, later on. I'd talk to the ruby-openssl developers, to discuss how we can avoid any remaining messages on the queue. To 2: SSL_get_error() inspects the shared error queue under some conditions. It's maybe poor API design, but it's documented behaviour [2]. So we certainly have to get along with it. To 3: To make libpq independent to a previous error state, the error queue might be cleared with a call to ERR_clear_error() prior SSL_connect/read/write as in the attached trivial patch. This would make libpq robust against other uses of openssl within the application. What do you think about clearing the OpenSSL error queue in libpq in that way? [1] https://bitbucket.org/ged/ruby-pg/issue/142/async_exec-over-ssl-connection-can-fail-on [2] http://www.openssl.org/docs/ssl/SSL_get_error.html [3] http://www.educatedguesswork.org/movabletype/archives/2005/03/curse_you_opens.html diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index b1ad776..2a09c5c 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -323,6 +323,8 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len) /* SSL_read can write to the socket, so we need to disable SIGPIPE */ DISABLE_SIGPIPE(conn, spinfo, return -1); + /* There could be errors left on OpenSSL's error queue from the application */ + ERR_clear_error(); rloop: SOCK_ERRNO_SET(0); @@ -485,6 +487,8 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len) int err; DISABLE_SIGPIPE(conn, spinfo, return -1); + /* There could be errors left on OpenSSL's error queue from the application */ + ERR_clear_error(); SOCK_ERRNO_SET(0); n = SSL_write(conn-ssl, ptr, len); @@ -1375,6 +1379,9 @@ open_client_SSL(PGconn *conn) { int r; + /* There could be errors left on OpenSSL's error queue from the application */ + ERR_clear_error(); + r = SSL_connect(conn-ssl); if (r = 0) { -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64
Am Freitag, 9. Dezember 2011, 15:31:17 schrieb Andrew Dunstan: Yeah, fair enough. I'll work on that. Many thanks for reviewing, tweaking and commiting the patch! One thing I wonder about, is this snippet. Is the define really needed now? * The Mingw64 headers choke if this is already defined - they * define it themselves. */ #if !defined(__MINGW64_VERSION_MAJOR) || defined(WIN32_ONLY_COMPILER) #define _WINSOCKAPI_ #endif #include winsock2.h Kind regards, Lars Kanis -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64
Am Montag, 12. Dezember 2011, 10:19:46 schrieb Andrew Dunstan: On 12/12/2011 09:54 AM, Lars Kanis wrote: Am Freitag, 9. Dezember 2011, 15:31:17 schrieb Andrew Dunstan: Yeah, fair enough. I'll work on that. Many thanks for reviewing, tweaking and commiting the patch! One thing I wonder about, is this snippet. Is the define really needed now? * The Mingw64 headers choke if this is already defined - they * define it themselves. */ #if !defined(__MINGW64_VERSION_MAJOR) || defined(WIN32_ONLY_COMPILER) #define _WINSOCKAPI_ #endif #includewinsock2.h As previously discussed, unless you can prove it's not needed I don't want to remove it, on the if it ain't broke don't fix it principle. I believe it is needed for at least some older compilers (specifically some of those used by buildfarm animals narwhal, frogmouth, mastodon, hamerkop and currawong), and it doesn't appear to be hurting anything. As you can see above it's been disabled for all Mingw-w64 compilers. Ok. Thanks for clarification. Kind regards, Lars -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64
Hi PostgreSQL hackers, support for Mingw-w64 compiler was added to postgres with commit 91812df. Unfortunately only the 64 bit output is working right now. This issue was already highlighted with initial patch in http://archives.postgresql.org/pgsql-bugs/2011-07/msg00059.php Mingw-w64 uses the same header files for 32 and 64 bit compiles. So the same conditions apply to mingw-w32 bit as for the WIN64 case. In WIN64 WSAAPI is defined to nothing, but in 32 bit to stdcall, so it needs to be used in the accept-parameter check, too. Maybe you prefer PASCAL instead of WSAAPI in configure. I tested successful compilation for the following platforms: - i686-w64-mingw32 - gcc v4.6.1 - x86_64-w64-mingw32 - gcc v4.6.1 - i586-mingw32msvc - gcc v4.4.4 - x86_64-linux-gnu - gcc v4.6.1 -- Kind regards, Lars Kanis diff --git a/config/ac_func_accept_argtypes.m4 b/config/ac_func_accept_argtypes.m4 index 1e77179..a82788d 100644 --- a/config/ac_func_accept_argtypes.m4 +++ b/config/ac_func_accept_argtypes.m4 @@ -46,7 +46,7 @@ AC_DEFUN([AC_FUNC_ACCEPT_ARGTYPES], [AC_CACHE_VAL(ac_cv_func_accept_arg1,dnl [AC_CACHE_VAL(ac_cv_func_accept_arg2,dnl [AC_CACHE_VAL(ac_cv_func_accept_arg3,dnl -[for ac_cv_func_accept_return in 'int' 'unsigned int PASCAL' 'SOCKET'; do +[for ac_cv_func_accept_return in 'int' 'unsigned int PASCAL' 'SOCKET WSAAPI'; do for ac_cv_func_accept_arg1 in 'int' 'unsigned int' 'SOCKET'; do for ac_cv_func_accept_arg2 in 'struct sockaddr *' 'const struct sockaddr *' 'void *'; do for ac_cv_func_accept_arg3 in 'int' 'size_t' 'socklen_t' 'unsigned int' 'void'; do diff --git a/configure b/configure index 58fea90..4118caf 100755 --- a/configure +++ b/configure @@ -18808,7 +18808,7 @@ else if test ${ac_cv_func_accept_arg3+set} = set; then $as_echo_n (cached) 6 else - for ac_cv_func_accept_return in 'int' 'unsigned int PASCAL' 'SOCKET'; do + for ac_cv_func_accept_return in 'int' 'unsigned int PASCAL' 'SOCKET WSAAPI'; do for ac_cv_func_accept_arg1 in 'int' 'unsigned int' 'SOCKET'; do for ac_cv_func_accept_arg2 in 'struct sockaddr *' 'const struct sockaddr *' 'void *'; do for ac_cv_func_accept_arg3 in 'int' 'size_t' 'socklen_t' 'unsigned int' 'void'; do diff --git a/src/include/c.h b/src/include/c.h index 0391860..040990f 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -58,17 +58,22 @@ #endif #include postgres_ext.h -#if _MSC_VER = 1400 || defined(WIN64) -#define errcode __msvc_errcode -#include crtdefs.h -#undef errcode -#endif - #include stdio.h #include stdlib.h #include string.h #include stddef.h #include stdarg.h + +/* __MINGW64_VERSION_MAJOR is related to both 32/64 bit gcc compiles by + * mingw-w64, however it gots defined only after + * #include any standard mingw header + */ +#if _MSC_VER = 1400 || defined(__MINGW64_VERSION_MAJOR) +#define errcode __msvc_errcode +#include crtdefs.h +#undef errcode +#endif + #ifdef HAVE_STRINGS_H #include strings.h #endif diff --git a/src/include/port/win32.h b/src/include/port/win32.h index 34f4004..d4acfae 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -31,7 +31,7 @@ * The Mingw64 headers choke if this is already defined - they * define it themselves. */ -#if !defined(WIN64) || defined(WIN32_ONLY_COMPILER) +#if !defined(__MINGW64_VERSION_MAJOR) || defined(WIN32_ONLY_COMPILER) #define _WINSOCKAPI_ #endif #include winsock2.h diff --git a/src/port/getaddrinfo.c b/src/port/getaddrinfo.c index db19878..60c522f 100644 --- a/src/port/getaddrinfo.c +++ b/src/port/getaddrinfo.c @@ -329,7 +329,7 @@ gai_strerror(int errcode) return Not enough memory; #endif #ifdef EAI_NODATA -#if !defined(WIN64) !defined(WIN32_ONLY_COMPILER) /* MSVC/WIN64 duplicate */ +#if !defined(__MINGW64_VERSION_MAJOR) !defined(WIN32_ONLY_COMPILER) /* MSVC/WIN64 duplicate */ case EAI_NODATA: return No host data of that type was found; #endif -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64
Can you please provide me with some howto on building PG sources with mingw-w64? For 32/64 bit mingw-v4.6.1 on ubuntu 11.10: apt-get install flex gcc-mingw-w64 ./configure --host=i686-w64-mingw32 --build=x86_64-linux --without-zlib make and ./configure --host=x86_64-w64-mingw32 --build=x86_64-linux --without-zlib make For 32 bit mingw-v4.4.4 on ubuntu 11.10: apt-get install flex gcc-mingw32 ./configure --host=i586-mingw32msvc --build=x86_64-linux --without-zlib make Regards, Lars Kanis
Re: [HACKERS] [PATCH] PostgreSQL fails to build with 32bit MinGW-w64
Isn't it better to check the value of macros itsef rather than checking for system dependent macros that does not directly relate to the issue? specifically for getaddrinfo.c case I think #if EAI_NODATA != EAI_NONAME is a better check than checking for #if !defined(__MINGW64_VERSION_MAJOR) !defined(WIN32_ONLY_COMPILER) /* MSVC/WIN64 duplicate */ Yes it's better and it works for all described test environments. For the win32.h, I really don't understand why _WINSOCKAPI_ was defined before winsock2.h some google suggests that defining _WINSOCKAPI_ before windows.h prevents inclusion of winsock.h but that does not have relation to inclusion of winsock2.h and if winsock2.h is included first, it should be ok. If this guess is right, perhaps it could be better to remove the three lines. #if !defined(WIN64) || defined(WIN32_ONLY_COMPILER) #define _WINSOCKAPI_ #endif I only changed this for consistency. For me, it works without that define in all test environments, too. +/* __MINGW64_VERSION_MAJOR is related to both 32/64 bit gcc compiles by + * mingw-w64, however it gots defined only after Why not use __MINGW32__, which is defined without including any headers? At least in mingw32 v4.4.4 there is no crtdefs.h. I couldn't find a proper define that relates directly to that issue, so attached is a somewhat cumbersome MINGW version check. -- Regards, Lars Kanis diff --git a/config/ac_func_accept_argtypes.m4 b/config/ac_func_accept_argtypes.m4 index 1e77179..a82788d 100644 --- a/config/ac_func_accept_argtypes.m4 +++ b/config/ac_func_accept_argtypes.m4 @@ -46,7 +46,7 @@ AC_DEFUN([AC_FUNC_ACCEPT_ARGTYPES], [AC_CACHE_VAL(ac_cv_func_accept_arg1,dnl [AC_CACHE_VAL(ac_cv_func_accept_arg2,dnl [AC_CACHE_VAL(ac_cv_func_accept_arg3,dnl -[for ac_cv_func_accept_return in 'int' 'unsigned int PASCAL' 'SOCKET'; do +[for ac_cv_func_accept_return in 'int' 'unsigned int PASCAL' 'SOCKET WSAAPI'; do for ac_cv_func_accept_arg1 in 'int' 'unsigned int' 'SOCKET'; do for ac_cv_func_accept_arg2 in 'struct sockaddr *' 'const struct sockaddr *' 'void *'; do for ac_cv_func_accept_arg3 in 'int' 'size_t' 'socklen_t' 'unsigned int' 'void'; do diff --git a/configure b/configure index de9ba5a..150ceb0 100755 --- a/configure +++ b/configure @@ -18808,7 +18808,7 @@ else if test ${ac_cv_func_accept_arg3+set} = set; then $as_echo_n (cached) 6 else - for ac_cv_func_accept_return in 'int' 'unsigned int PASCAL' 'SOCKET'; do + for ac_cv_func_accept_return in 'int' 'unsigned int PASCAL' 'SOCKET WSAAPI'; do for ac_cv_func_accept_arg1 in 'int' 'unsigned int' 'SOCKET'; do for ac_cv_func_accept_arg2 in 'struct sockaddr *' 'const struct sockaddr *' 'void *'; do for ac_cv_func_accept_arg3 in 'int' 'size_t' 'socklen_t' 'unsigned int' 'void'; do diff --git a/src/include/c.h b/src/include/c.h index 0391860..db2cb60 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -58,7 +58,7 @@ #endif #include postgres_ext.h -#if _MSC_VER = 1400 || defined(WIN64) +#if _MSC_VER = 1400 || (__MINGW32__ (__GNUC__ 4 || (__GNUC__ = 4 __GNUC_MINOR__ = 6))) #define errcode __msvc_errcode #include crtdefs.h #undef errcode diff --git a/src/include/port/win32.h b/src/include/port/win32.h index 34f4004..2e72ecc 100644 --- a/src/include/port/win32.h +++ b/src/include/port/win32.h @@ -27,13 +27,6 @@ #undef ERROR -/* - * The Mingw64 headers choke if this is already defined - they - * define it themselves. - */ -#if !defined(WIN64) || defined(WIN32_ONLY_COMPILER) -#define _WINSOCKAPI_ -#endif #include winsock2.h #include ws2tcpip.h #include windows.h diff --git a/src/port/getaddrinfo.c b/src/port/getaddrinfo.c index db19878..721b335 100644 --- a/src/port/getaddrinfo.c +++ b/src/port/getaddrinfo.c @@ -328,12 +328,10 @@ gai_strerror(int errcode) case EAI_MEMORY: return Not enough memory; #endif -#ifdef EAI_NODATA -#if !defined(WIN64) !defined(WIN32_ONLY_COMPILER) /* MSVC/WIN64 duplicate */ +#if defined(EAI_NODATA) EAI_NODATA != EAI_NONAME /* MSVC/WIN64 duplicate */ case EAI_NODATA: return No host data of that type was found; #endif -#endif #ifdef EAI_SERVICE case EAI_SERVICE: return Class type not found; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] user mapping extension to pg_ident.conf
Am Dienstag, 21. Juli 2009 16:01:01 schrieben Sie: Doing it on the client presents a certain challenge when it comes to certificates for example - or really in any case where you need to map the username to something else. It would be quite convenient to have that ability controlled from the server side. We'd have to have some way to communicate down that the username specified was the default one and not a user-specified one (or we're back at overriding), but if the actual mapping could be controlled server-side it would be a lot more convenient. I thought about doing it on the client side too, but server side mapping seemed to me more flexible. In fact one could do a client side mapping (without knowledge of the auth method), by greping the username of the external system out of the error text from the server and doing a second auth with it. Just I don't like this ugly hack. There was another mail, where I described the use of the mapping patch: http://archives.postgresql.org/pgsql-hackers/2009-06/msg01496.php Please have a look on it. One can give different mappings for combinations of internal and external username. This way you could easy use different roles within the database with differnent applications, although the external auth system gives the same username. The dummy-user of the first mail was not the best example. I'm not the expert with PGs internals. So if you have a better way to get an usermapping based on the external auth, I could do a bit of research in this area, because I need something like this. regards Lars Kanis signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] [PATCH] user mapping extension to pg_ident.conf
Am Montag, 29. Juni 2009 18:12:13 schrieben Sie: Lars Kanis ka...@comcard.de writes: The problem I have, is that I want to use an ordinary windows application, which connects to an arbitrary ODBC data source. This application stores a fixed username und password for the connection within it's own binary data file. It doesn't know anything about TLS-connection nor smartcard based authentication. All this is done in the libpg.dll. This seems to boil down to I'm willing to damage PG's authentication mechanisms to an unlimited extent to work around one broken proprietary windows app. I'm not excited about it. Oh no! You totally misunderstood me. So please give me another try to explain the addition: It's NOT about one stupid or broken (it isn't) application that has to be corrected. It's NOT about a specific problem with ODBC connections. It's a much more general thing, regardless which application/programming language/connection-library you use. I wouldn't post a quick an dirty hack to work around one broken proprietary windows app! The situation with any external single sign-on authentication method (GSSAPI, SSPI, Kerberos, Certificate authentication) is that you always have to provide a username. It is differentiated between the SYSTEM-USERNAME and the PG-USERNAME. The SYSTEM-USERNAME is the name the user is identified to by the external authentication. It is usually done with cryptographic validation of some tickets or certificates. So this value is the important one in terms of security. The PG-USERNAME is what the application tells the server the user wants to act as, in the database. It has no value at all in terms of security - it's just a wish. It is verified through the mapping of pg_ident.conf whether the wish can be granted with the given SYSTEM-USERNAME. The problem arises when you want to work with 1:1 mapping of SYSTEM-USERNAME to PG-USERNAME. You don't want to let the user type in the PG-USERNAME he wants to act as (I have never seen an kerberos-enabled application to do so). So the application has to take into account which authentication method you actually use, to extract the exact username from credentials/certificate. That means you have to open a second channel to your SSO-System to provide the correct username to the PG-server. And it means you have to do some sort of duplicated code in the application, which is already realised within libpq. Currently you can not say: Dear libpq, please identify the current user and authenticate to the PG-server (by whatever authentication methods you're allowed to by pg_hba.conf), and log in to the own account of the user. My addition allows to abstract the application from the used authentication method. It adds an level of indirection between PG-USERNAME (given by the application) and the EFFECTIVE-USERNAME (for actual db-role) based on the SYSTEM-USERNAME (given by the SSO-system). And it adds it in a way that fits nicely into the already existend mapping-file pg_ident.conf. Let's show the following example-table in pg_ident.conf: # MAPNAME SYSTEM-USERNAMEPG-USERNAME EFFECTIVE-USERNAME gssapi-user/^(.*)@domain\.com$simple-role \1 gssapi-user/^use...@domain\.com$ super-roleuser_a gssapi-user/^use...@domain\.com$ super-roleuser_c The first line says: Any user with an account on the PG-server and valid GSSAPI-credentials can log in to it's own PG-account. The application doesn't need to care about which user is actually working with it. It simply needs to say to server to use the users 'simple-role'. You can write the application completely independent to the authentication method to use. For whatever security relevant action in this or another application, you can log in to the 'super-role' of the user. But only user_a and user_c are able to do so. Likewise the application don't need to care about the actually used SSO-method or system. Also to mention: The addition is fully backward compatible. Have you even considered the potential for security problems arising from this? Yes, of course. The primary goal of the addition is to get more security. Some questions I considered: Can I go around permission checks which are carried out by PG? That's the most important question, I think. As I stated above, the PG-USERNAME always has an informative nature. It's just a wish with no value in terms of security. So it is safe to change the name, as long as all usual permission checks are done on the changed one. In fact they are done. The identification and validation of users credentials are completed, so we can trust the SYSTEM-USERNAME. Then the pg_ident.conf comes in action and determines the EFFECTIVE-USERNAME. Any further permission checks are done afterwards. So the EFFECTIVE-USERNAME is checked against login-permission and so on. Does it get more security to give every application user it's own account within the database? Yes
Re: [HACKERS] [PATCH] user mapping extension to pg_ident.conf
# MAPNAME SYSTEM-USERNAMEPG-USERNAME EFFECTIVE-USERNAME gssapi-user/^(.*)@domain\.com$simple-role \1 gssapi-user/^use...@domain\.com$ super-roleuser_a gssapi-user/^use...@domain\.com$ super-roleuser_c My fault. The lower lines should be: gssapi-user/^use...@domain\.com$ super-rolesuper_user_a gssapi-user/^use...@domain\.com$ super-rolesuper_user_c regards Lars Kanis signature.asc Description: This is a digitally signed message part.
[HACKERS] [PATCH] user mapping extension to pg_ident.conf
Hi all, this patch adds the possibility to map the login-rolename to a different rolename actually used for permissions. What is it used for? I'm working with smartcard based TLS-authentication to connect to the PG server. Authentication is done with the keys and certificates from the card within the TLS handshake. Certificate-CN and login-username have to be the same or have to match by the pg_ident.conf. The role actually used for permissions is always the login-username. This patch allowes, to change the actually permissions to a role based on the certificate-CN. It is realised by an additional column in pg_ident.conf. When using ODBC, you have to setup a fixed username which is used for login. Different permissions depending on the CN of the certificate on the current smartcard could be achieved by the following line: # MAPNAME SYSTEM-USERNAMEPG-USERNAME EFFECTIVE-USERNAME ssl-user /(.*) dummy \1 The extension could be similar used for kerberos authentication, too. Bytheway I refactored the pg_ident-code a little bit, to avoid duplicated code and to allow substitution of more than one match (\2, \3 etc). Questions (I'm quite new to the PG-sources and used to write Ruby code): - Is this something useful - or is there a much easier way? - Are there any implementation shortcomings? regards Lars Kanis diff -ur postgresql-8.4rc1.orig/src/backend/libpq/auth.c postgresql-8.4rc1/src/backend/libpq/auth.c --- postgresql-8.4rc1.orig/src/backend/libpq/auth.c 2009-06-11 16:48:57.0 +0200 +++ postgresql-8.4rc1/src/backend/libpq/auth.c 2009-06-29 14:02:40.0 +0200 @@ -777,7 +777,7 @@ } ret = check_usermap(port-hba-usermap, port-user_name, kusername, - pg_krb_caseins_users); + pg_krb_caseins_users, port-user_name); krb5_free_ticket(pg_krb5_context, ticket); krb5_auth_con_free(pg_krb5_context, auth_context); @@ -1069,7 +1069,7 @@ } ret = check_usermap(port-hba-usermap, port-user_name, gbuf.value, - pg_krb_caseins_users); + pg_krb_caseins_users, port-user_name); gss_release_buffer(lmin_s, gbuf); @@ -1360,12 +1360,12 @@ namebuf = palloc(strlen(accountname) + strlen(domainname) + 2); sprintf(namebuf, %...@%s, accountname, domainname); - retval = check_usermap(port-hba-usermap, port-user_name, namebuf, true); + retval = check_usermap(port-hba-usermap, port-user_name, namebuf, true, port-user_name); pfree(namebuf); return retval; } else - return check_usermap(port-hba-usermap, port-user_name, accountname, true); + return check_usermap(port-hba-usermap, port-user_name, accountname, true, port-user_name); } #endif /* ENABLE_SSPI */ @@ -1847,7 +1847,7 @@ return STATUS_ERROR; } - return check_usermap(port-hba-usermap, port-user_name, ident_user, false); + return check_usermap(port-hba-usermap, port-user_name, ident_user, false, port-user_name); } @@ -2184,9 +2184,9 @@ port-user_name))); return STATUS_ERROR; } - + /* Just pass the certificate CN to the usermap check */ - return check_usermap(port-hba-usermap, port-user_name, port-peer_cn, false); + return check_usermap(port-hba-usermap, port-user_name, port-peer_cn, false, port-user_name); } #endif diff -ur postgresql-8.4rc1.orig/src/backend/libpq/hba.c postgresql-8.4rc1/src/backend/libpq/hba.c --- postgresql-8.4rc1.orig/src/backend/libpq/hba.c 2009-06-11 16:48:58.0 +0200 +++ postgresql-8.4rc1/src/backend/libpq/hba.c 2009-06-29 15:08:08.0 +0200 @@ -31,6 +31,7 @@ #include storage/fd.h #include utils/flatfiles.h #include utils/guc.h +#include utils/memutils.h @@ -1418,6 +1419,68 @@ return true; } +/* case (in-)sensitive string compare */ +static int strcmp_with_case( const char *str1, const char *str2, bool case_insensitive ){ + if (case_insensitive) + { + return pg_strcasecmp(str1, str2); + } + else + { + return strcmp(str1, str2); + } +} + +/* + Substitudes \1, \2, etc. within subst_in_str, based on the regexp-matches in extract_from_str. + + returns substituded string. It has to be pfree'd. +*/ +static char *regexp_substitude(size_t nr_matches, regmatch_t *matches, const char *extract_from_str, const char *subst_in_str){ + char *ofs; + char *psubst_out_str; + int nr_match; + char *psubst_in_str; + + psubst_in_str = psubst_out_str = pstrdup(subst_in_str); + + for(nr_match = 1; nr_match = nr_matches; nr_match++){ + char subst_marker[5]; + + snprintf(subst_marker, sizeof(subst_marker), \\%d, nr_match); + + if ((ofs = strstr(psubst_in_str, subst_marker)) != NULL) + { + /* substitution of the first argument requested */ + if (matches[nr_match].rm_so 0) + { +pfree(psubst_in_str); +return NULL; + } + + /* +* length: original length minus length of \1 plus length of match +* plus null terminator +*/ + psubst_out_str = palloc0(strlen(psubst_in_str) - 2 + (matches[nr_match].rm_eo - matches[nr_match].rm_so) + 1); + strncpy
Re: [HACKERS] [PATCH] user mapping extension to pg_ident.conf
Am Montag, 29. Juni 2009 16:26:56 schrieben Sie: Lars Kanis ka...@comcard.de writes: this patch adds the possibility to map the login-rolename to a different rolename actually used for permissions. This seems like an ugly addition with a very narrow use case. Can't you accomplish what you want with the existing usermap facility? You're right, my description is a bit incomplete. The problem I have, is that I want to use an ordinary windows application, which connects to an arbitrary ODBC data source. This application stores a fixed username und password for the connection within it's own binary data file. It doesn't know anything about TLS-connection nor smartcard based authentication. All this is done in the libpg.dll. It works fine so far, as long as I want to work with the sigle role given by the fixed username. I could map any cn-contents to this one user by writing: # MAPNAME SYSTEM-USERNAMEPG-USERNAME ssl-user /.*fixed_user The db internal role is always that given by the application. But I need to work with the role of the certificate-cn of the current smartcard, the application doesn't know about. Because the username is stored within the applications own binary data file I'm not able to change it according to the pluged in card. I think the same problem occurs with kerberos authentication. You can't get the role based on your kerberos ticket, when the username is not set likewise. So it seemed to me quite useful, to not just set which external name matches which login-username, but also to set which userrole is actually used for granted privilegs. This is done by an additional column with the same characteristic as column PG-USERNAME. Another way could be to add an parameter to the hba line which tells the server to not care about the login username, but to only use the external (CN) name. But this wouldn't have the flexibility of regexps like in pg_ident.conf. Hope this clarifies a bit. regards Lars Kanis signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Problems with index-scan on regexp in 8.1
Am Montag, 7. November 2005 14:13 schrieb Martijn van Oosterhout: On Mon, Nov 07, 2005 at 07:50:20AM +0100, Lars Kanis wrote: We're using Postgres 8.0.2 on SuSE10.0 (64-Bit). Tests on 8.1 beta 4 have shown no problems but this one: SELECT * FROM mitglieder WHERE lower(vorname::text)='lars' does a bitmap-index-scan like this: Check your locales. For non-ASCII locales the normal shortcuts for regex optimisation can't apply. Evidently your old installation uses a different locale from your new one. You should be able to make this work by declaring your index with text_pattern_ops, like so: CREATE INDEX myindex ON mytable(mycolumn text_pattern_ops); Hope this helps, Thank you much, it helps. The initdb-locales were different. pattern_ops work quite fine with the regexps. So I don't have any complaints to 8.1. kind regards Lars Kanis pgpSc4MeqUFZ4.pgp Description: PGP signature
[HACKERS] Problems with index-scan on regexp in 8.1
We're using Postgres 8.0.2 on SuSE10.0 (64-Bit). Tests on 8.1 beta 4 have shown no problems but this one: SELECT * FROM mitglieder WHERE lower(vorname::text)='lars' does a bitmap-index-scan like this: Bitmap Heap Scan on mitglieder (cost=10.68..3770.52 rows=1051 width=226) Recheck Cond: (lower((vorname)::text) = 'lars'::text) - Bitmap Index Scan on mitgl_lower_namen_idx (cost=0.00..10.68 rows=1051 width=0) Index Cond: (lower((vorname)::text) = 'lars'::text) but a regular expression always results in a seqscan: SELECT * FROM mitglieder WHERE lower(vorname::text)~'^lars' Seq Scan on mitglieder (cost=0.00..79703.73 rows=1 width=226) Filter: (lower((vorname)::text) ~ '^lars'::text) whereas V8.0.2 does a proper index-scan: Index Scan using mitgl_lower_namen_idx on mitglieder (cost=0.01..18.05 rows=4 width=225) Index Cond: ((lower((vorname)::text) = 'lars'::text) AND (lower((vorname)::text) 'lart'::text)) Filter: (lower((vorname)::text) ~ '^lars'::text) The use of indexes for regexp is quite important for the search in our interactive frontend. kind regards Lars Kanis ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match