New functions in sslinfo module
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
postgresql-9.3.4-sslinfo-sslextensions.patchtext/x-diff; name=postgresql-9.3.4-sslinfo-sslextensions.patchDownload
--- contrib/sslinfo/sslinfo.c 2014-03-17 23:35:47.000000000 +0400
+++ contrib/sslinfo/sslinfo.c 2014-04-18 11:09:49.567775647 +0400
@@ -5,6 +5,8 @@
* This file is distributed under BSD-style license.
*
* contrib/sslinfo/sslinfo.c
+ *
+ * Extension functions written by Dmitry Voronin carriingfate92@yandex.ru, CNIIEISU.
*/
#include "postgres.h"
@@ -14,9 +16,11 @@
#include "miscadmin.h"
#include "utils/builtins.h"
#include "mb/pg_wchar.h"
+#include "funcapi.h"
#include <openssl/x509.h>
#include <openssl/asn1.h>
+#include <openssl/x509v3.h>
PG_MODULE_MAGIC;
@@ -35,6 +39,11 @@
Datum X509_NAME_to_text(X509_NAME *name);
Datum ASN1_STRING_to_text(ASN1_STRING *str);
+X509_EXTENSION *get_extension(X509* certificate, char *name);
+Datum ssl_get_extension_value(PG_FUNCTION_ARGS);
+Datum ssl_is_critical_extension(PG_FUNCTION_ARGS);
+Datum ssl_get_count_of_extensions(PG_FUNCTION_ARGS);
+Datum ssl_get_extension_names(PG_FUNCTION_ARGS);
/*
* Indicates whether current session uses SSL
@@ -371,3 +380,146 @@
PG_RETURN_NULL();
return X509_NAME_to_text(X509_get_issuer_name(MyProcPort->peer));
}
+
+
+X509_EXTENSION *get_extension(X509* certificate, char *name) {
+ int extension_nid = 0;
+ int locate = 0;
+
+ extension_nid = OBJ_sn2nid(name);
+ if (extension_nid == NID_undef) {
+ extension_nid = OBJ_ln2nid(name);
+ if (extension_nid == NID_undef)
+ return NULL;
+ }
+ locate = X509_get_ext_by_NID(certificate, extension_nid, -1);
+ return X509_get_ext(certificate, locate);
+}
+
+/* Returns value of extension.
+ *
+ * This function returns value of extension by short name in client certificate.
+ *
+ * Returns text datum.
+ */
+
+PG_FUNCTION_INFO_V1(ssl_get_extension_value);
+Datum
+ssl_get_extension_value(PG_FUNCTION_ARGS) {
+ X509 *certificate = MyProcPort -> peer;
+ X509_EXTENSION *extension = NULL;
+ char *extension_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ BIO *bio = NULL;
+ char *value = NULL;
+ char nullterm = '\0';
+ text *result = NULL;
+
+ if (certificate == NULL)
+ PG_RETURN_NULL();
+
+ extension = get_extension(certificate, extension_name);
+ if (extension == NULL)
+ elog(ERROR, "Extension by name \"%s\" is not found in certificate", extension_name);
+
+ bio = BIO_new(BIO_s_mem());
+ X509V3_EXT_print(bio, extension, -1, -1);
+ BIO_write(bio, &nullterm, 1);
+ BIO_get_mem_data(bio, &value);
+
+ result = cstring_to_text(value);
+ BIO_free(bio);
+
+ PG_RETURN_TEXT_P(result);
+}
+
+/* Returns status of extension
+ *
+ * Returns true, if extension is critical and false, if it is not.
+ *
+ * Returns bool datum
+ */
+PG_FUNCTION_INFO_V1(ssl_is_critical_extension);
+Datum
+ssl_is_critical_extension(PG_FUNCTION_ARGS) {
+ X509 *certificate = MyProcPort -> peer;
+ X509_EXTENSION *extension = NULL;
+ char *extension_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ int critical = 0;
+
+ if (certificate == NULL)
+ PG_RETURN_NULL();
+
+ extension = get_extension(certificate, extension_name);
+ if (extension == NULL)
+ elog(ERROR, "Extension name \"%s\" is not found in certificate", extension_name);
+
+ critical = X509_EXTENSION_get_critical(extension);
+ PG_RETURN_BOOL(critical);
+}
+
+/* Returns count of extensions in client certificate
+ *
+ * Returns int datum
+ */
+PG_FUNCTION_INFO_V1(ssl_get_count_of_extensions);
+Datum
+ssl_get_count_of_extensions(PG_FUNCTION_ARGS) {
+ X509 *certificate = MyProcPort -> peer;
+
+ if (certificate == NULL)
+ PG_RETURN_NULL();
+
+ PG_RETURN_INT32(X509_get_ext_count(certificate));
+}
+
+/* Returns short names of extensions in client certificate
+ *
+ * Returns setof text datum
+ */
+PG_FUNCTION_INFO_V1(ssl_get_extension_names);
+Datum
+ssl_get_extension_names(PG_FUNCTION_ARGS) {
+ X509 *certificate = MyProcPort -> peer;
+ FuncCallContext *funcctx;
+ STACK_OF(X509_EXTENSION) *extension_stack = NULL;
+ MemoryContext oldcontext;
+ int call = 0;
+ int max_calls = 0;
+ X509_EXTENSION *extension = NULL;
+ ASN1_OBJECT *object = NULL;
+ int extension_nid = 0;
+ text* result = NULL;
+
+ if (certificate == NULL)
+ PG_RETURN_NULL();
+
+ extension_stack = certificate -> cert_info -> extensions;
+ if (extension_stack == NULL)
+ PG_RETURN_NULL();
+
+ if (SRF_IS_FIRSTCALL()) {
+ funcctx = SRF_FIRSTCALL_INIT();
+ oldcontext = MemoryContextSwitchTo(funcctx -> multi_call_memory_ctx);
+ funcctx -> max_calls = X509_get_ext_count(certificate);
+ MemoryContextSwitchTo(oldcontext);
+ }
+ funcctx = SRF_PERCALL_SETUP();
+
+ call = funcctx -> call_cntr;
+ max_calls = funcctx -> max_calls;
+
+ if (call < max_calls) {
+ extension = sk_X509_EXTENSION_value(extension_stack, call);
+ object = X509_EXTENSION_get_object(extension);
+ extension_nid = OBJ_obj2nid(object);
+
+ if (extension_nid == NID_undef)
+ elog(ERROR, "Unknown extension in certificate");
+
+ result = cstring_to_text(OBJ_nid2sn(extension_nid));
+
+ SRF_RETURN_NEXT(funcctx, (Datum) result);
+ }
+ SRF_RETURN_DONE(funcctx);
+}
+
--- contrib/sslinfo/sslinfo--1.0.sql 2014-03-17 23:35:47.000000000 +0400
+++ contrib/sslinfo/sslinfo--1.0.sql 2014-04-18 11:12:03.768470905 +0400
@@ -38,3 +38,19 @@
CREATE FUNCTION ssl_issuer_dn() RETURNS text
AS 'MODULE_PATHNAME', 'ssl_issuer_dn'
LANGUAGE C STRICT;
+
+CREATE OR REPLACE FUNCTION ssl_get_extension_value(text) RETURNS text
+AS 'MODULE_PATHNAME', 'ssl_get_extension_value'
+LANGUAGE C STRICT;
+
+CREATE OR REPLACE FUNCTION ssl_is_critical_extension(text) RETURNS boolean
+AS 'MODULE_PATHNAME', 'ssl_is_critical_extension'
+LANGUAGE C STRICT;
+
+CREATE OR REPLACE FUNCTION ssl_get_count_of_extensions() RETURNS integer
+AS 'MODULE_PATHNAME', 'ssl_get_count_of_extensions'
+LANGUAGE C STRICT;
+
+CREATE OR REPLACE FUNCTION ssl_get_extension_names() RETURNS SETOF text
+AS 'MODULE_PATHNAME', 'ssl_get_extension_names'
+LANGUAGE C STRICT;
+
--- contrib/sslinfo/sslinfo--unpackaged--1.0.sql 2014-04-18 11:17:44.937238072 +0400
+++ contrib/sslinfo/sslinfo--unpackaged--1.0.sql 2014-04-18 11:16:45.185110931 +0400
@@ -11,6 +11,11 @@
ALTER EXTENSION sslinfo ADD function ssl_client_dn();
ALTER EXTENSION sslinfo ADD function ssl_issuer_dn();
+ALTER EXTENSION sslinfo ADD function ssl_get_extension_value();
+ALTER EXTENSION sslinfo ADD function ssl_is_critical_extension();
+ALTER EXTENSION sslinfo ADD function ssl_count_of_extensions();
+ALTER EXTENSION sslinfo ADD function ssl_get_extension_names();
+
-- These functions were not in 9.0:
CREATE FUNCTION ssl_version() RETURNS text
On Mon, Apr 21, 2014 at 1:48 PM, Воронин Дмитрий <carriingfate92@yandex.ru>
wrote:
Hello,
I make an a patch, which adds 4 functions to sslinfo extension module:
1) ssl_get_count_of_extensions() --- get count of X509v3 extensions from
client certificate;
2) ssl_get_extension_names() --- get short names of X509v3 extensions from
client certificate;
3) ssl_get_extension_value(text) --- get value of extension from
certificate
(argument --- short name of extension);
4) ssl_is_critical_extension(text) --- returns true, if extension is
critical and false, if is not (argument --- short name of extension).You can view some information of certificate's extensions via those
functions.
I want, that my functions will be included in PostgreSQL release.What do you think about it?
Please avoid creating a new thread each time you send a new version of the
same patch. Previous message was here:
/messages/by-id/1135491397673046@web9m.yandex.ru
With my previous answer here:
/messages/by-id/CAB7nPqRVFhnPnQL9ND+K=WA-YF_N1fAirx=s6fawk9F6ANLwBQ@mail.gmail.com
As I already mentioned last time, please register this patch to the
upcoming commit fest beginning in June:
https://commitfest.postgresql.org/action/commitfest_view?id=22
This way, you will be sure that your patch will get at least one fair
review and that progress will be made on the feature you are proposing.
The development cycle of 9.4 is over, but your patch could get into 9.5.
You seem as well to have developed this patch using a tarball of 9.3.4 code
by generating diffs from it, you will need a development environment with
git. Here are some guidelines you can refer to (those are the same URLs as
in my previous email btw...):
https://wiki.postgresql.org/wiki/Submitting_a_Patch
https://wiki.postgresql.org/wiki/Working_with_Git
https://wiki.postgresql.org/wiki/Creating_Clean_Patches
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
Attachments:
new-functions-sslinfo.patchtext/x-diff; name=new-functions-sslinfo.patchDownload
*** a/contrib/sslinfo/sslinfo--1.0.sql
--- b/contrib/sslinfo/sslinfo--1.0.sql
***************
*** 38,40 **** LANGUAGE C STRICT;
--- 38,56 ----
CREATE FUNCTION ssl_issuer_dn() RETURNS text
AS 'MODULE_PATHNAME', 'ssl_issuer_dn'
LANGUAGE C STRICT;
+
+ CREATE OR REPLACE FUNCTION ssl_get_extension_value(text) RETURNS text
+ AS 'MODULE_PATHNAME', 'ssl_get_extension_value'
+ LANGUAGE C STRICT;
+
+ CREATE OR REPLACE FUNCTION ssl_is_critical_extension(text) RETURNS boolean
+ AS 'MODULE_PATHNAME', 'ssl_is_critical_extension'
+ LANGUAGE C STRICT;
+
+ CREATE OR REPLACE FUNCTION ssl_get_count_of_extensions() RETURNS integer
+ AS 'MODULE_PATHNAME', 'ssl_get_count_of_extensions'
+ LANGUAGE C STRICT;
+
+ CREATE OR REPLACE FUNCTION ssl_get_extension_names() RETURNS SETOF text
+ AS 'MODULE_PATHNAME', 'ssl_get_extension_names'
+ LANGUAGE C STRICT;
\ No newline at end of file
\ No newline at end of file
*** a/contrib/sslinfo/sslinfo--unpackaged--1.0.sql
--- b/contrib/sslinfo/sslinfo--unpackaged--1.0.sql
***************
*** 11,16 **** ALTER EXTENSION sslinfo ADD function ssl_issuer_field(text);
--- 11,21 ----
ALTER EXTENSION sslinfo ADD function ssl_client_dn();
ALTER EXTENSION sslinfo ADD function ssl_issuer_dn();
+ ALTER EXTENSION sslinfo ADD function ssl_get_extension_value();
+ ALTER EXTENSION sslinfo ADD function ssl_is_critical_extension();
+ ALTER EXTENSION sslinfo ADD function ssl_count_of_extensions();
+ ALTER EXTENSION sslinfo ADD function ssl_get_extension_names();
+
-- These functions were not in 9.0:
CREATE FUNCTION ssl_version() RETURNS text
*** a/contrib/sslinfo/sslinfo.c
--- b/contrib/sslinfo/sslinfo.c
***************
*** 5,10 ****
--- 5,12 ----
* This file is distributed under BSD-style license.
*
* contrib/sslinfo/sslinfo.c
+ *
+ * Extension functions written by Dmitry Voronin carriingfate92@yandex.ru, CNIIEISU.
*/
#include "postgres.h"
***************
*** 14,31 ****
#include "miscadmin.h"
#include "utils/builtins.h"
#include "mb/pg_wchar.h"
#include <openssl/x509.h>
#include <openssl/asn1.h>
PG_MODULE_MAGIC;
! 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);
/*
* Indicates whether current session uses SSL
--- 16,49 ----
#include "miscadmin.h"
#include "utils/builtins.h"
#include "mb/pg_wchar.h"
+ #include "funcapi.h"
#include <openssl/x509.h>
#include <openssl/asn1.h>
+ #include <openssl/x509v3.h>
PG_MODULE_MAGIC;
! Datum ssl_is_used(PG_FUNCTION_ARGS);
! Datum ssl_version(PG_FUNCTION_ARGS);
! Datum ssl_cipher(PG_FUNCTION_ARGS);
! Datum ssl_client_cert_present(PG_FUNCTION_ARGS);
! Datum ssl_client_serial(PG_FUNCTION_ARGS);
! Datum ssl_client_dn_field(PG_FUNCTION_ARGS);
! Datum ssl_issuer_field(PG_FUNCTION_ARGS);
! Datum ssl_client_dn(PG_FUNCTION_ARGS);
! Datum ssl_issuer_dn(PG_FUNCTION_ARGS);
! Datum X509_NAME_field_to_text(X509_NAME *name, text *fieldName);
! Datum X509_NAME_to_text(X509_NAME *name);
! Datum ASN1_STRING_to_text(ASN1_STRING *str);
+ X509_EXTENSION *get_extension(X509* certificate, char *name);
+ Datum ssl_get_extension_value(PG_FUNCTION_ARGS);
+ Datum ssl_is_critical_extension(PG_FUNCTION_ARGS);
+ Datum ssl_get_count_of_extensions(PG_FUNCTION_ARGS);
+ Datum ssl_get_extension_names(PG_FUNCTION_ARGS);
/*
* Indicates whether current session uses SSL
***************
*** 42,48 **** ssl_is_used(PG_FUNCTION_ARGS)
/*
! * Returns SSL version currently in use.
*/
PG_FUNCTION_INFO_V1(ssl_version);
Datum
--- 60,66 ----
/*
! * Returns SSL cipher currently in use.
*/
PG_FUNCTION_INFO_V1(ssl_version);
Datum
***************
*** 68,74 **** ssl_cipher(PG_FUNCTION_ARGS)
/*
! * Indicates whether current client provided a certificate
*
* Function has no arguments. Returns bool. True if current session
* is SSL session and client certificate is verified, otherwise false.
--- 86,92 ----
/*
! * Indicates whether current client have provided a certificate
*
* Function has no arguments. Returns bool. True if current session
* is SSL session and client certificate is verified, otherwise false.
***************
*** 129,135 **** ssl_client_serial(PG_FUNCTION_ARGS)
* Returns Datum, which can be directly returned from a C language SQL
* function.
*/
! static Datum
ASN1_STRING_to_text(ASN1_STRING *str)
{
BIO *membuf;
--- 147,153 ----
* Returns Datum, which can be directly returned from a C language SQL
* function.
*/
! Datum
ASN1_STRING_to_text(ASN1_STRING *str)
{
BIO *membuf;
***************
*** 148,154 **** ASN1_STRING_to_text(ASN1_STRING *str)
nullterm = '\0';
BIO_write(membuf, &nullterm, 1);
size = BIO_get_mem_data(membuf, &sp);
! dp = pg_any_to_server(sp, size - 1, PG_UTF8);
result = cstring_to_text(dp);
if (dp != sp)
pfree(dp);
--- 166,175 ----
nullterm = '\0';
BIO_write(membuf, &nullterm, 1);
size = BIO_get_mem_data(membuf, &sp);
! dp = (char *) pg_do_encoding_conversion((unsigned char *) sp,
! size - 1,
! PG_UTF8,
! GetDatabaseEncoding());
result = cstring_to_text(dp);
if (dp != sp)
pfree(dp);
***************
*** 170,176 **** ASN1_STRING_to_text(ASN1_STRING *str)
* Returns result of ASN1_STRING_to_text applied to appropriate
* part of name
*/
! static Datum
X509_NAME_field_to_text(X509_NAME *name, text *fieldName)
{
char *string_fieldname;
--- 191,197 ----
* Returns result of ASN1_STRING_to_text applied to appropriate
* part of name
*/
! Datum
X509_NAME_field_to_text(X509_NAME *name, text *fieldName)
{
char *string_fieldname;
***************
*** 275,281 **** ssl_issuer_field(PG_FUNCTION_ARGS)
* Returns: text datum which contains string representation of
* X509_NAME
*/
! static Datum
X509_NAME_to_text(X509_NAME *name)
{
BIO *membuf = BIO_new(BIO_s_mem());
--- 296,302 ----
* Returns: text datum which contains string representation of
* X509_NAME
*/
! Datum
X509_NAME_to_text(X509_NAME *name)
{
BIO *membuf = BIO_new(BIO_s_mem());
***************
*** 310,316 **** X509_NAME_to_text(X509_NAME *name)
nullterm = '\0';
BIO_write(membuf, &nullterm, 1);
size = BIO_get_mem_data(membuf, &sp);
! dp = pg_any_to_server(sp, size - 1, PG_UTF8);
result = cstring_to_text(dp);
if (dp != sp)
pfree(dp);
--- 331,340 ----
nullterm = '\0';
BIO_write(membuf, &nullterm, 1);
size = BIO_get_mem_data(membuf, &sp);
! dp = (char *) pg_do_encoding_conversion((unsigned char *) sp,
! size - 1,
! PG_UTF8,
! GetDatabaseEncoding());
result = cstring_to_text(dp);
if (dp != sp)
pfree(dp);
***************
*** 356,358 **** ssl_issuer_dn(PG_FUNCTION_ARGS)
--- 380,525 ----
PG_RETURN_NULL();
return X509_NAME_to_text(X509_get_issuer_name(MyProcPort->peer));
}
+
+
+ X509_EXTENSION *get_extension(X509* certificate, char *name) {
+ int extension_nid = 0;
+ int locate = 0;
+
+ extension_nid = OBJ_sn2nid(name);
+ if (extension_nid == NID_undef) {
+ extension_nid = OBJ_ln2nid(name);
+ if (extension_nid == NID_undef)
+ return NULL;
+ }
+ locate = X509_get_ext_by_NID(certificate, extension_nid, -1);
+ return X509_get_ext(certificate, locate);
+ }
+
+ /* Returns value of extension.
+ *
+ * This function returns value of extension by short name in client certificate.
+ *
+ * Returns text datum.
+ */
+
+ PG_FUNCTION_INFO_V1(ssl_get_extension_value);
+ Datum
+ ssl_get_extension_value(PG_FUNCTION_ARGS) {
+ X509 *certificate = MyProcPort -> peer;
+ X509_EXTENSION *extension = NULL;
+ char *extension_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ BIO *bio = NULL;
+ char *value = NULL;
+ char nullterm = '\0';
+ text *result = NULL;
+
+ if (certificate == NULL)
+ PG_RETURN_NULL();
+
+ extension = get_extension(certificate, extension_name);
+ if (extension == NULL)
+ elog(ERROR, "Extension by name \"%s\" is not found in certificate", extension_name);
+
+ bio = BIO_new(BIO_s_mem());
+ X509V3_EXT_print(bio, extension, -1, -1);
+ BIO_write(bio, &nullterm, 1);
+ BIO_get_mem_data(bio, &value);
+
+ result = cstring_to_text(value);
+ BIO_free(bio);
+
+ PG_RETURN_TEXT_P(result);
+ }
+
+ /* Returns status of extension
+ *
+ * Returns true, if extension is critical and false, if it is not.
+ *
+ * Returns bool datum
+ */
+ PG_FUNCTION_INFO_V1(ssl_is_critical_extension);
+ Datum
+ ssl_is_critical_extension(PG_FUNCTION_ARGS) {
+ X509 *certificate = MyProcPort -> peer;
+ X509_EXTENSION *extension = NULL;
+ char *extension_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ int critical = 0;
+
+ if (certificate == NULL)
+ PG_RETURN_NULL();
+
+ extension = get_extension(certificate, extension_name);
+ if (extension == NULL)
+ elog(ERROR, "Extension name \"%s\" is not found in certificate", extension_name);
+
+ critical = X509_EXTENSION_get_critical(extension);
+ PG_RETURN_BOOL(critical);
+ }
+
+ /* Returns count of extensions in client certificate
+ *
+ * Returns int datum
+ */
+ PG_FUNCTION_INFO_V1(ssl_get_count_of_extensions);
+ Datum
+ ssl_get_count_of_extensions(PG_FUNCTION_ARGS) {
+ X509 *certificate = MyProcPort -> peer;
+
+ if (certificate == NULL)
+ PG_RETURN_NULL();
+
+ PG_RETURN_INT32(X509_get_ext_count(certificate));
+ }
+
+ /* Returns short names of extensions in client certificate
+ *
+ * Returns setof text datum
+ */
+ PG_FUNCTION_INFO_V1(ssl_get_extension_names);
+ Datum
+ ssl_get_extension_names(PG_FUNCTION_ARGS) {
+ X509 *certificate = MyProcPort -> peer;
+ FuncCallContext *funcctx;
+ STACK_OF(X509_EXTENSION) *extension_stack = NULL;
+ MemoryContext oldcontext;
+ int call = 0;
+ int max_calls = 0;
+ X509_EXTENSION *extension = NULL;
+ ASN1_OBJECT *object = NULL;
+ int extension_nid = 0;
+ text* result = NULL;
+
+ if (certificate == NULL)
+ PG_RETURN_NULL();
+
+ extension_stack = certificate -> cert_info -> extensions;
+ if (extension_stack == NULL)
+ PG_RETURN_NULL();
+
+ if (SRF_IS_FIRSTCALL()) {
+ funcctx = SRF_FIRSTCALL_INIT();
+ oldcontext = MemoryContextSwitchTo(funcctx -> multi_call_memory_ctx);
+ funcctx -> max_calls = X509_get_ext_count(certificate);
+ MemoryContextSwitchTo(oldcontext);
+ }
+ funcctx = SRF_PERCALL_SETUP();
+
+ call = funcctx -> call_cntr;
+ max_calls = funcctx -> max_calls;
+
+ if (call < max_calls) {
+ extension = sk_X509_EXTENSION_value(extension_stack, call);
+ object = X509_EXTENSION_get_object(extension);
+ extension_nid = OBJ_obj2nid(object);
+
+ if (extension_nid == NID_undef)
+ elog(ERROR, "Unknown extension in certificate");
+
+ result = cstring_to_text(OBJ_nid2sn(extension_nid));
+
+ SRF_RETURN_NEXT(funcctx, (Datum) result);
+ }
+ SRF_RETURN_DONE(funcctx);
+ }
+
On Mon, Apr 21, 2014 at 2:51 PM, Воронин Дмитрий
<carriingfate92@yandex.ru>wrote:
I put patch generated on git diffs to this letter. I make an a thread in
postgresql commit fest:
https://commitfest.postgresql.org/action/patch_view?id=1438
Thanks!
--
Michael
On 04/21/2014 07:51 AM, О©╫О©╫О©╫О©╫О©╫О©╫О©╫ О©╫О©╫О©╫О©╫О©╫О©╫О©╫ wrote:
I put patch generated on git diffs to this letter. I make an a thread in
postgresql commit fest:
https://commitfest.postgresql.org/action/patch_view?id=1438
Thanks for the patch, it seems like a useful feature.
=== General ===
- Applies cleanly to HEAD and compiles without warnings.
- The test suite passes.
- x509v3 support was added in OpenSSL 0.9.2 and sslinfo already depends
heavily on OpenSSL so no new dependencies.
=== User functionality ===
- If you are a user of the sslinfo extension the new functions should be
useful additions.
- I tested the code without SSL, with certificate but without client
certificate, with client certificates first without extensions and the
with. All of this worked fine except for some usability which could be
improved, see below.
- I cannot see the use for ssl_get_count_of_extensions(). When would
anyone need to know the number of extensions? I think this is an
implementation detail of OpenSSL which we do not need to expose. If any
user wants this feature he can count the extension names.
- Documentation is missing for the new functions.
- I think the names of the new functions should be change. Below are my
suggestions. Other suggestions are welcome.
* ssl_extension_value(text)
* ssl_extension_is_critical()
* ssl_extension_names()
* ssl_extension_count() (If we should keep it.)
- Would it be interesting to have a set returning function which returns
all extensions with both the names and the values? Like the below.
$ SELECT * FROM ssl_extensions();
name | value
------------------+------------------------------------------------------
basicConstraints | CA:FALSE
keyUsage | Digital Signature, Non Repudiation, Key Encipherment
(2 rows)
- I do not think that ssl_get_extension_names() should return a single
row with NULL when the certificate has no extensions or when there is no
certificate at all. Returning zero rows in this case should make it
easier to use.
- Currently ssl_is_critical_extension() and ssl_get_extension_value()
throw an error when the requested extension is not in the certificate.
I am not sure if I like this behavior. I think I would prefer if the
code always throws an error when name lookup fails, and never when it is
successful. For a valid extension name I think NULL should be returned
when it does not exist.
=== Code review: main ===
- Why are X509_NAME_field_to_text(), X509_NAME_to_text(),
ASN1_STRING_to_text() and get_extension() not static? None of these are
a symbol which should be exported.
- I have not worked with extension myself, but since your change adds
functions to the extension I think you need to create a version 1.1
instead of modifying 1.0 in place. If so you also need to write an
upgrade script from 1.0 to 1.1. See dblink--1.0--1.1.sql for an example.
- Please break out the comment fix for ssl_cipher() into a separate patch.
- Why do you use pg_do_encoding_conversion() over pg_any_to_server()?
pg_any_to_server() is implemented using pg_do_encoding_conversion().
- I think you should use OBJ_txt2nid() rather than OBJ_sn2nid() +
OBJ_ln2nid(). You should probably also use OBJ_txt2obj() since
X509_get_ext_by_NID() will call OBJ_nid2obj() anyway.
- You should free the extension_name string. I do not think it is ok to
leak it to the end of the query.
- I think you need to convert the extension values and names to the
server encoding. I just wonder if we need to support data which is
incorrectly encoded.
=== Code review: style issues ===
- Trailing whitespace in sslinfo--1.0.sql and sslinfo.c.q
- sslinfo--1.0.sql does not end in a newline.
- I do not think the PostgreSQL project adds authors in the top comment
of files in cases like this. Authors get credit in the commit messages.
- I think you can remove the prototypes of all the ssl_* functions.
- Adding the have in "Indicates whether current client have provided a
certificate" is not necessary. The previous wording looks correct to my
non-native speaker eyes.
- Too much white space in variable declarations in get_extension().
- Extra space before -1 in "X509_get_ext_by_NID(certificate,
extension_nid, -1);"
- Please do not initialize variables unless necessary. Compilers are
pretty good at warning about uninitialized usage. For example both
locate and extension_nid do not need to be initialized.
- Remove empty line directly before ssl_get_extension_value().
- Try to follow variable naming conventions from other functions (e.g.
use nid rather than extension_nid, membuf rather than bio, sp rather
than value).
- I am pretty sure the variable you call locate should be called
location (or loc for short).
- There should not be any spaces around "->".
- The declaration of *extension in ssl_get_extension_value is not
aligned properly.
- Remove white space in variable declaration in
ssl_get_count_of_extensions().
- Incorrectly alignment of variable declarations in
ssl_get_extension_names().
- All the "Returns X datum" comments look redundant to me, but this is a
matter of preference.
- The star when declaring result in ssl_get_extension_names() should be
put on the other side of the white space.
--
Andreas Karlsson
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
<p>24.06.2014, 00:07, "Andreas Karlsson" <<a href="mailto:andreas@proxel.se">andreas@proxel.se</a>>:</p><blockquote> On 04/21/2014 07:51 AM, Воронин Дмитрий wrote:<br /><blockquote> I put patch generated on git diffs to this letter. I make an a thread in<br /> postgresql commit fest:<br /> <a href="https://commitfest.postgresql.org/action/patch_view?id=1438">https://commitfest.postgresql.org/action/patch_view?id=1438</a></blockquote> Thanks for the patch, it seems like a useful feature.<br /><br /> === General ===<br /><br /> - Applies cleanly to HEAD and compiles without warnings.<br /><br /> - The test suite passes.<br /><br /> - x509v3 support was added in OpenSSL 0.9.2 and sslinfo already depends<br /> heavily on OpenSSL so no new dependencies.<br /><br /> === User functionality ===<br /><br /> - If you are a user of the sslinfo extension the new functions should be<br /> useful additions.<br /><br /> - I tested the code without SSL, with certificate but without client<br /> certificate, with client certificates first without extensions and the<br /> with. All of this worked fine except for some usability which could be<br /> improved, see below.<br /><br /> - I cannot see the use for ssl_get_count_of_extensions(). When would<br /> anyone need to know the number of extensions? I think this is an<br /> implementation detail of OpenSSL which we do not need to expose. If any<br /> user wants this feature he can count the extension names.<br /><br /> - Documentation is missing for the new functions.<br /><br /> - I think the names of the new functions should be change. Below are my<br /> suggestions. Other suggestions are welcome.<br /><br /> * ssl_extension_value(text)<br /> * ssl_extension_is_critical()<br /> * ssl_extension_names()<br /> * ssl_extension_count() (If we should keep it.)<br /><br /> - Would it be interesting to have a set returning function which returns<br /> all extensions with both the names and the values? Like the below.<br /><br /> $ SELECT * FROM ssl_extensions();<br /> name | value<br /> ------------------+------------------------------------------------------<br /> basicConstraints | CA:FALSE<br /> keyUsage | Digital Signature, Non Repudiation, Key Encipherment<br /> (2 rows)<br /><br /> - I do not think that ssl_get_extension_names() should return a single<br /> row with NULL when the certificate has no extensions or when there is no<br /> certificate at all. Returning zero rows in this case should make it<br /> easier to use.<br /><br /> - Currently ssl_is_critical_extension() and ssl_get_extension_value()<br /> throw an error when the requested extension is not in the certificate.<br /><br /> I am not sure if I like this behavior. I think I would prefer if the<br /> code always throws an error when name lookup fails, and never when it is<br /> successful. For a valid extension name I think NULL should be returned<br /> when it does not exist.<br /><br /> === Code review: main ===<br /><br /> - Why are X509_NAME_field_to_text(), X509_NAME_to_text(),<br /> ASN1_STRING_to_text() and get_extension() not static? None of these are<br /> a symbol which should be exported.<br /><br /> - I have not worked with extension myself, but since your change adds<br /> functions to the extension I think you need to create a version 1.1<br /> instead of modifying 1.0 in place. If so you also need to write an<br /> upgrade script from 1.0 to 1.1. See dblink--1.0--1.1.sql for an example.<br /><br /> - Please break out the comment fix for ssl_cipher() into a separate patch.<br /><br /> - Why do you use pg_do_encoding_conversion() over pg_any_to_server()?<br /> pg_any_to_server() is implemented using pg_do_encoding_conversion().<br /><br /> - I think you should use OBJ_txt2nid() rather than OBJ_sn2nid() +<br /> OBJ_ln2nid(). You should probably also use OBJ_txt2obj() since<br /> X509_get_ext_by_NID() will call OBJ_nid2obj() anyway.<br /><br /> - You should free the extension_name string. I do not think it is ok to<br /> leak it to the end of the query.<br /><br /> - I think you need to convert the extension values and names to the<br /> server encoding. I just wonder if we need to support data which is<br /> incorrectly encoded.<br /><br /> === Code review: style issues ===<br /><br /> - Trailing whitespace in sslinfo--1.0.sql and sslinfo.c.q<br /><br /> - sslinfo--1.0.sql does not end in a newline.<br /><br /> - I do not think the PostgreSQL project adds authors in the top comment<br /> of files in cases like this. Authors get credit in the commit messages.<br /><br /> - I think you can remove the prototypes of all the ssl_* functions.<br /><br /> - Adding the have in "Indicates whether current client have provided a<br /> certificate" is not necessary. The previous wording looks correct to my<br /> non-native speaker eyes.<br /><br /> - Too much white space in variable declarations in get_extension().<br /><br /> - Extra space before -1 in "X509_get_ext_by_NID(certificate,<br /> extension_nid, -1);"<br /><br /> - Please do not initialize variables unless necessary. Compilers are<br /> pretty good at warning about uninitialized usage. For example both<br /> locate and extension_nid do not need to be initialized.<br /><br /> - Remove empty line directly before ssl_get_extension_value().<br /><br /> - Try to follow variable naming conventions from other functions (e.g.<br /> use nid rather than extension_nid, membuf rather than bio, sp rather<br /> than value).<br /><br /> - I am pretty sure the variable you call locate should be called<br /> location (or loc for short).<br /><br /> - There should not be any spaces around "->".<br /><br /> - The declaration of *extension in ssl_get_extension_value is not<br /> aligned properly.<br /><br /> - Remove white space in variable declaration in<br /> ssl_get_count_of_extensions().<br /><br /> - Incorrectly alignment of variable declarations in<br /> ssl_get_extension_names().<br /><br /> - All the "Returns X datum" comments look redundant to me, but this is a<br /> matter of preference.<br /><br /> - The star when declaring result in ssl_get_extension_names() should be<br /> put on the other side of the white space.<br /><br /> --<br /> Andreas Karlsson</blockquote><p>Hello, Andreas!<br /><br /><span lang="en"><span>I apologize, that I am writing this message today. Thank you for testing my patch!<br /><br />About user functionality</span></span>. <br />I rename my functions, your suggestions are seemed great. When I wrote those functions, I created names like others in sslinfo extension. OK, I will rename it.<br /><br />About ssl_get_extension_count(). I will delete this function. <br />I will modify functions ssl_extensions(), that it returns a set (key, value). Could you get me an example of code those function? <br /><br /> >>> - Why are X509_NAME_field_to_text(), X509_NAME_to_text(), ASN1_STRING_to_text() and get_extension() not static? None of these are a symbol which should be exported.<br />>>> Why do you use pg_do_encoding_conversion() over pg_any_to_server()? pg_any_to_server() is implemented using pg_do_encoding_conversion().<br /><br />I don't write a code of those functions and I can't answer on your question.</p><p>I will fix your notes and create a new patch version. Thank you.<br />--</p><p>Best regards, Dmitry Voronin</p>
Oh, how can I write a documentation for my functions?
02.07.2014, 16:17, "О©╫О©╫О©╫О©╫О©╫О©╫О©╫ О©╫О©╫О©╫О©╫О©╫О©╫О©╫" <carriingfate92@yandex.ru>:
24.06.2014, 00:07, "Andreas Karlsson" <andreas@proxel.se>:
О©╫On 04/21/2014 07:51 AM, О©╫О©╫О©╫О©╫О©╫О©╫О©╫ О©╫О©╫О©╫О©╫О©╫О©╫О©╫ wrote:
О©╫О©╫I put patch generated on git diffs to this letter. I make an a thread in
О©╫О©╫postgresql commit fest:
О©╫О©╫https://commitfest.postgresql.org/action/patch_view?id=1438О©╫Thanks for the patch, it seems like a useful feature.
О©╫=== General ===
О©╫- Applies cleanly to HEAD and compiles without warnings.
О©╫- The test suite passes.
О©╫- x509v3 support was added in OpenSSL 0.9.2 and sslinfo already depends
О©╫heavily on OpenSSL so no new dependencies.О©╫=== User functionality ===
О©╫- If you are a user of the sslinfo extension the new functions should be
О©╫useful additions.О©╫- I tested the code without SSL, with certificate but without client
О©╫certificate, with client certificates first without extensions and the
О©╫with. All of this worked fine except for some usability which could be
О©╫improved, see below.О©╫- I cannot see the use for ssl_get_count_of_extensions(). When would
О©╫anyone need to know the number of extensions? I think this is an
О©╫implementation detail of OpenSSL which we do not need to expose. If any
О©╫user wants this feature he can count the extension names.О©╫- Documentation is missing for the new functions.
О©╫- I think the names of the new functions should be change. Below are my
О©╫suggestions. Other suggestions are welcome.О©╫* ssl_extension_value(text)
О©╫* ssl_extension_is_critical()
О©╫* ssl_extension_names()
О©╫* ssl_extension_count() (If we should keep it.)О©╫- Would it be interesting to have a set returning function which returns
О©╫all extensions with both the names and the values? Like the below.О©╫$ SELECT * FROM ssl_extensions();
О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫name О©╫О©╫О©╫О©╫О©╫О©╫| О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫value
О©╫------------------+------------------------------------------------------
О©╫О©╫О©╫basicConstraints | CA:FALSE
О©╫О©╫О©╫keyUsage О©╫О©╫О©╫О©╫О©╫О©╫О©╫О©╫| Digital Signature, Non Repudiation, Key Encipherment
О©╫(2 rows)О©╫- I do not think that ssl_get_extension_names() should return a single
О©╫row with NULL when the certificate has no extensions or when there is no
О©╫certificate at all. Returning zero rows in this case should make it
О©╫easier to use.О©╫- Currently ssl_is_critical_extension() and ssl_get_extension_value()
О©╫throw an error when the requested extension is not in the certificate.О©╫I am not sure if I like this behavior. I think I would prefer if the
О©╫code always throws an error when name lookup fails, and never when it is
О©╫successful. For a valid extension name I think NULL should be returned
О©╫when it does not exist.О©╫=== Code review: main ===
О©╫- Why are X509_NAME_field_to_text(), X509_NAME_to_text(),
О©╫ASN1_STRING_to_text() and get_extension() not static? None of these are
О©╫a symbol which should be exported.О©╫- I have not worked with extension myself, but since your change adds
О©╫functions to the extension I think you need to create a version 1.1
О©╫instead of modifying 1.0 in place. If so you also need to write an
О©╫upgrade script from 1.0 to 1.1. See dblink--1.0--1.1.sql for an example.О©╫- Please break out the comment fix for ssl_cipher() into a separate patch.
О©╫- Why do you use pg_do_encoding_conversion() over pg_any_to_server()?
О©╫pg_any_to_server() is implemented using pg_do_encoding_conversion().О©╫- I think you should use OBJ_txt2nid() rather than OBJ_sn2nid() +
О©╫OBJ_ln2nid(). You should probably also use OBJ_txt2obj() since
О©╫X509_get_ext_by_NID() will call OBJ_nid2obj() anyway.О©╫- You should free the extension_name string. I do not think it is ok to
О©╫leak it to the end of the query.О©╫- I think you need to convert the extension values and names to the
О©╫server encoding. I just wonder if we need to support data which is
О©╫incorrectly encoded.О©╫=== Code review: style issues ===
О©╫- Trailing whitespace in sslinfo--1.0.sql and sslinfo.c.q
О©╫- sslinfo--1.0.sql does not end in a newline.
О©╫- I do not think the PostgreSQL project adds authors in the top comment
О©╫of files in cases like this. Authors get credit in the commit messages.О©╫- I think you can remove the prototypes of all the ssl_* functions.
О©╫- Adding the have in "Indicates whether current client have provided a
О©╫certificate" is not necessary. The previous wording looks correct to my
О©╫non-native speaker eyes.О©╫- Too much white space in variable declarations in get_extension().
О©╫- Extra space before -1 in "X509_get_ext_by_NID(certificate,
О©╫extension_nid, О©╫-1);"О©╫- Please do not initialize variables unless necessary. Compilers are
О©╫pretty good at warning about uninitialized usage. For example both
О©╫locate and extension_nid do not need to be initialized.О©╫- Remove empty line directly before ssl_get_extension_value().
О©╫- Try to follow variable naming conventions from other functions (e.g.
О©╫use nid rather than extension_nid, membuf rather than bio, sp rather
О©╫than value).О©╫- I am pretty sure the variable you call locate should be called
О©╫location (or loc for short).О©╫- There should not be any spaces around "->".
О©╫- The declaration of *extension in ssl_get_extension_value is not
О©╫aligned properly.О©╫- Remove white space in variable declaration in
О©╫ssl_get_count_of_extensions().О©╫- Incorrectly alignment of variable declarations in
О©╫ssl_get_extension_names().О©╫- All the "Returns X datum" comments look redundant to me, but this is a
О©╫matter of preference.О©╫- The star when declaring result in ssl_get_extension_names() should be
О©╫put on the other side of the white space.О©╫--
О©╫Andreas KarlssonHello, Andreas!
I apologize, that I am writing this message today. Thank you for testing my patch!
About user functionality.
I rename my functions, your suggestions are seemed great. When I wrote those functions, I created names like others in sslinfo extension. OK, I will rename it.About ssl_get_extension_count(). I will delete this function.
I will modify functions ssl_extensions(), that it returns a set (key, value). Could you get me an example of code those function?О©╫>>> - Why are X509_NAME_field_to_text(), X509_NAME_to_text(), ASN1_STRING_to_text() and get_extension() not static? None of these are a symbol which should be exported.
Why do you use pg_do_encoding_conversion() over pg_any_to_server()? pg_any_to_server() is implemented using pg_do_encoding_conversion().
I don't write a code of those functions and I can't answer on your question.
I will fix your notes and create a new patch version. Thank you.
--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 2, 2014 at 9:19 PM, Воронин Дмитрий
<carriingfate92@yandex.ru> wrote:
Oh, how can I write a documentation for my functions?
You will need to edit the sgml documentation and to include the diffs
in your patch. Hence in your case simply list the new functions and a
description of what they do in doc/src/sgml/sslinfo.sgml.
--
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 07/02/2014 02:17 PM, О©╫О©╫О©╫О©╫О©╫О©╫О©╫ О©╫О©╫О©╫О©╫О©╫О©╫О©╫ wrote:
I apologize, that I am writing this message today. Thank you for testing
my patch!
You are welcome!
I will modify functions ssl_extensions(), that it returns a set (key,
value). Could you get me an example of code those function?
You can look at hstore_each() in hstore_op.c.
- Why are X509_NAME_field_to_text(), X509_NAME_to_text(),
ASN1_STRING_to_text() and get_extension() not static? None of these are
a symbol which should be exported.Why do you use pg_do_encoding_conversion() over pg_any_to_server()?
pg_any_to_server() is implemented using pg_do_encoding_conversion().
I don't write a code of those functions and I can't answer on your question.
Hm, I thought I saw them changed from static to not in the diff after
applying your patch. Maybe I just misread the patch.
--
Andreas Karlsson
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
postgresql-9.3.4-sslinfo-sslextensions-2.0.patchtext/x-diff; name=postgresql-9.3.4-sslinfo-sslextensions-2.0.patchDownload
*** a/contrib/sslinfo/sslinfo.c
--- b/contrib/sslinfo/sslinfo.c
***************
*** 5,10 ****
--- 5,12 ----
* This file is distributed under BSD-style license.
*
* contrib/sslinfo/sslinfo.c
+ *
+ * Extension functions written by Dmitry Voronin carriingfate92@yandex.ru, CNIIEISU.
*/
#include "postgres.h"
***************
*** 14,29 ****
#include "miscadmin.h"
#include "utils/builtins.h"
#include "mb/pg_wchar.h"
#include <openssl/x509.h>
#include <openssl/asn1.h>
PG_MODULE_MAGIC;
- 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);
/*
* Indicates whether current session uses SSL
--- 16,49 ----
#include "miscadmin.h"
#include "utils/builtins.h"
#include "mb/pg_wchar.h"
+ #include "funcapi.h"
#include <openssl/x509.h>
#include <openssl/asn1.h>
+ #include <openssl/x509v3.h>
+
PG_MODULE_MAGIC;
+ Datum ssl_is_used(PG_FUNCTION_ARGS);
+ Datum ssl_version(PG_FUNCTION_ARGS);
+ Datum ssl_cipher(PG_FUNCTION_ARGS);
+ Datum ssl_client_cert_present(PG_FUNCTION_ARGS);
+ Datum ssl_client_serial(PG_FUNCTION_ARGS);
+ Datum ssl_client_dn_field(PG_FUNCTION_ARGS);
+ Datum ssl_issuer_field(PG_FUNCTION_ARGS);
+ Datum ssl_client_dn(PG_FUNCTION_ARGS);
+ Datum ssl_issuer_dn(PG_FUNCTION_ARGS);
+ Datum X509_NAME_field_to_text(X509_NAME *name, text *fieldName);
+ Datum X509_NAME_to_text(X509_NAME *name);
+ Datum ASN1_STRING_to_text(ASN1_STRING *str);
+
+ X509_EXTENSION *get_extension(X509* certificate, char *name);
+ Datum ssl_get_extension_value(PG_FUNCTION_ARGS);
+ Datum ssl_is_critical_extension(PG_FUNCTION_ARGS);
+ Datum ssl_get_count_of_extensions(PG_FUNCTION_ARGS);
+ Datum ssl_get_extension_names(PG_FUNCTION_ARGS);
/*
* Indicates whether current session uses SSL
***************
*** 40,46 **** ssl_is_used(PG_FUNCTION_ARGS)
/*
! * Returns SSL version currently in use.
*/
PG_FUNCTION_INFO_V1(ssl_version);
Datum
--- 60,66 ----
/*
! * Returns SSL cipher currently in use.
*/
PG_FUNCTION_INFO_V1(ssl_version);
Datum
***************
*** 66,72 **** ssl_cipher(PG_FUNCTION_ARGS)
/*
! * Indicates whether current client provided a certificate
*
* Function has no arguments. Returns bool. True if current session
* is SSL session and client certificate is verified, otherwise false.
--- 86,92 ----
/*
! * Indicates whether current client have provided a certificate
*
* Function has no arguments. Returns bool. True if current session
* is SSL session and client certificate is verified, otherwise false.
***************
*** 121,133 **** ssl_client_serial(PG_FUNCTION_ARGS)
* current database encoding if possible. Any invalid characters are
* replaced by question marks.
*
! * Parameter: str - OpenSSL ASN1_STRING structure. Memory management
* of this structure is responsibility of caller.
*
* Returns Datum, which can be directly returned from a C language SQL
* function.
*/
! static Datum
ASN1_STRING_to_text(ASN1_STRING *str)
{
BIO *membuf;
--- 141,153 ----
* current database encoding if possible. Any invalid characters are
* replaced by question marks.
*
! * Parameter: str - OpenSSL ASN1_STRING structure. Memory management
* of this structure is responsibility of caller.
*
* Returns Datum, which can be directly returned from a C language SQL
* function.
*/
! Datum
ASN1_STRING_to_text(ASN1_STRING *str)
{
BIO *membuf;
***************
*** 146,152 **** ASN1_STRING_to_text(ASN1_STRING *str)
nullterm = '\0';
BIO_write(membuf, &nullterm, 1);
size = BIO_get_mem_data(membuf, &sp);
! dp = pg_any_to_server(sp, size - 1, PG_UTF8);
result = cstring_to_text(dp);
if (dp != sp)
pfree(dp);
--- 166,175 ----
nullterm = '\0';
BIO_write(membuf, &nullterm, 1);
size = BIO_get_mem_data(membuf, &sp);
! dp = (char *) pg_do_encoding_conversion((unsigned char *) sp,
! size - 1,
! PG_UTF8,
! GetDatabaseEncoding());
result = cstring_to_text(dp);
if (dp != sp)
pfree(dp);
***************
*** 168,174 **** ASN1_STRING_to_text(ASN1_STRING *str)
* Returns result of ASN1_STRING_to_text applied to appropriate
* part of name
*/
! static Datum
X509_NAME_field_to_text(X509_NAME *name, text *fieldName)
{
char *string_fieldname;
--- 191,197 ----
* Returns result of ASN1_STRING_to_text applied to appropriate
* part of name
*/
! Datum
X509_NAME_field_to_text(X509_NAME *name, text *fieldName)
{
char *string_fieldname;
***************
*** 273,279 **** ssl_issuer_field(PG_FUNCTION_ARGS)
* Returns: text datum which contains string representation of
* X509_NAME
*/
! static Datum
X509_NAME_to_text(X509_NAME *name)
{
BIO *membuf = BIO_new(BIO_s_mem());
--- 296,302 ----
* Returns: text datum which contains string representation of
* X509_NAME
*/
! Datum
X509_NAME_to_text(X509_NAME *name)
{
BIO *membuf = BIO_new(BIO_s_mem());
***************
*** 308,314 **** X509_NAME_to_text(X509_NAME *name)
nullterm = '\0';
BIO_write(membuf, &nullterm, 1);
size = BIO_get_mem_data(membuf, &sp);
! dp = pg_any_to_server(sp, size - 1, PG_UTF8);
result = cstring_to_text(dp);
if (dp != sp)
pfree(dp);
--- 331,340 ----
nullterm = '\0';
BIO_write(membuf, &nullterm, 1);
size = BIO_get_mem_data(membuf, &sp);
! dp = (char *) pg_do_encoding_conversion((unsigned char *) sp,
! size - 1,
! PG_UTF8,
! GetDatabaseEncoding());
result = cstring_to_text(dp);
if (dp != sp)
pfree(dp);
***************
*** 354,356 **** ssl_issuer_dn(PG_FUNCTION_ARGS)
--- 380,548 ----
PG_RETURN_NULL();
return X509_NAME_to_text(X509_get_issuer_name(MyProcPort->peer));
}
+
+
+ /*
+ * Returns extension object by given certificate and it's name.
+ *
+ * Returns X509_EXTENSION* or NULL, if extension is not found in certificate.
+ */
+ X509_EXTENSION *get_extension(X509* cert, char *name) {
+ int nid;
+ int loc;
+
+ nid = OBJ_txt2nid(name);
+ if (nid == NID_undef)
+ return NULL;
+
+ loc = X509_get_ext_by_NID(cert, nid, -1);
+ return X509_get_ext(cert, loc);
+ }
+
+
+ /* 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';
+ text *result = NULL;
+
+ if (cert == NULL)
+ PG_RETURN_NULL();
+
+ if (OBJ_txt2nid(ext_name) == NID_undef)
+ elog(ERROR, "Unknown extension name \"%s\"", ext_name);
+
+ ext = get_extension(cert, ext_name);
+ if (ext == NULL)
+ PG_RETURN_NULL();
+
+ membuf = BIO_new(BIO_s_mem());
+ X509V3_EXT_print(membuf, ext, -1, -1);
+ BIO_write(membuf, &nullterm, 1);
+ BIO_get_mem_data(membuf, &val);
+
+ result = cstring_to_text(val);
+ BIO_free(membuf);
+
+ PG_RETURN_TEXT_P(result);
+ }
+
+
+ /* 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;
+
+ if (cert == NULL)
+ PG_RETURN_NULL();
+
+ if (OBJ_txt2nid(ext_name) == NID_undef)
+ elog(ERROR, "Unknown extension name \"%s\"", ext_name);
+
+ ext = get_extension(cert, ext_name);
+ if (ext == NULL)
+ PG_RETURN_NULL();
+
+ critical = X509_EXTENSION_get_critical(ext);
+
+ PG_RETURN_BOOL(critical);
+ }
+
+
+ /* Returns short names of extensions in client certificate
+ *
+ * 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 (cert == NULL)
+ PG_RETURN_NULL();
+
+ ext_stack = cert->cert_info->extensions;
+ if (ext_stack == NULL)
+ PG_RETURN_NULL();
+
+ if (SRF_IS_FIRSTCALL()) {
+ funcctx = SRF_FIRSTCALL_INIT();
+ oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+ 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 *));
+
+ ext = sk_X509_EXTENSION_value(ext_stack, call);
+ obj = X509_EXTENSION_get_object(ext);
+ nid = OBJ_obj2nid(obj);
+
+ if (nid == NID_undef)
+ elog(ERROR, "Unknown extension in certificate");
+
+ values[0] = (char *) OBJ_nid2sn(nid);
+
+ membuf = BIO_new(BIO_s_mem());
+ X509V3_EXT_print(membuf, ext, -1, -1);
+ BIO_write(membuf, &nullterm, 1);
+ BIO_get_mem_data(membuf, &values[1]);
+
+ 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
***************
*** 24,34 ****
<variablelist>
<varlistentry>
<term>
<function>ssl_is_used() returns boolean</function>
- <indexterm>
- <primary>ssl_is_used</primary>
- </indexterm>
</term>
<listitem>
<para>
--- 24,34 ----
<variablelist>
<varlistentry>
+ <indexterm>
+ <primary>ssl_is_used</primary>
+ </indexterm>
<term>
<function>ssl_is_used() returns boolean</function>
</term>
<listitem>
<para>
***************
*** 39,49 ****
</varlistentry>
<varlistentry>
<term>
<function>ssl_version() returns text</function>
- <indexterm>
- <primary>ssl_version</primary>
- </indexterm>
</term>
<listitem>
<para>
--- 39,49 ----
</varlistentry>
<varlistentry>
+ <indexterm>
+ <primary>ssl_version</primary>
+ </indexterm>
<term>
<function>ssl_version() returns text</function>
</term>
<listitem>
<para>
***************
*** 54,64 ****
</varlistentry>
<varlistentry>
<term>
<function>ssl_cipher() returns text</function>
- <indexterm>
- <primary>ssl_cipher</primary>
- </indexterm>
</term>
<listitem>
<para>
--- 54,64 ----
</varlistentry>
<varlistentry>
+ <indexterm>
+ <primary>ssl_cipher</primary>
+ </indexterm>
<term>
<function>ssl_cipher() returns text</function>
</term>
<listitem>
<para>
***************
*** 69,79 ****
</varlistentry>
<varlistentry>
<term>
<function>ssl_client_cert_present() returns boolean</function>
- <indexterm>
- <primary>ssl_client_cert_present</primary>
- </indexterm>
</term>
<listitem>
<para>
--- 69,79 ----
</varlistentry>
<varlistentry>
+ <indexterm>
+ <primary>ssl_client_cert_present</primary>
+ </indexterm>
<term>
<function>ssl_client_cert_present() returns boolean</function>
</term>
<listitem>
<para>
***************
*** 85,95 ****
</varlistentry>
<varlistentry>
<term>
<function>ssl_client_serial() returns numeric</function>
- <indexterm>
- <primary>ssl_client_serial</primary>
- </indexterm>
</term>
<listitem>
<para>
--- 85,95 ----
</varlistentry>
<varlistentry>
+ <indexterm>
+ <primary>ssl_client_serial</primary>
+ </indexterm>
<term>
<function>ssl_client_serial() returns numeric</function>
</term>
<listitem>
<para>
***************
*** 109,119 ****
</varlistentry>
<varlistentry>
<term>
<function>ssl_client_dn() returns text</function>
- <indexterm>
- <primary>ssl_client_dn</primary>
- </indexterm>
</term>
<listitem>
<para>
--- 109,119 ----
</varlistentry>
<varlistentry>
+ <indexterm>
+ <primary>ssl_client_dn</primary>
+ </indexterm>
<term>
<function>ssl_client_dn() returns text</function>
</term>
<listitem>
<para>
***************
*** 132,142 ****
</varlistentry>
<varlistentry>
<term>
<function>ssl_issuer_dn() returns text</function>
- <indexterm>
- <primary>ssl_issuer_dn</primary>
- </indexterm>
</term>
<listitem>
<para>
--- 132,142 ----
</varlistentry>
<varlistentry>
+ <indexterm>
+ <primary>ssl_issuer_dn</primary>
+ </indexterm>
<term>
<function>ssl_issuer_dn() returns text</function>
</term>
<listitem>
<para>
***************
*** 157,167 ****
</varlistentry>
<varlistentry>
<term>
<function>ssl_client_dn_field(fieldname text) returns text</function>
- <indexterm>
- <primary>ssl_client_dn_field</primary>
- </indexterm>
</term>
<listitem>
<para>
--- 157,167 ----
</varlistentry>
<varlistentry>
+ <indexterm>
+ <primary>ssl_client_dn_field</primary>
+ </indexterm>
<term>
<function>ssl_client_dn_field(fieldname text) returns text</function>
</term>
<listitem>
<para>
***************
*** 206,216 **** emailAddress
</varlistentry>
<varlistentry>
<term>
<function>ssl_issuer_field(fieldname text) returns text</function>
- <indexterm>
- <primary>ssl_issuer_field</primary>
- </indexterm>
</term>
<listitem>
<para>
--- 206,216 ----
</varlistentry>
<varlistentry>
+ <indexterm>
+ <primary>ssl_issuer_field</primary>
+ </indexterm>
<term>
<function>ssl_issuer_field(fieldname text) returns text</function>
</term>
<listitem>
<para>
***************
*** 220,225 **** emailAddress
--- 220,271 ----
</listitem>
</varlistentry>
</variablelist>
+
+ <variablelist>
+ <varlistentry>
+ <indexterm>
+ <primary>ssl_extension_value</primary>
+ </indexterm>
+ <term>
+ <function>ssl_extension_value(name text) returns text</function>
+ </term>
+ <listitem>
+ <para>
+ Returns value of extension by given extension name.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <variablelist>
+ <varlistentry>
+ <indexterm>
+ <primary>ssl_extension_is_critical</primary>
+ </indexterm>
+ <term>
+ <function>ssl_extension_is_critical(text) returns boolean</function>
+ </term>
+ <listitem>
+ <para>
+ Returns TRUE if extension is critical and FALSE otherwise.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <variablelist>
+ <varlistentry>
+ <indexterm>
+ <primary>ssl_extension_names</primary>
+ </indexterm>
+ <term>
+ <function>ssl_extension_names() returns setof extension</function>
+ </term>
+ <listitem>
+ <para>
+ Returns pairs of extension names and values. The type <structfield>extension</> contains 2 columns: <structfield>name</> and <structfield>value</>.
+ </para>
+ </listitem>
+ </varlistentry>
+
</sect2>
<sect2>
On Fri, Jul 18, 2014 at 3:12 PM, Воронин Дмитрий
<carriingfate92@yandex.ru> wrote:
I make a new version of patch. I corrected your notes for my previous
version of patch. Could you test it? Thank you.
I just had a look at the new version of this patch, and this is
lacking a couple of things.
First, this patch has been developed using a tarball of 9.3.4. sslinfo
is not a module changing frequently so you are lucky that this is not
generating diffs with HEAD but you should try to get yourself familiar
with git and submit patches that are based on HEAD to facilitate their
review and integration. You can have a look here to begin with:
https://wiki.postgresql.org/wiki/Working_with_GIT
Then, here are more comments about the patch:
- Update sslinfo to 1.1. Here are all the details about how to do it:
http://www.postgresql.org/docs/devel/static/extend-extensions.html#AEN57147
Well, basically this is only creating sslinfo--1.0--1.1.sql to be able
to do an upgrade, copy sslinfo--1.0.sql to sslinfo--1.1.sql with the
new objects defined and dumping sslinfo.control.
- In your previous patches I saw that you defined the new functions in
sslinfo--1.0.sql but in the new version of your patch it is not the
case.
- Please remove all the diff noise in sslinfo.sgml, like this one:
*** 157,167 ****
<varlistentry>
<term>
<function>ssl_client_dn_field(fieldname text) returns text</function>
- <indexterm>
- <primary>ssl_client_dn_field</primary>
- </indexterm>
</term>
<listitem>
<para>
--- 157,167 ----
</varlistentry>
<varlistentry>
+ <indexterm>
+ <primary>ssl_client_dn_field</primary>
+ </indexterm>
Your patch should only include documentation for the new functions.
- Please remove whitespaces, there are quite a lot of them.
- in 9.5, PG_FUNCTION_INFO_V1 does the function declaration for you,
making following block useless:
[...]
+Datum ssl_is_used(PG_FUNCTION_ARGS);
+Datum ssl_version(PG_FUNCTION_ARGS);
+Datum ssl_cipher(PG_FUNCTION_ARGS);
[...]
- Please avoid if blocks of this type, this is not consistent with the
project style:
if (SRF_IS_FIRSTCALL()) {
[...]
}
One newline for each bracket. Here is the manual page referencing code
conventions:
http://www.postgresql.org/docs/current/static/source.html
Most of those comments have been already mentioned by Andreas in one
of his emails upthread but you have obviously not solved the issues he
has reported.
This looks like a useful feature, but at this point of the commit fest
and considering the number of structural changes still needed I will
mark this patch as "Returned with feedback". Feel free to resubmit to
the next commit fest in August with an updated patch of course!
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