[pgpool-hackers: 3563] Re: [PATCH] Feature: Support for CRL (Certificate Revocation List)

Umar Hayat m.umarkiani at gmail.com
Mon Mar 23 22:49:26 JST 2020 

Hi,
Please find attached fix. I made the test action generic so that I can work
with old SSL versions.

Regards
Umar Hayat

On Mon, Mar 23, 2020 at 5:13 AM Tatsuo Ishii <ishii at sraoss.co.jp> wrote:

> Hi Umar Hayat,
>
> Unfortunately still build farm is failing.
>
> =========================================================================
> * master  PostgreSQL 11  CentOS6
> testing 024.cert_auth...failed.
>
> * master  PostgreSQL 12  CentOS6
> testing 024.cert_auth...failed.
>
> * master  PostgreSQL 9.6  CentOS6
> testing 024.cert_auth...failed.
>
> * master  PostgreSQL 9.5  CentOS6
> testing 024.cert_auth...failed.
> =========================================================================
>
> Subject: Re: [pgpool-hackers: 3551] Re: [PATCH] Feature: Support for CRL
> (Certificate Revocation List)
> Date: Thu, 19 Mar 2020 15:25:03 +0500
> Message-ID: <
> CACnGOt0xTAmyFy9azF44Z_2smdgwzKxjzFNkhY7mvp64CgPSKg at mail.gmail.com>
>
> > I saw the logs and it is generating 512bit key for certificate on CentOS6
> > and as result there is this error *"digest too big for rsa key".* On
> > CentOS7 and Debian Openssl 1.1.1 generates 2048bit keys by default. AFAIK
> > 512 keys is not supported with TLSv1.2.
> > In patch attached, I updated defaults bits value to 2048, Hope this would
> > fix the problem. Also printed openssl version to check what version is
> used
> > on BF, and would help in future to debug.
> > I will keep an eye on BF for tomorrows build to see results.
> >
> > Regards
> > Umar Hayat
> >
> > On Thu, Mar 19, 2020 at 6:59 AM Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
> >
> >> After pushing this, build farm is conituouly failing with
> >> 024.cert_auth on CentOS6. CentOS7 and 8 seem OK. For now, build farm
> >> log and pgpool.log attached. (Full information can be otained from
> >> https://pgpool.net/buildfarm/).
> >>
> >> From: Tatsuo Ishii <ishii at sraoss.co.jp>
> >> Subject: [pgpool-hackers: 3551] Re: [PATCH] Feature: Support for CRL
> >> (Certificate Revocation List)
> >> Date: Sat, 14 Mar 2020 12:20:20 +0900 (JST)
> >> Message-ID: <20200314.122020.1318630628622464420.t-ishii at sraoss.co.jp>
> >>
> >> > Hi Usama,
> >> >
> >> > Thank you for the confirmantion. I have pushed Umar's patch. I have
> >> > made a small modification to the doc:
> >> >
> >> > +      Specifies the name of the file containing the SSL server
> >> > +      certificate revocation list (CRL). The default is empty,
> >> >
> >> > to:
> >> >
> >> >       Specifies the path to the file containing the SSL server
> >> >       certificate revocation list (CRL).
> >> >
> >> > because ssl_crl_file should be a full path to the CRL file, rather
> >> > than just a file name.
> >> >
> >> > Also I have added a Japanese doc.
> >> >
> >> > Umar Hayat,
> >> > Thank you for your contribution!
> >> >
> >> >> Hi Ishii-San
> >> >>
> >> >> The patch is good now and does what it is intended for.
> >> >> We can commit it to the master branch.
> >> >>
> >> >> Thanks
> >> >> Best regards
> >> >> Muhammad Usama
> >> >>
> >> >>
> >> >> On Wed, Mar 11, 2020 at 4:30 PM Tatsuo Ishii <ishii at sraoss.co.jp>
> >> wrote:
> >> >>
> >> >>> Hi Umar,
> >> >>>
> >> >>> Now the regression test passed!
> >> >>> Thank you for the update.
> >> >>>
> >> >>> Usama,
> >> >>>
> >> >>> Do you agree to commit this patch?
> >> >>>
> >> >>> Best regards,
> >> >>> --
> >> >>> Tatsuo Ishii
> >> >>> SRA OSS, Inc. Japan
> >> >>> English: http://www.sraoss.co.jp/index_en.php
> >> >>> Japanese:http://www.sraoss.co.jp
> >> >>>
> >> >>> > Hi Ishii,
> >> >>> > I guess I have to learn pgpool regression structure a bit more
> and It
> >> >>> came
> >> >>> > out not a good idea updating and existing test :-)
> >> >>> > I was updating the 'ssl_crl_file' in pgpool.conf after calling
> >> >>> './startall'
> >> >>> > , which I suppose would not load the update configuration.
> >> >>> > In latest patch I moved that update operation before './startall'
> and
> >> >>> added
> >> >>> > extra check to validate that pgpool configuration is correctly
> >> updated.
> >> >>> > I hope this patch would fix the issue. Please find attached.
> >> >>> >
> >> >>> > Regards
> >> >>> > Umar Hayat
> >> >>> >
> >> >>> >
> >> >>> >
> >> >>> >
> >> >>> > On Wed, Mar 11, 2020 at 3:04 AM Tatsuo Ishii <ishii at sraoss.co.jp>
> >> wrote:
> >> >>> >
> >> >>> >> Unfortunately the regression test failed again.
> >> >>> >>
> >> >>> >> $ ./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
> >> >>> >>
> >> >>> >> regression log attached.
> >> >>> >>
> >> >>> >> > Thanks Usama for feedback. updated patch with suggested changed
> >> >>> attached.
> >> >>> >> > Please see comments inline.
> >> >>> >> >
> >> >>> >> > Ishii,
> >> >>> >> > IMO, tests were failing because some certificates generation
> and
> >> >>> signing
> >> >>> >> > was using global configuration. Previously, I only provided
> >> >>> >> > minimum configuration for CRL generation.
> >> >>> >> > Now I provided minimum custom configuration for Cert generation
> >> and
> >> >>> >> Signing
> >> >>> >> > too. I hope now that last tests should be passing now.
> >> >>> >> >
> >> >>> >> > Regards
> >> >>> >> > Umar Hayat
> >> >>> >> >
> >> >>> >> >
> >> >>> >> > On Wed, Mar 4, 2020 at 7:19 PM Muhammad Usama <
> m.usama at gmail.com>
> >> >>> wrote:
> >> >>> >> >
> >> >>> >> >> 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?
> >> >>> >> >>
> >> >>> >> >> If CA is not configure then certification validation will fail
> >> while
> >> >>> >> > validating certification.
> >> >>> >> >
> >> >>> >> >>
> >> >>> >> >> 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.
> >> >>> >> >>
> >> >>> >> >> I wasn't sure either that would matter as we are just
> registering
> >> >>> event
> >> >>> >> at
> >> >>> >> > that state, but as suggest i reorder the code as you suggested
> (
> >> as
> >> >>> >> > Postgres does too )
> >> >>> >> >
> >> >>> >> >> 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
> >> >>> >> >>
> >> >>> >> >> Comment fixed.
> >> >>> >> >
> >> >>> >> >> 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.
> >> >>> >> >>
> >> >>> >> >>  PGSSLROOTCERT defined.
> >> >>> >> >
> >> >>> >> >> 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
> >> >>> >> >>>
> >> >>> >> >>
> >> >>> >>
> >> >>>
> >> > _______________________________________________
> >> > 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/20200323/f380da17/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: regression-024-fix_CentOS6_SSL3_Revoked.diff
Type: application/octet-stream
Size: 722 bytes
Desc: not available
URL: <http://www.sraoss.jp/pipermail/pgpool-hackers/attachments/20200323/f380da17/attachment-0001.obj>

More information about the pgpool-hackers mailing list