On Sat, Aug 22, 2015 at 11:24 PM, Heikki Linnakangas <hlinn...@iki.fi> wrote:
> On 07/08/2015 01:15 PM, Дмитрий Воронин wrote:
>>
>> 07.07.2015, 18:34, "Michael Paquier" <michael.paqu...@gmail.com>:
>>
>>> Speaking of which, I have rewritten the patch as attached. This looks
>>> way cleaner than the previous version submitted. Dmitry, does that
>>> look fine for you?
>>> I am switching this patch as "Waiting on Author".
>>
>>
>> Michael, hello. I'm looking your variant of patch. You create
>> function ssl_extensions_info(), that gives information of SSL
>> extensions in more informative view. So, it's cool.
>
>
> Should check the return value of every OpenSSL call for errors. At least
> BIO_new() could return NULL, but check all the docs of the others too.
Right, agreed for BIO_new(). BIO_write and BIO_get_mem_data can return
negative error code, but that's not necessarily the indication of an
error by looking at the docs, so I'd rather let them as-is.
X509V3_EXT_print is not documented but it can return <= 0 state code
if the operation fails so I guess that it makes sense to elog an
ERROR. sk_X509_EXTENSION_value and X509_EXTENSION_get_object return
NULL in case of failure (looking at the code tree of openssl), and
OBJ_obj2nid will return NID_undef in this case, so I think the code
as-is is fine. Another interesting thing is that BIO_free can fail and
we don't track that.
By the way, perhaps it would be worth doing similar things for the
other calls of BIO_free and BIO_new, no? I have attached a second
patch.
> Are all the functions used in the patch available in all the versions of
> OpenSSL we support?
We support openssl down to 0.9.7, right? And those functions are
present there (I recall vaguely checking that when looking at this
patch, and I just rechecked in case I missed something).
> Other than those little things, looks good to me.
Thanks!
--
Michael
From eabb75a8cdaa81303de8c74b8d097bf3e0138d38 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Sun, 23 Aug 2015 21:24:22 +0900
Subject: [PATCH 1/2] Add function for SSL extension information in sslinfo
This is done with the addition of a new function called ssl_extension_info.
---
contrib/sslinfo/Makefile | 3 +-
contrib/sslinfo/sslinfo--1.0--1.1.sql | 13 ++
.../sslinfo/{sslinfo--1.0.sql => sslinfo--1.1.sql} | 12 +-
contrib/sslinfo/sslinfo.c | 169 ++++++++++++++++++++-
contrib/sslinfo/sslinfo.control | 2 +-
doc/src/sgml/sslinfo.sgml | 19 +++
6 files changed, 212 insertions(+), 6 deletions(-)
create mode 100644 contrib/sslinfo/sslinfo--1.0--1.1.sql
rename contrib/sslinfo/{sslinfo--1.0.sql => sslinfo--1.1.sql} (81%)
diff --git a/contrib/sslinfo/Makefile b/contrib/sslinfo/Makefile
index 86cbf05..f6c1472 100644
--- a/contrib/sslinfo/Makefile
+++ b/contrib/sslinfo/Makefile
@@ -4,7 +4,8 @@ MODULE_big = sslinfo
OBJS = sslinfo.o $(WIN32RES)
EXTENSION = sslinfo
-DATA = sslinfo--1.0.sql sslinfo--unpackaged--1.0.sql
+DATA = sslinfo--1.0--1.1.sql sslinfo--1.1.sql \
+ sslinfo--unpackaged--1.0.sql
PGFILEDESC = "sslinfo - information about client SSL certificate"
ifdef USE_PGXS
diff --git a/contrib/sslinfo/sslinfo--1.0--1.1.sql b/contrib/sslinfo/sslinfo--1.0--1.1.sql
new file mode 100644
index 0000000..c98a3ae
--- /dev/null
+++ b/contrib/sslinfo/sslinfo--1.0--1.1.sql
@@ -0,0 +1,13 @@
+/* contrib/sslinfo/sslinfo--1.0--1.1.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "ALTER EXTENSION sslinfo UPDATE TO '1.1'" to load this file. \quit
+
+CREATE OR REPLACE FUNCTION ssl_extension_info(
+ OUT name text,
+ OUT value text,
+ OUT iscritical boolean
+)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'ssl_extension_info'
+LANGUAGE C STRICT;
diff --git a/contrib/sslinfo/sslinfo--1.0.sql b/contrib/sslinfo/sslinfo--1.1.sql
similarity index 81%
rename from contrib/sslinfo/sslinfo--1.0.sql
rename to contrib/sslinfo/sslinfo--1.1.sql
index 79ce656..d63ddd5 100644
--- a/contrib/sslinfo/sslinfo--1.0.sql
+++ b/contrib/sslinfo/sslinfo--1.1.sql
@@ -1,4 +1,4 @@
-/* contrib/sslinfo/sslinfo--1.0.sql */
+/* contrib/sslinfo/sslinfo--1.1.sql */
-- complain if script is sourced in psql, rather than via CREATE EXTENSION
\echo Use "CREATE EXTENSION sslinfo" to load this file. \quit
@@ -38,3 +38,13 @@ LANGUAGE C STRICT;
CREATE FUNCTION ssl_issuer_dn() RETURNS text
AS 'MODULE_PATHNAME', 'ssl_issuer_dn'
LANGUAGE C STRICT;
+
+/* new in 1.1 */
+CREATE OR REPLACE FUNCTION ssl_extension_info(
+ OUT name text,
+ OUT value text,
+ OUT iscritical boolean
+)
+RETURNS SETOF record
+AS 'MODULE_PATHNAME', 'ssl_extension_info'
+LANGUAGE C STRICT;
diff --git a/contrib/sslinfo/sslinfo.c b/contrib/sslinfo/sslinfo.c
index da201bd..959c628 100644
--- a/contrib/sslinfo/sslinfo.c
+++ b/contrib/sslinfo/sslinfo.c
@@ -9,13 +9,18 @@
#include "postgres.h"
#include "fmgr.h"
-#include "utils/numeric.h"
-#include "libpq/libpq-be.h"
+#include "funcapi.h"
#include "miscadmin.h"
-#include "utils/builtins.h"
+
+#include "access/htup_details.h"
+#include "catalog/pg_type.h"
+#include "libpq/libpq-be.h"
#include "mb/pg_wchar.h"
+#include "utils/builtins.h"
+#include "utils/numeric.h"
#include <openssl/x509.h>
+#include <openssl/x509v3.h>
#include <openssl/asn1.h>
PG_MODULE_MAGIC;
@@ -24,6 +29,13 @@ static Datum X509_NAME_field_to_text(X509_NAME *name, text *fieldName);
static Datum X509_NAME_to_text(X509_NAME *name);
static Datum ASN1_STRING_to_text(ASN1_STRING *str);
+/*
+ * Function context for data persisting over repeated calls.
+ */
+typedef struct
+{
+ TupleDesc tupdesc;
+} SSLExtensionInfoContext;
/*
* Indicates whether current session uses SSL
@@ -354,3 +366,154 @@ ssl_issuer_dn(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
return X509_NAME_to_text(X509_get_issuer_name(MyProcPort->peer));
}
+
+
+/*
+ * Returns information about available SSL extensions.
+ *
+ * Returns setof record made of the following values:
+ * - name of the extension.
+ * - value of the extension.
+ * - critical status of the extension.
+ */
+PG_FUNCTION_INFO_V1(ssl_extension_info);
+Datum
+ssl_extension_info(PG_FUNCTION_ARGS)
+{
+ X509 *cert = MyProcPort->peer;
+ FuncCallContext *funcctx;
+ STACK_OF(X509_EXTENSION) *ext_stack = NULL;
+ int call_cntr;
+ int max_calls;
+ MemoryContext oldcontext;
+ SSLExtensionInfoContext *fctx; /* User function context. */
+
+ if (SRF_IS_FIRSTCALL())
+ {
+
+ TupleDesc tupdesc;
+
+ /* create a function context for cross-call persistence */
+ funcctx = SRF_FIRSTCALL_INIT();
+
+ /*
+ * Switch to memory context appropriate for multiple function calls
+ */
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+
+ /* Create a user function context for cross-call persistence */
+ fctx = (SSLExtensionInfoContext *) palloc(sizeof(SSLExtensionInfoContext));
+
+ /*
+ * Need a tuple descriptor representing one INT and one TEXT column.
+ */
+ tupdesc = CreateTemplateTupleDesc(3, false);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name",
+ TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 2, "value",
+ TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 3, "is_critical",
+ BOOLOID, -1, 0);
+
+ fctx->tupdesc = BlessTupleDesc(tupdesc);
+
+ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("function returning record called in context "
+ "that cannot accept type record")));
+
+ /* Get all extensions of certificate */
+ if (cert && cert->cert_info)
+ ext_stack = cert->cert_info->extensions;
+
+ /* Set max_calls as a count of extensions in certificate */
+ max_calls = cert != NULL ? X509_get_ext_count(cert) : 0;
+
+ if (cert != NULL &&
+ ext_stack != NULL &&
+ max_calls > 0)
+ {
+ /* got results, keep track of them */
+ funcctx->max_calls = max_calls;
+ funcctx->user_fctx = fctx;
+ }
+ else
+ {
+ /* fast track when no results */
+ MemoryContextSwitchTo(oldcontext);
+ SRF_RETURN_DONE(funcctx);
+ }
+
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ /* stuff done on every call of the function */
+ funcctx = SRF_PERCALL_SETUP();
+
+ /*
+ * Initialize per-call variables.
+ */
+ call_cntr = funcctx->call_cntr;
+ max_calls = funcctx->max_calls;
+ fctx = funcctx->user_fctx;
+
+ Assert(cert != NULL && cert->cert_info != NULL);
+ ext_stack = cert->cert_info->extensions;
+
+ /* do when there is more left to send */
+ if (call_cntr < max_calls)
+ {
+ Datum values[3];
+ bool nulls[3];
+ char *buf;
+ HeapTuple tuple;
+ Datum result;
+ BIO *membuf = NULL;
+ X509_EXTENSION *ext = NULL;
+ ASN1_OBJECT *obj = NULL;
+ int nid;
+ char nullterm = '\0';
+
+ /* Get extension from certificate */
+ ext = sk_X509_EXTENSION_value(ext_stack, call_cntr);
+ obj = X509_EXTENSION_get_object(ext);
+ nid = OBJ_obj2nid(obj);
+ if (nid == NID_undef)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("Unknown extension at position %d",
+ call_cntr)));
+
+ /* Get extension name */
+ values[0] = CStringGetTextDatum(OBJ_nid2sn(nid));
+ nulls[0] = false;
+
+ /* Get next the extension value */
+ membuf = BIO_new(BIO_s_mem());
+ if (membuf == NULL)
+ elog(ERROR, "Failed to create BIO");
+ if (X509V3_EXT_print(membuf, ext, 0, 0) <= 0)
+ elog(ERROR, "Failed to print extension");
+ BIO_write(membuf, &nullterm, 1);
+ BIO_get_mem_data(membuf, &buf);
+ values[1] = CStringGetTextDatum(buf);
+ nulls[1] = false;
+
+ /* Get critical status */
+ values[2] = BoolGetDatum(X509_EXTENSION_get_critical(ext));
+ nulls[2] = false;
+
+ /* Build tuple */
+ tuple = heap_form_tuple(fctx->tupdesc, values, nulls);
+ result = HeapTupleGetDatum(tuple);
+
+ if (BIO_free(membuf) == 0)
+ elog(ERROR, "Failed to free BIO");
+
+ SRF_RETURN_NEXT(funcctx, result);
+ }
+
+ /* Do when there is no more left */
+ SRF_RETURN_DONE(funcctx);
+}
diff --git a/contrib/sslinfo/sslinfo.control b/contrib/sslinfo/sslinfo.control
index 1d2f058..dfcf17e 100644
--- a/contrib/sslinfo/sslinfo.control
+++ b/contrib/sslinfo/sslinfo.control
@@ -1,5 +1,5 @@
# sslinfo extension
comment = 'information about SSL certificates'
-default_version = '1.0'
+default_version = '1.1'
module_pathname = '$libdir/sslinfo'
relocatable = true
diff --git a/doc/src/sgml/sslinfo.sgml b/doc/src/sgml/sslinfo.sgml
index 22ef439..bcc27d4 100644
--- a/doc/src/sgml/sslinfo.sgml
+++ b/doc/src/sgml/sslinfo.sgml
@@ -219,6 +219,21 @@ emailAddress
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term>
+ <function>ssl_extension_info() returns setof record</function>
+ <indexterm>
+ <primary>ssl_extension_info</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Provide information about extensions of client certificate: extension name,
+ extension value, and if it is a critical extension.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</sect2>
@@ -228,6 +243,10 @@ emailAddress
<para>
Victor Wagner <email>vi...@cryptocom.ru</email>, Cryptocom LTD
</para>
+
+ <para>
+ Dmitry Voronin <email>carriingfat...@yandex.ru</email>
+ </para>
<para>
E-Mail of Cryptocom OpenSSL development group:
--
2.5.0
From d9292f649585716c37c1a31c58deee9017c51fb7 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Sun, 23 Aug 2015 22:15:48 +0900
Subject: [PATCH 2/2] Add more sanity checks in sslinfo
Those are more cosmetic changes, and an error in those code paths is
unlikely to happen, still it looks better to fail properly should an
error happen in those code paths when calling openssl routines related
to BIO manipulation.
---
contrib/sslinfo/sslinfo.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/contrib/sslinfo/sslinfo.c b/contrib/sslinfo/sslinfo.c
index 959c628..8e07d59 100644
--- a/contrib/sslinfo/sslinfo.c
+++ b/contrib/sslinfo/sslinfo.c
@@ -150,6 +150,8 @@ ASN1_STRING_to_text(ASN1_STRING *str)
text *result;
membuf = BIO_new(BIO_s_mem());
+ if (membuf == NULL)
+ elog(ERROR, "Failed to create BIO");
(void) BIO_set_close(membuf, BIO_CLOSE);
ASN1_STRING_print_ex(membuf, str,
((ASN1_STRFLGS_RFC2253 & ~ASN1_STRFLGS_ESC_MSB)
@@ -162,7 +164,8 @@ ASN1_STRING_to_text(ASN1_STRING *str)
result = cstring_to_text(dp);
if (dp != sp)
pfree(dp);
- BIO_free(membuf);
+ if (BIO_free(membuf) == 0)
+ elog(ERROR, "Failed to free BIO");
PG_RETURN_TEXT_P(result);
}
@@ -301,15 +304,24 @@ X509_NAME_to_text(X509_NAME *name)
char *dp;
text *result;
+ if (membuf == NULL)
+ elog(ERROR, "Failed to create BIO");
+
(void) BIO_set_close(membuf, BIO_CLOSE);
for (i = 0; i < count; i++)
{
e = X509_NAME_get_entry(name, i);
nid = OBJ_obj2nid(X509_NAME_ENTRY_get_object(e));
+ if (nid == NID_undef)
+ elog(ERROR, "Failed to get NID for ASN1_OBJECT object");
v = X509_NAME_ENTRY_get_data(e);
field_name = OBJ_nid2sn(nid);
- if (!field_name)
+ if (field_name == NULL)
field_name = OBJ_nid2ln(nid);
+ if (field_name)
+ elog(ERROR,
+ "Failed to convert the NID %d to an ASN1_OBJECT structure",
+ nid);
BIO_printf(membuf, "/%s=", field_name);
ASN1_STRING_print_ex(membuf, v,
((ASN1_STRFLGS_RFC2253 & ~ASN1_STRFLGS_ESC_MSB)
@@ -324,7 +336,8 @@ X509_NAME_to_text(X509_NAME *name)
result = cstring_to_text(dp);
if (dp != sp)
pfree(dp);
- BIO_free(membuf);
+ if (BIO_free(membuf) == 0)
+ elog(ERROR, "Failed to free BIO");
PG_RETURN_TEXT_P(result);
}
--
2.5.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
