Re: [HACKERS] Better name for PQsslAttributes()

2015-11-07 Thread Lars Kanis
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()

2015-11-07 Thread Lars Kanis
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()

2015-11-06 Thread Lars Kanis
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

2012-11-11 Thread Lars Kanis
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

2012-10-23 Thread Lars Kanis
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

2011-12-12 Thread Lars Kanis
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

2011-12-12 Thread Lars Kanis
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

2011-11-24 Thread Lars Kanis
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

2011-11-24 Thread Lars Kanis
 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

2011-11-24 Thread Lars Kanis
 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

2009-07-21 Thread Lars Kanis
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

2009-06-30 Thread Lars Kanis
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

2009-06-30 Thread Lars Kanis
 # 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

2009-06-29 Thread Lars Kanis
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

2009-06-29 Thread Lars Kanis
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

2005-11-08 Thread Lars Kanis
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

2005-11-07 Thread Lars Kanis
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