Re: New functions
Hello, Michael.
Please, attach new version of my patch to commitfest page.
--О©╫
Best regards, Dmitry Voronin
Attachments:
ssl-extensions-3.patchtext/x-diff; name=ssl-extensions-3.patchDownload
*** /dev/null
--- b/contrib/sslinfo/sslinfo--1.0--1.1.sql
***************
*** 0 ****
--- 1,21 ----
+ /* 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_value(text) RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_extension_value'
+ LANGUAGE C STRICT;
+
+ CREATE OR REPLACE FUNCTION ssl_extension_is_critical(text) RETURNS boolean
+ AS 'MODULE_PATHNAME', 'ssl_extension_is_critical'
+ LANGUAGE C STRICT;
+
+ CREATE TYPE extension AS (
+ name text,
+ value text
+ );
+
+ CREATE OR REPLACE FUNCTION ssl_extension_names() RETURNS SETOF extension
+ AS 'MODULE_PATHNAME', 'ssl_extension_names'
+ LANGUAGE C STRICT;
*** /dev/null
--- b/contrib/sslinfo/sslinfo--1.1.sql
***************
*** 0 ****
--- 1,58 ----
+ /* 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
+
+ CREATE FUNCTION ssl_client_serial() RETURNS numeric
+ AS 'MODULE_PATHNAME', 'ssl_client_serial'
+ LANGUAGE C STRICT;
+
+ CREATE FUNCTION ssl_is_used() RETURNS boolean
+ AS 'MODULE_PATHNAME', 'ssl_is_used'
+ LANGUAGE C STRICT;
+
+ CREATE FUNCTION ssl_version() RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_version'
+ LANGUAGE C STRICT;
+
+ CREATE FUNCTION ssl_cipher() RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_cipher'
+ LANGUAGE C STRICT;
+
+ CREATE FUNCTION ssl_client_cert_present() RETURNS boolean
+ AS 'MODULE_PATHNAME', 'ssl_client_cert_present'
+ LANGUAGE C STRICT;
+
+ CREATE FUNCTION ssl_client_dn_field(text) RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_client_dn_field'
+ LANGUAGE C STRICT;
+
+ CREATE FUNCTION ssl_issuer_field(text) RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_issuer_field'
+ LANGUAGE C STRICT;
+
+ CREATE FUNCTION ssl_client_dn() RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_client_dn'
+ 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_value(text) RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_extension_value'
+ LANGUAGE C STRICT;
+
+ CREATE OR REPLACE FUNCTION ssl_extension_is_critical(text) RETURNS boolean
+ AS 'MODULE_PATHNAME', 'ssl_extension_is_critical'
+ LANGUAGE C STRICT;
+
+ CREATE TYPE extension AS (
+ name text,
+ value text
+ );
+
+ CREATE OR REPLACE FUNCTION ssl_extension_names() RETURNS SETOF extension
+ AS 'MODULE_PATHNAME', 'ssl_extension_names'
+ LANGUAGE C STRICT;
*** a/contrib/sslinfo/sslinfo.c
--- b/contrib/sslinfo/sslinfo.c
***************
*** 14,21 ****
--- 14,23 ----
#include "miscadmin.h"
#include "utils/builtins.h"
#include "mb/pg_wchar.h"
+ #include "funcapi.h"
#include <openssl/x509.h>
+ #include <openssl/x509v3.h>
#include <openssl/asn1.h>
PG_MODULE_MAGIC;
***************
*** 23,28 **** PG_MODULE_MAGIC;
--- 25,31 ----
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);
+ static bool get_extension(X509 *cert, const char *ext_name, X509_EXTENSION **extension);
/*
***************
*** 354,356 **** ssl_issuer_dn(PG_FUNCTION_ARGS)
--- 357,581 ----
PG_RETURN_NULL();
return X509_NAME_to_text(X509_get_issuer_name(MyProcPort->peer));
}
+
+
+ /*
+ * Returns extension object by given certificate and extension's name.
+ *
+ * Try to get extension from certificate by extension's name.
+ * We returns at extension param pointer to X509_EXTENSION.
+ *
+ * Returns true, if we have found extension in certificate and false, if we not.
+ */
+ static bool get_extension(X509* cert, const char *ext_name, X509_EXTENSION **extension)
+ {
+ int nid = 0;
+ int loc = 0;
+
+ /* try to convert extension name to ObjectID */
+ nid = OBJ_txt2nid(ext_name);
+ /* Not success ? */
+ if (nid == NID_undef)
+ return false;
+
+ loc = X509_get_ext_by_NID(cert, nid, -1);
+
+ /* palloc memory for extension and copy it */
+ *extension = (X509_EXTENSION *) palloc(sizeof(X509_EXTENSION *));
+ memcpy(*extension, X509_get_ext(cert, loc), sizeof(X509_EXTENSION));
+
+ return true;
+ }
+
+
+ /* Returns value of extension
+ *
+ * This function returns value of extension by given name in client certificate.
+ *
+ * Returns text datum.
+ */
+ PG_FUNCTION_INFO_V1(ssl_extension_value);
+ Datum
+ ssl_extension_value(PG_FUNCTION_ARGS)
+ {
+ X509 *cert = MyProcPort->peer;
+ X509_EXTENSION *ext = NULL;
+ char *ext_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ BIO *membuf = NULL;
+ char *val = NULL;
+ char nullterm = '\0';
+ bool error = false;
+
+ /* If we have no ssl security */
+ if (cert == NULL)
+ PG_RETURN_NULL();
+
+ /* If extension's converting from text name to extension's OID failed (return NID_undef) */
+ if (OBJ_txt2nid(ext_name) == NID_undef)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("Unknown extension name \"%s\"", ext_name)));
+
+ /* Extension's name is correct, try to get extension object from certificate */
+ error = get_extension(cert, ext_name, &ext);
+
+ /* Not found? */
+ if (!error)
+ PG_RETURN_NULL();
+
+ /* Print extension to BIO */
+ membuf = BIO_new(BIO_s_mem());
+ X509V3_EXT_print(membuf, ext, 0, 0);
+ BIO_write(membuf, &nullterm, 1);
+ BIO_get_mem_data(membuf, &val);
+
+ /* Copy value */
+ val = pstrdup(val);
+
+ /* Clear BIO */
+ BIO_free(membuf);
+
+ /* free extension */
+ if (ext)
+ pfree(ext);
+
+ PG_RETURN_TEXT_P(cstring_to_text(val));
+ }
+
+
+ /* Returns status of extension
+ *
+ * Returns true, if extension is critical and false, if it is not.
+ *
+ * Returns bool datum.
+ */
+ PG_FUNCTION_INFO_V1(ssl_extension_is_critical);
+ Datum
+ ssl_extension_is_critical(PG_FUNCTION_ARGS)
+ {
+ X509 *cert = MyProcPort->peer;
+ X509_EXTENSION *ext = NULL;
+ char *ext_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ int critical;
+ bool error = false;
+
+ /* If we have no ssl security */
+ if (cert == NULL)
+ PG_RETURN_NULL();
+
+ /* If extension's converting from text name to extension's OID failed (return NID_undef) */
+ if (OBJ_txt2nid(ext_name) == NID_undef)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("Unknown extension name \"%s\"", ext_name)));
+
+ /* Extension's name is correct, try to get extension object from certificate */
+ error = get_extension(cert, ext_name, &ext);
+
+ /* Not found? */
+ if (!error)
+ PG_RETURN_NULL();
+
+ critical = X509_EXTENSION_get_critical(ext);
+
+ /* free extension */
+ if (ext)
+ pfree(ext);
+
+ PG_RETURN_BOOL(critical);
+ }
+
+
+ /* Returns key-value pairs of extension name and it's value
+ *
+ * This function print extensions of client certificate at table form (extension's name and it's value).
+ *
+ * Returns setof text datum.
+ */
+ PG_FUNCTION_INFO_V1(ssl_extension_names);
+ Datum
+ ssl_extension_names(PG_FUNCTION_ARGS)
+ {
+ X509 *cert = MyProcPort->peer;
+ FuncCallContext *funcctx;
+ STACK_OF(X509_EXTENSION) *ext_stack = NULL;
+ int call;
+ int max_calls;
+ TupleDesc tupdesc;
+ AttInMetadata *attinmeta;
+ MemoryContext oldcontext;
+ char **values;
+ HeapTuple tuple;
+ int nid;
+ X509_EXTENSION *ext = NULL;
+ ASN1_OBJECT *obj = NULL;
+ BIO *membuf = NULL;
+ char nullterm = '\0';
+
+ /* If we have no ssl security */
+ if (cert == NULL)
+ PG_RETURN_NULL();
+
+ /* Get all extensions of certificate */
+ ext_stack = cert->cert_info->extensions;
+
+ /* Certificate does not have extensions? */
+ if (ext_stack == NULL)
+ PG_RETURN_NULL();
+
+ if (SRF_IS_FIRSTCALL())
+ {
+ funcctx = SRF_FIRSTCALL_INIT();
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+ /* Set mac_calls as a count of extensions in certificate */
+ funcctx->max_calls = X509_get_ext_count(cert);
+
+ 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")));
+
+ attinmeta = TupleDescGetAttInMetadata(tupdesc);
+ funcctx->attinmeta = attinmeta;
+
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ funcctx = SRF_PERCALL_SETUP();
+ call = funcctx->call_cntr;
+ max_calls = funcctx->max_calls;
+ attinmeta = funcctx->attinmeta;
+
+ if (call < max_calls)
+ {
+ values = (char **) palloc(2 * sizeof(char *));
+
+ /* Get extension from certificate */
+ ext = sk_X509_EXTENSION_value(ext_stack, call);
+ 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)));
+
+ /* Set extension name as first value in array */
+ values[0] = (char *) OBJ_nid2sn(nid);
+
+ /* Print extension's value and set as second value in array */
+ membuf = BIO_new(BIO_s_mem());
+ X509V3_EXT_print(membuf, ext, 0, 0);
+ BIO_write(membuf, &nullterm, 1);
+ BIO_get_mem_data(membuf, &values[1]);
+
+ /* Build tuple */
+ tuple = BuildTupleFromCStrings(attinmeta, values);
+
+ BIO_free(membuf);
+
+ SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(tuple));
+ }
+ SRF_RETURN_DONE(funcctx);
+ }
*** a/doc/src/sgml/sslinfo.sgml
--- b/doc/src/sgml/sslinfo.sgml
***************
*** 219,224 **** emailAddress
--- 219,266 ----
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term>
+ <function>ssl_extension_value(extension_name text) returns text</function>
+ <indexterm>
+ <primary>ssl_extension_value</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Returns value of extension by given name in client certificate.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term>
+ <function>ssl_extension_is_critical(extension_name text) returns boolean</function>
+ <indexterm>
+ <primary>ssl_extension_is_critical</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Returns TRUE if extension is critical and FALSE if not.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term>
+ <function>ssl_extension_names() returns SETOF text </function>
+ <indexterm>
+ <primary>ssl_extension_names</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Print extensions of client certificate at table form (extension's name and it's value).
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</sect2>
***************
*** 228,233 **** emailAddress
--- 270,279 ----
<para>
Victor Wagner <email>vitus@cryptocom.ru</email>, Cryptocom LTD
</para>
+
+ <para>
+ Extension's functions: Dmitry Voronin <email>carriingfate92@yandex.ru</email>
+ </para>
<para>
E-Mail of Cryptocom OpenSSL development group:
О©╫Please, attach new version of my patch to commitfest page.
Micheal, I found a way to attach patch. sorry to trouble.
--О©╫
Best regards, Dmitry Voronin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 23, 2015 at 1:48 AM, Воронин Дмитрий
<carriingfate92@yandex.ru> wrote:
Please, attach new version of my patch to commitfest page.
Michael, I found a way to attach patch. sorry to trouble.
Cool. Thanks. I am seeing your patch entry here:
https://commitfest.postgresql.org/5/192/
I'll try to take a look at it for the next commit fest, but please do
not expect immediate feedback things are getting wrapped up for 9.5.
Regards,
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 23, 2015 at 12:46 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Mon, Mar 23, 2015 at 1:48 AM, Воронин Дмитрий
<carriingfate92@yandex.ru> wrote:Please, attach new version of my patch to commitfest page.
Michael, I found a way to attach patch. sorry to trouble.
Cool. Thanks. I am seeing your patch entry here:
https://commitfest.postgresql.org/5/192/
I'll try to take a look at it for the next commit fest, but please do
not expect immediate feedback things are getting wrapped up for 9.5.
OK, I am back on this patch and I am just looking at it. There are a
couple of issues that need to be fixed, and I will send back feedback
and a new patch by tomorrow.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 23, 2015 at 12:46 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Mon, Mar 23, 2015 at 1:48 AM, Воронин Дмитрий
<carriingfate92@yandex.ru> wrote:Please, attach new version of my patch to commitfest page.
Michael, I found a way to attach patch. sorry to trouble.
Cool. Thanks. I am seeing your patch entry here:
https://commitfest.postgresql.org/5/192/
I'll try to take a look at it for the next commit fest, but please do
not expect immediate feedback things are getting wrapped up for 9.5.
OK, so I have looked at this patch in more details. And here are some comments:
1) As this is an upgrade to sslinfo 1.1, sslinfo--1.0.sql is not necessary.
2) contrib/sslinfo/Makefile needs to be updated with
sslinfo--1.0--1.1.sql and sslinfo--1.1.sql.
3) This return type is not necessary:
+ CREATE TYPE extension AS (
+ name text,
+ value text
+ );
+
+ CREATE OR REPLACE FUNCTION ssl_extension_names() RETURNS SETOF extension
+ AS 'MODULE_PATHNAME', 'ssl_extension_names'
+ LANGUAGE C STRICT;
Instead, I think that we should make ssl_extension_names return a
SETOF record with some OUT parameters. Also, using a tuple descriptor
saved in the user context would bring more readability.
4) sslinfo.control needs to be updated to 1.1.
5) I think that it is better to return an empty result should no
client certificate be present or should ssl be disabled for a given
connection. And the patch failed to do that with SRF_RETURN_DONE.
6) The code is critically lacking comments, and contains many typos.
7) The return status of get_extention() is not necessary. All the code
paths calling it simply ERROR should the status be false. It is better
to move the error message directly in the function and remove the
status code.
8) As proposed, the patch adds 3 new functions:
ssl_extension_is_critical, ssl_extension_value and
ssl_extension_names. But actually I am wondering why
ssl_extension_is_critical and ssl_extension_value are actually useful.
I mean, what counts is the extension information about the existing
client certificate, no? Hence I think that we should remove
ssl_extension_is_critical and ssl_extension_value, and extend
ssl_extension_names with a new boolean flag is_critical to determine
if a extension given is critical or not. Let's rename
ssl_extension_names to ssl_extension_info at the same time.
get_extension is not needed anymore with that as well.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".
Regards,
--
Michael
Attachments:
0001-Add-ssl_extension_info-in-sslinfo.patchtext/x-diff; charset=US-ASCII; name=0001-Add-ssl_extension_info-in-sslinfo.patchDownload
From 6ab5ec9ea6705eaf196b07e76b04a78fa0a0f220 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Tue, 7 Jul 2015 23:01:24 +0900
Subject: [PATCH] Add ssl_extension_info in sslinfo
This new function provides information about extension in client
certificates: extension name, extension value and critical status.
---
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 | 167 ++++++++++++++++++++-
contrib/sslinfo/sslinfo.control | 2 +-
doc/src/sgml/sslinfo.sgml | 19 +++
6 files changed, 210 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..07b9ac2 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,152 @@ 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);
+#define SSLINFO_EXTENSION_INFO_NUM
+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());
+ X509V3_EXT_print(membuf, ext, 0, 0);
+ 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);
+
+ BIO_free(membuf);
+
+ 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>vitus@cryptocom.ru</email>, Cryptocom LTD
</para>
+
+ <para>
+ Dmitry Voronin <email>carriingfate92@yandex.ru</email>
+ </para>
<para>
E-Mail of Cryptocom OpenSSL development group:
--
2.4.5
07.07.2015, 18:34, "Michael Paquier" <michael.paquier@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".
О©╫Regards,
О©╫--
О©╫Michael
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.
--О©╫
Best regards, Dmitry Voronin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jul 8, 2015 at 9:15 PM, Дмитрий Воронин
<carriingfate92@yandex.ru> wrote:
07.07.2015, 18:34, "Michael Paquier" <michael.paquier@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".
Regards,
--
MichaelMichael, 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.
OK cool. I think a committer could look at it, hence switching it to
"Ready for Committer".
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Jul 8, 2015 at 10:18 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Wed, Jul 8, 2015 at 9:15 PM, Дмитрий Воронин
<carriingfate92@yandex.ru> wrote:07.07.2015, 18:34, "Michael Paquier" <michael.paquier@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".
Regards,
--
MichaelMichael, 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.
OK cool. I think a committer could look at it, hence switching it to
"Ready for Committer".
Note for committers: attached is a small script that will generate a
client certificate with extensions enabled. This is helpful when
testing this patch. Once created, then simply connect with something
like this connection string:
"host=127.0.0.1 sslmode=verify-full sslcert=client.crt
sslkey=client.key sslrootcert=server.crt"
By querying the new function implemented by this patch the information
about the client certificate extensions will show up.
--
Michael
Attachments:
On 07/08/2015 01:15 PM, О©╫О©╫О©╫О©╫О©╫О©╫О©╫ О©╫О©╫О©╫О©╫О©╫О©╫О©╫ wrote:
07.07.2015, 18:34, "Michael Paquier" <michael.paquier@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.
Are all the functions used in the patch available in all the versions of
OpenSSL we support?
Other than those little things, looks good to me.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Aug 22, 2015 at 11:24 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 07/08/2015 01:15 PM, Дмитрий Воронин wrote:
07.07.2015, 18:34, "Michael Paquier" <michael.paquier@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
Attachments:
0001-Add-function-for-SSL-extension-information-in-sslinf.patchtext/x-patch; charset=US-ASCII; name=0001-Add-function-for-SSL-extension-information-in-sslinf.patchDownload
From eabb75a8cdaa81303de8c74b8d097bf3e0138d38 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@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>vitus@cryptocom.ru</email>, Cryptocom LTD
</para>
+
+ <para>
+ Dmitry Voronin <email>carriingfate92@yandex.ru</email>
+ </para>
<para>
E-Mail of Cryptocom OpenSSL development group:
--
2.5.0
0002-Add-more-sanity-checks-in-sslinfo.patchtext/x-patch; charset=US-ASCII; name=0002-Add-more-sanity-checks-in-sslinfo.patchDownload
From d9292f649585716c37c1a31c58deee9017c51fb7 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@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
On Sun, Aug 23, 2015 at 10:21 PM, Michael Paquier wrote:
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.
And... This second patch had a stupid mistake making for example
ssl_client_dn() fail, so here they are again.
--
Michael
Attachments:
0001-Add-function-for-SSL-extension-information-in-sslinf.patchtext/x-patch; charset=US-ASCII; name=0001-Add-function-for-SSL-extension-information-in-sslinf.patchDownload
From eabb75a8cdaa81303de8c74b8d097bf3e0138d38 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@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>vitus@cryptocom.ru</email>, Cryptocom LTD
</para>
+
+ <para>
+ Dmitry Voronin <email>carriingfate92@yandex.ru</email>
+ </para>
<para>
E-Mail of Cryptocom OpenSSL development group:
--
2.5.0
0002-Add-more-sanity-checks-in-sslinfo.patchtext/x-patch; charset=US-ASCII; name=0002-Add-more-sanity-checks-in-sslinfo.patchDownload
From 0b19c86eb5e2cbfe169959298f2cc147f5a83bf1 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@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..5a1162c 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 == NULL)
+ 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
Michael Paquier wrote:
On Sun, Aug 23, 2015 at 10:21 PM, Michael Paquier wrote:
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.And... This second patch had a stupid mistake making for example
ssl_client_dn() fail, so here they are again.
What stupid mistake was this? I checked your 0002 files and they looked
identical. I just pushed it all the way back to 9.0, after changing the
elogs to ereports.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Sep 8, 2015 at 7:31 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Michael Paquier wrote:
On Sun, Aug 23, 2015 at 10:21 PM, Michael Paquier wrote:
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.And... This second patch had a stupid mistake making for example
ssl_client_dn() fail, so here they are again.What stupid mistake was this? I checked your 0002 files and they looked
identical. I just pushed it all the way back to 9.0, after changing the
elogs to ereports.
Never mind. I cannot recall either.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Michael Paquier wrote:
Note for committers: attached is a small script that will generate a
client certificate with extensions enabled. This is helpful when
testing this patch. Once created, then simply connect with something
like this connection string:
"host=127.0.0.1 sslmode=verify-full sslcert=client.crt
sslkey=client.key sslrootcert=server.crt"
By querying the new function implemented by this patch the information
about the client certificate extensions will show up.
Thanks, this was useful.
I made a couple extra cleanups to the patch, namely: do not call
CreateTemplateTupleDesc() just to discard the resulting tupdesc with a
subsequent get_call_result_type(); and do not write a \0 to the
BIO_s_mem, and instead use BIO_get_mem_data's return value as length
when converting str to text *.
And pushed.
FWIW I now think I made a mistake with the error checks that I
backpatched, because the wording of the error messages I used "failed to
foo" is frowned upon by our message style guidelines. Should be "could
not do foo" instead.
Thanks,
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Sep 8, 2015 at 9:32 AM, Alvaro Herrera wrote:
Michael Paquier wrote:
I made a couple extra cleanups to the patch, namely: do not call
CreateTemplateTupleDesc() just to discard the resulting tupdesc with a
subsequent get_call_result_type(); and do not write a \0 to the
BIO_s_mem, and instead use BIO_get_mem_data's return value as length
when converting str to text *.
Thanks!
FWIW I now think I made a mistake with the error checks that I
backpatched, because the wording of the error messages I used "failed to
foo" is frowned upon by our message style guidelines. Should be "could
not do foo" instead.
Well... Feel free to use that if it makes you gain two minutes.
--
Michael
Attachments:
20150908_sslinfo_messages.patchbinary/octet-stream; name=20150908_sslinfo_messages.patchDownload
diff --git a/contrib/sslinfo/sslinfo.c b/contrib/sslinfo/sslinfo.c
index e00f8a2..0f16cc2 100644
--- a/contrib/sslinfo/sslinfo.c
+++ b/contrib/sslinfo/sslinfo.c
@@ -149,7 +149,7 @@ ASN1_STRING_to_text(ASN1_STRING *str)
if (membuf == NULL)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("failed to create OpenSSL BIO structure")));
+ errmsg("could not create OpenSSL BIO structure")));
(void) BIO_set_close(membuf, BIO_CLOSE);
ASN1_STRING_print_ex(membuf, str,
((ASN1_STRFLGS_RFC2253 & ~ASN1_STRFLGS_ESC_MSB)
@@ -305,7 +305,7 @@ X509_NAME_to_text(X509_NAME *name)
if (membuf == NULL)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("failed to create BIO")));
+ errmsg("could not create BIO")));
(void) BIO_set_close(membuf, BIO_CLOSE);
for (i = 0; i < count; i++)
@@ -315,7 +315,7 @@ X509_NAME_to_text(X509_NAME *name)
if (nid == NID_undef)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("failed to get NID for ASN1_OBJECT object")));
+ errmsg("could not get NID for ASN1_OBJECT object")));
v = X509_NAME_ENTRY_get_data(e);
field_name = OBJ_nid2sn(nid);
if (field_name == NULL)
@@ -323,7 +323,7 @@ X509_NAME_to_text(X509_NAME *name)
if (field_name == NULL)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("failed to convert NID %d to an ASN1_OBJECT structure", nid)));
+ errmsg("could not convert 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)
@@ -523,6 +523,6 @@ ssl_extension_info(PG_FUNCTION_ARGS)
SRF_RETURN_NEXT(funcctx, result);
}
- /* Do when there is no more left */
+ /* Done when there is no more left */
SRF_RETURN_DONE(funcctx);
}
Michael Paquier wrote:
On Tue, Sep 8, 2015 at 9:32 AM, Alvaro Herrera wrote:
Michael Paquier wrote:
I made a couple extra cleanups to the patch, namely: do not call
CreateTemplateTupleDesc() just to discard the resulting tupdesc with a
subsequent get_call_result_type(); and do not write a \0 to the
BIO_s_mem, and instead use BIO_get_mem_data's return value as length
when converting str to text *.Thanks!
FWIW I now think I made a mistake with the error checks that I
backpatched, because the wording of the error messages I used "failed to
foo" is frowned upon by our message style guidelines. Should be "could
not do foo" instead.Well... Feel free to use that if it makes you gain two minutes.
Thanks. I changed the ones for BIO_free() too and pushed.
Those comments /* done if we're done */ look a bit redundant to me. I
considered just removing it altogether ...
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers