Thread: be-secure.c patch
Hello PG folks,
the attachement contains a simple patch to adding of verification of client's certificate(s)
the attachement contains a simple patch to adding of verification of client's certificate(s)
against CRL on server side in mutual SSL authentication.
The CRL file has name "root.crl" and it must be stored in PGDATA directory.
The CRL file has name "root.crl" and it must be stored in PGDATA directory.
With best regards
L. Hohos
Attachment
=?iso-8859-2?Q?Libor_Hoho=B9?= <liho@d-prog.cz> writes:
> the attachement contains a simple patch to adding of verification of client=
> 's certificate(s)
> against CRL on server side in mutual SSL authentication.
> The CRL file has name "root.crl" and it must be stored in PGDATA directory.
Uh, why exactly would we want that? It sounds like it duplicates the
existing root.crt functionality.
regards, tom lane
=?iso-8859-2?Q?Libor_Hoho=B9?= <liho@d-prog.cz> writes:
>> It sounds like it duplicates the
>> existing root.crt functionality.
> root.crT is file with X509 certificate of Certification Authority
> root.crL is file with X509 Certificate Revocation List issued by this
> Certification Authority
Oh, is that what it does? Is this documented anywhere?
regards, tom lane
This has been saved for the 8.2 release:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold
---------------------------------------------------------------------------
Libor Hoho� wrote:
> Hello PG folks,
> the attachement contains a simple patch to adding of verification of client's certificate(s)
> against CRL on server side in mutual SSL authentication.
> The CRL file has name "root.crl" and it must be stored in PGDATA directory.
>
> With best regards
> L. Hohos
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Does this need any documentation adjustments?
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Libor Hoho� wrote:
> Hello PG folks,
> the attachement contains a simple patch to adding of verification of client's certificate(s)
> against CRL on server side in mutual SSL authentication.
> The CRL file has name "root.crl" and it must be stored in PGDATA directory.
>
> With best regards
> L. Hohos
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org
--
Bruce Momjian http://candle.pha.pa.us
SRA OSS, Inc. http://www.sraoss.com
+ If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Does this need any documentation adjustments?
It's pretty useless without any documentation ... which was my original
complaint about it IIRC.
regards, tom lane
Patch adjusted and applied. Thanks. I added documentation about SSL Certificate Revocation List (CRL) files. We throw a log message of "root.crl" does exist. Perhaps we should just silently say nothing, but that seems dangerous. --------------------------------------------------------------------------- Libor Hoho� wrote: > Hello PG folks, > the attachement contains a simple patch to adding of verification of client's certificate(s) > against CRL on server side in mutual SSL authentication. > The CRL file has name "root.crl" and it must be stored in PGDATA directory. > > With best regards > L. Hohos [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Have you searched our list archives? > > http://archives.postgresql.org -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/runtime.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/runtime.sgml,v retrieving revision 1.370 diff -c -c -r1.370 runtime.sgml *** doc/src/sgml/runtime.sgml 11 Apr 2006 21:04:52 -0000 1.370 --- doc/src/sgml/runtime.sgml 27 Apr 2006 02:27:13 -0000 *************** *** 1553,1559 **** the file <filename>root.crt</filename> in the data directory. When present, a client certificate will be requested from the client during SSL connection startup, and it must have been signed by one of the ! certificates present in <filename>root.crt</filename>. </para> <para> --- 1553,1561 ---- the file <filename>root.crt</filename> in the data directory. When present, a client certificate will be requested from the client during SSL connection startup, and it must have been signed by one of the ! certificates present in <filename>root.crt</filename>. Certificate ! Revocation List (CRL) entries are also checked if the file ! <filename>root.crl</filename> exists. </para> <para> *************** *** 1564,1572 **** <para> The files <filename>server.key</>, <filename>server.crt</>, ! and <filename>root.crt</filename> are only examined during server ! start; so you must restart the server to make changes in them take ! effect. </para> </sect1> --- 1566,1574 ---- <para> The files <filename>server.key</>, <filename>server.crt</>, ! <filename>root.crt</filename>, and <filename>root.crl</filename> ! are only examined during server start; so you must restart ! the server to make changes in them take effect. </para> </sect1> Index: src/backend/libpq/be-secure.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/libpq/be-secure.c,v retrieving revision 1.63 diff -c -c -r1.63 be-secure.c *** src/backend/libpq/be-secure.c 21 Mar 2006 18:18:35 -0000 1.63 --- src/backend/libpq/be-secure.c 27 Apr 2006 02:27:13 -0000 *************** *** 102,107 **** --- 102,108 ---- #ifdef USE_SSL #define ROOT_CERT_FILE "root.crt" + #define ROOT_CRL_FILE "root.crl" #define SERVER_CERT_FILE "server.crt" #define SERVER_PRIVATE_KEY_FILE "server.key" *************** *** 794,799 **** --- 795,822 ---- } else { + /* + * Check the Certificate Revocation List (CRL) if file exists. + * http://searchsecurity.techtarget.com/sDefinition/0,,sid14_gci803160,00.html + */ + X509_STORE *cvstore = SSL_CTX_get_cert_store(SSL_context); + + if (cvstore) + { + if (X509_STORE_load_locations(cvstore, ROOT_CRL_FILE, NULL) != 0) + /* setting the flags to check against the complete CRL chain */ + X509_STORE_set_flags(cvstore, + X509_V_FLAG_CRL_CHECK|X509_V_FLAG_CRL_CHECK_ALL); + else + { + /* Not fatal - we do not require CRL */ + ereport(LOG, + (errmsg("SSL Certificate Revocation List (CRL) file \"%s\" not found, skipping: %s", + ROOT_CRL_FILE, SSLerrmessage()), + errdetail("Will not check certificates against CRL."))); + } + } + SSL_CTX_set_verify(SSL_context, (SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT |
I am now wondering if fe-secure.c, the front-end code, should also check for "root.crl". The attached patch implents it. Is it a good idea? Also, if you look in interfaces/libpq/fe-secure.c at some NOT_USED macros you can see there are a few things we don't implement. Can that be improved? --------------------------------------------------------------------------- > Patch adjusted and applied. Thanks. > > I added documentation about SSL Certificate Revocation List (CRL) files. > > We throw a log message of "root.crl" does exist. Perhaps we should just > silently say nothing, but that seems dangerous. > > --------------------------------------------------------------------------- > > > > Libor Hoho<B9> wrote: > > Hello PG folks, > > the attachement contains a simple patch to adding of verification of > client's certificate(s) > > against CRL on server side in mutual SSL authentication. > > The CRL file has name "root.crl" and it must be stored in PGDATA > directory. -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/interfaces/libpq/fe-secure.c =================================================================== RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v retrieving revision 1.79 diff -c -c -r1.79 fe-secure.c *** src/interfaces/libpq/fe-secure.c 27 Apr 2006 14:02:36 -0000 1.79 --- src/interfaces/libpq/fe-secure.c 27 Apr 2006 14:08:18 -0000 *************** *** 125,135 **** --- 125,137 ---- #define USER_CERT_FILE ".postgresql/postgresql.crt" #define USER_KEY_FILE ".postgresql/postgresql.key" #define ROOT_CERT_FILE ".postgresql/root.crt" + #define ROOT_CRL_FILE ".postgresql/root.crl" #else /* On Windows, the "home" directory is already PostgreSQL-specific */ #define USER_CERT_FILE "postgresql.crt" #define USER_KEY_FILE "postgresql.key" #define ROOT_CERT_FILE "root.crt" + #define ROOT_CRL_FILE "root.crl" #endif #ifdef NOT_USED *************** *** 784,789 **** --- 786,793 ---- snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE); if (stat(fnbuf, &buf) == 0) { + X509_STORE *cvstore; + if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, NULL)) { char *err = SSLerrmessage(); *************** *** 795,800 **** --- 799,813 ---- return -1; } + if ((cvstore = SSL_CTX_get_cert_store(SSL_context)) != NULL) + { + if (X509_STORE_load_locations(cvstore, ROOT_CRL_FILE, NULL) != 0) + /* setting the flags to check against the complete CRL chain */ + X509_STORE_set_flags(cvstore, + X509_V_FLAG_CRL_CHECK|X509_V_FLAG_CRL_CHECK_ALL); + /* if not found, silently ignore; we do not require CRL */ + } + SSL_CTX_set_verify(SSL_context, SSL_VERIFY_PEER, verify_cb); } }
Bruce Momjian wrote: > > I am now wondering if fe-secure.c, the front-end code, should also check > for "root.crl". The attached patch implents it. Updated patch attached and applied. It adds CRL checking to libpq. It returns an error if the CRL file exists, but the library can't process it, just like the backend. -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/interfaces/libpq/fe-secure.c =================================================================== RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v retrieving revision 1.79 diff -c -c -r1.79 fe-secure.c *** src/interfaces/libpq/fe-secure.c 27 Apr 2006 14:02:36 -0000 1.79 --- src/interfaces/libpq/fe-secure.c 6 May 2006 02:21:50 -0000 *************** *** 125,135 **** --- 125,137 ---- #define USER_CERT_FILE ".postgresql/postgresql.crt" #define USER_KEY_FILE ".postgresql/postgresql.key" #define ROOT_CERT_FILE ".postgresql/root.crt" + #define ROOT_CRL_FILE ".postgresql/root.crl" #else /* On Windows, the "home" directory is already PostgreSQL-specific */ #define USER_CERT_FILE "postgresql.crt" #define USER_KEY_FILE "postgresql.key" #define ROOT_CERT_FILE "root.crt" + #define ROOT_CRL_FILE "root.crl" #endif #ifdef NOT_USED *************** *** 784,789 **** --- 786,793 ---- snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE); if (stat(fnbuf, &buf) == 0) { + X509_STORE *cvstore; + if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, NULL)) { char *err = SSLerrmessage(); *************** *** 795,800 **** --- 799,826 ---- return -1; } + if ((cvstore = SSL_CTX_get_cert_store(SSL_context)) != NULL) + { + /* setting the flags to check against the complete CRL chain */ + if (X509_STORE_load_locations(cvstore, ROOT_CRL_FILE, NULL) != 0) + /* OpenSSL 0.96 does not support X509_V_FLAG_CRL_CHECK */ + #ifdef X509_V_FLAG_CRL_CHECK + X509_STORE_set_flags(cvstore, + X509_V_FLAG_CRL_CHECK|X509_V_FLAG_CRL_CHECK_ALL); + /* if not found, silently ignore; we do not require CRL */ + #else + { + char *err = SSLerrmessage(); + + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("Installed SSL library does not support CRL certificates, file \"%s\"\n"), + fnbuf); + SSLerrfree(err); + return -1; + } + #endif + } + SSL_CTX_set_verify(SSL_context, SSL_VERIFY_PEER, verify_cb); } }