Re: [HACKERS] [PATCH v11] GSSAPI encryption support

2016-04-04 Thread Robbie Harwood
Michael Paquier  writes:

> On Sat, Apr 2, 2016 at 7:34 AM, Robbie Harwood  wrote:
>
>>   Since I still can't reproduce this locally (left a client machine and
>>   a process on the same machine retrying for over an hour on your test
>>   case and didn't see it), could you provide me with some more
>>   information on why repalloc is complaining?
>>   Is this a low memory situation where alloc might have failed?
>
> No, this is an assertion failure, and it seems that you are compiling
> this code without --enable-cassert, without the switch the code
> actually works.

You are right.  I now see the assertion failure.

>>   That pointer looks like it's on the heap, is that correct?
>
> appendBinaryStringInfo depends on palloc calls that allocate memory
> depending on the memory context used. It looks that what's just
> missing in your logic is a private memory context that be_gssapi_write
> and be_gssapi_read can use to handle the allocation of the
> communication buffers.

Thank you very much for the pointer!  I will work in memory context
management for the next version.


signature.asc
Description: PGP signature


Re: [HACKERS] [PATCH v11] GSSAPI encryption support

2016-04-02 Thread Michael Paquier
On Sat, Apr 2, 2016 at 7:34 AM, Robbie Harwood  wrote:
> - Attempt to address a crash Michael is observing by switching to using
>   the StringInfo/pqExpBuffer management functions over my own code as
>   much as possible.  Michael, if this doesn't fix it, I'm out of ideas.

Nope, it doesn't.

>   Since I still can't reproduce this locally (left a client machine and
>   a process on the same machine retrying for over an hour on your test
>   case and didn't see it), could you provide me with some more
>   information on why repalloc is complaining?
>   Is this a low memory situation where alloc might have failed?

No, this is an assertion failure, and it seems that you are compiling
this code without --enable-cassert, without the switch the code
actually works.

>   What's your setup look like?

Just a simple Linux VM running krb5kdc with 386MB of memory, with
Postgres running locally as well.

>   That pointer looks like it's on the heap, is that correct?

appendBinaryStringInfo depends on palloc calls that allocate memory
depending on the memory context used. It looks that what's just
missing in your logic is a private memory context that be_gssapi_write
and be_gssapi_read can use to handle the allocation of the
communication buffers.
-- 
Michael


-- 
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 v11] GSSAPI encryption support

2016-04-01 Thread Robbie Harwood
Hello friends,

Song and dance, here's v11 both here and on my github:
https://github.com/frozencemetery/postgres/tree/feature/gssencrypt11

Changes from v10:

- Attempt to address a crash Michael is observing by switching to using
  the StringInfo/pqExpBuffer management functions over my own code as
  much as possible.  Michael, if this doesn't fix it, I'm out of ideas.
  Since I still can't reproduce this locally (left a client machine and
  a process on the same machine retrying for over an hour on your test
  case and didn't see it), could you provide me with some more
  information on why repalloc is complaining?  Is this a low memory
  situation where alloc might have failed?  What's your setup look like?
  That pointer looks like it's on the heap, is that correct?  I really
  don't have a handle on what's gone wrong here.

- Switch to using parse_bool for handling gss_encrypt.

- Remove accidental whitespace change.

Thanks!
From 945805d45e8021f92ad73518b3a74ac6bab89525 Mon Sep 17 00:00:00 2001
From: Robbie Harwood 
Date: Fri, 26 Feb 2016 16:07:05 -0500
Subject: [PATCH 1/3] Move common GSSAPI code into its own files

On both the frontend and backend, prepare for GSSAPI encryption suport
by moving common code for error handling into a common file.  Other than
build-system changes, no code changes occur in this patch.

Thanks to Michael Paquier for the Windows fixes.
---
 configure   |  2 +
 configure.in|  1 +
 src/Makefile.global.in  |  1 +
 src/backend/libpq/Makefile  |  4 ++
 src/backend/libpq/auth.c| 63 +---
 src/backend/libpq/be-gssapi-common.c| 74 +
 src/include/libpq/be-gssapi-common.h| 26 
 src/interfaces/libpq/Makefile   |  4 ++
 src/interfaces/libpq/fe-auth.c  | 48 +
 src/interfaces/libpq/fe-gssapi-common.c | 63 
 src/interfaces/libpq/fe-gssapi-common.h | 21 ++
 src/tools/msvc/Mkvcbuild.pm | 18 ++--
 12 files changed, 212 insertions(+), 113 deletions(-)
 create mode 100644 src/backend/libpq/be-gssapi-common.c
 create mode 100644 src/include/libpq/be-gssapi-common.h
 create mode 100644 src/interfaces/libpq/fe-gssapi-common.c
 create mode 100644 src/interfaces/libpq/fe-gssapi-common.h

diff --git a/configure b/configure
index b3f3abe..a5bd629 100755
--- a/configure
+++ b/configure
@@ -713,6 +713,7 @@ with_systemd
 with_selinux
 with_openssl
 krb_srvtab
+with_gssapi
 with_python
 with_perl
 with_tcl
@@ -5491,6 +5492,7 @@ $as_echo "$with_gssapi" >&6; }
 
 
 
+
 #
 # Kerberos configuration parameters
 #
diff --git a/configure.in b/configure.in
index 0bd90d7..4fd8f05 100644
--- a/configure.in
+++ b/configure.in
@@ -636,6 +636,7 @@ PGAC_ARG_BOOL(with, gssapi, no, [build with GSSAPI support],
   krb_srvtab="FILE:\$(sysconfdir)/krb5.keytab"
 ])
 AC_MSG_RESULT([$with_gssapi])
+AC_SUBST(with_gssapi)
 
 
 AC_SUBST(krb_srvtab)
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index e94d6a5..3dbc5c2 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -183,6 +183,7 @@ with_perl	= @with_perl@
 with_python	= @with_python@
 with_tcl	= @with_tcl@
 with_openssl	= @with_openssl@
+with_gssapi	= @with_gssapi@
 with_selinux	= @with_selinux@
 with_systemd	= @with_systemd@
 with_libxml	= @with_libxml@
diff --git a/src/backend/libpq/Makefile b/src/backend/libpq/Makefile
index 09410c4..a8cc9a0 100644
--- a/src/backend/libpq/Makefile
+++ b/src/backend/libpq/Makefile
@@ -21,4 +21,8 @@ ifeq ($(with_openssl),yes)
 OBJS += be-secure-openssl.o
 endif
 
+ifeq ($(with_gssapi),yes)
+OBJS += be-gssapi-common.o
+endif
+
 include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 57c2f48..73d493e 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -136,11 +136,7 @@ bool		pg_krb_caseins_users;
  *
  */
 #ifdef ENABLE_GSS
-#if defined(HAVE_GSSAPI_H)
-#include 
-#else
-#include 
-#endif
+#include "libpq/be-gssapi-common.h"
 
 static int	pg_GSS_recvauth(Port *port);
 #endif   /* ENABLE_GSS */
@@ -715,63 +711,6 @@ recv_and_check_password_packet(Port *port, char **logdetail)
  */
 #ifdef ENABLE_GSS
 
-#if defined(WIN32) && !defined(WIN32_ONLY_COMPILER)
-/*
- * MIT Kerberos GSSAPI DLL doesn't properly export the symbols for MingW
- * that contain the OIDs required. Redefine here, values copied
- * from src/athena/auth/krb5/src/lib/gssapi/generic/gssapi_generic.c
- */
-static const gss_OID_desc GSS_C_NT_USER_NAME_desc =
-{10, (void *) "\x2a\x86\x48\x86\xf7\x12\x01\x02\x01\x02"};
-static GSS_DLLIMP gss_OID GSS_C_NT_USER_NAME = &GSS_C_NT_USER_NAME_desc;
-#endif
-
-
-static void
-pg_GSS_error(int severity, char *errmsg, OM_uint32 maj_stat, OM_uint32 min_stat)
-{
-	gss_buffer_desc gmsg;
-	OM_uint32	lmin_s,
-msg_ctx;