[pgpool-hackers: 3533] Re: [PATCH] Feature: Support for CRL (Certificate Revocation List)
Muhammad Usama
m.usama at gmail.com
Wed Mar 4 23:19:08 JST 2020
Hi
I have looked into the patch and here are my two cents.
1- Does the "Certificate Revocation List (CRL)" have any effect if we do
not configure CA certificates?
Meaning what would be the behavior of the system if someone specifies
"ssl_crl_file" but leave out the ssl_ca_cert config?
2- In the patch you are loading the Certificate Revocation List after
configuring
SSL library to ask for client certificates
i.e after calling SSL_CTX_set_verify and SSL_CTX_set_client_CA_list
functions
Although I am not able to find any related SSL documentation on the
subject and not sure if it can cause any problem. But in the normal
sequence, we always load
the CA and other certificated before SSL_CTX_set_verify and
SSL_CTX_set_client_CA_list functions.
So I guess the safe bet is to move the loading of Certificate Revocation
List before
SSL_CTX_set_verify.
3- The comment /* OpenSSL 0.96 does not support X509_V_FLAG_CRL_CHECK */
mentions
OpenSSL version as 0.96, which is wrong and should be 0.9.6
4- In the test case you are only exporting PGSSLCERT and PGSSLKEY
environment variables
that means the test case will try to pick up the root certificate from the
default location
and on the systems where the root certificate would not be found there, the
test case will fail.
So I think you need to modify the test case and use PGSSLROOTCERT
environment variable to
explicitly specify the root certificate as well.
These comments are on top of Ishii-San's review comments.
Thanks
Best Regards
Muhammad Usama
On Wed, Mar 4, 2020 at 12:11 PM Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
> Hi Umar,
>
> Any update on this?
>
> From: Tatsuo Ishii <ishii at sraoss.co.jp>
> Subject: [pgpool-hackers: 3523] Re: [PATCH] Feature: Support for CRL
> (Certificate Revocation List)
> Date: Fri, 28 Feb 2020 14:27:55 +0900 (JST)
> Message-ID: <20200228.142755.205379439290164558.t-ishii at sraoss.co.jp>
>
> > Here are comments on your patch.
> >
> > - There are some extra trailing spaces.
> >
> > $ git apply ~/crl_support_with_testcase.diff
> > /home/t-ishii/crl_support_with_testcase.diff:42: trailing whitespace.
> > Specifies the name of the file containing the SSL server
> > /home/t-ishii/crl_support_with_testcase.diff:43: trailing whitespace.
> > certificate revocation list (CRL). The default is empty,
> > /home/t-ishii/crl_support_with_testcase.diff:199: new blank line at EOF.
> > +
> > warning: 3 lines add whitespace errors.
> >
> > - The pached source code compililes without any error.
> >
> > - the regression test (024.cert_auth) failed.
> >
> > ./regress.sh 024
> > creating pgpool-II temporary installation ...
> > moving pgpool_setup to temporary installation path ...
> > moving watchdog_setup to temporary installation path ...
> > using pgpool-II at
> /home/t-ishii/work/Pgpool-II/current/pgpool2/src/test/regression/temp/installed
> > *************************
> > REGRESSION MODE : install
> > PGPOOL-II :
> /home/t-ishii/work/Pgpool-II/current/pgpool2/src/test/regression/temp/installed
> > PostgreSQL bin : /usr/local/pgsql/bin
> > PostgreSQL Major version : 12
> > pgbench : /usr/local/pgsql/bin/pgbench
> > PostgreSQL jdbc :
> /usr/local/pgsql/share/postgresql-9.2-1003.jdbc4.jar
> > *************************
> > testing 024.cert_auth...failed.
> > out of 1 ok:0 failed:1 timeout:0
> >
> > This is Ubuntu 18.04.4 LTS.
> >
> > $ openssl version
> > OpenSSL 1.1.1 11 Sep 2018
> >
> > Please find attached log file for the
> > regression test.
> >
> > Best regards,
> > --
> > Tatsuo Ishii
> > SRA OSS, Inc. Japan
> > English: http://www.sraoss.co.jp/index_en.php
> > Japanese:http://www.sraoss.co.jp
> >
> >> Hi Umar,
> >>
> >> I seemed to miss your last email. I will take care your patch
> >> tomorrow morning.
> >>
> >> Best regards,
> >> --
> >> Tatsuo Ishii
> >> SRA OSS, Inc. Japan
> >> English: http://www.sraoss.co.jp/index_en.php
> >> Japanese:http://www.sraoss.co.jp
> >>
> >>> Hi Tatsuo,
> >>> Any update for last patch?
> >>> I will be sending more patches in the same area of SSL ( for few other
> >>> features ) and the those patches might create conflict on merge.
> >>>
> >>> Regards,
> >>> Umar Hayat
> >>> Principal Software Engineer
> >>> EnterpriseDB: https://www.enterprisedb.com
> >>>
> >>> On Wed, Feb 19, 2020 at 1:39 PM Umar Hayat <m.umarkiani at gmail.com>
> wrote:
> >>>
> >>>> Hi Tatsuo,
> >>>> Please find the attached updated patch with following changes:
> >>>> 1. Updated the description of '*ssl_crl_file'* configuration variable.
> >>>> 2. Updated test case '024.cert_auth' which verify valid CRL and
> invalid
> >>>> CRL ( CRL with revocation entry )
> >>>>
> >>>> Regards,
> >>>> Umar Hayat
> >>>>
> >>>>
> >>>> On Thu, Feb 13, 2020 at 3:43 AM Tatsuo Ishii <ishii at sraoss.co.jp>
> wrote:
> >>>>
> >>>>> > I just followed the description pattern used for other ssl
> variables. We
> >>>>> > can use PostgreSQL doc if we remove following two line from that:
> >>>>> > "Relative paths are relative to the data
> >>>>> > directory. This parameter can only be set in the postgresql.conf
> file
> >>>>> > or on the server command line.
> >>>>> > "
> >>>>>
> >>>>> Sounds good to me.
> >>>>>
> >>>>> > - It would be nice to include regression test patch. See
> >>>>> >> src/test/023.ssl_connection for an example.
> >>>>> >>
> >>>>> >
> >>>>> > Sure, I will create and send test patch in
> src/test/023.ssl_connection.
> >>>>> > I will try to generate CRL file for existing certification file in
> this
> >>>>> > this test. If thats not possible, then I have to generate new
> >>>>> certification
> >>>>> > and CRL file.
> >>>>>
> >>>>> Thank you. Looking forward to the new patch.
> >>>>>
> >>>>> Best regards,
> >>>>> --
> >>>>> Tatsuo Ishii
> >>>>> SRA OSS, Inc. Japan
> >>>>> English: http://www.sraoss.co.jp/index_en.php
> >>>>> Japanese:http://www.sraoss.co.jp
> >>>>>
> >>>>
> >> _______________________________________________
> >> pgpool-hackers mailing list
> >> pgpool-hackers at pgpool.net
> >> http://www.pgpool.net/mailman/listinfo/pgpool-hackers
> _______________________________________________
> pgpool-hackers mailing list
> pgpool-hackers at pgpool.net
> http://www.pgpool.net/mailman/listinfo/pgpool-hackers
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200304/1c7f5fda/attachment.html>
More information about the pgpool-hackers
mailing list