[pgpool-hackers: 3275] Re: ssl_ciphers

[Muhammad Usama]

> On 26-Mar-2019, at 12:25 PM, Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
> 
> Usama,
> 
>> Hi Ishii-San
>> 
>>> On Tue, Mar 26, 2019 at 9:50 AM Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
>>> 
>>> Hi Pgpool Developers,
>>> 
>>> Recently I got a request from a customer to add ssl_ciphers
>>> functionality of PostgreSQL. This allows to limit accepting SSL
>>> ciphers. The main motivation of this is to defend Pgpool-II from weak
>>> ciphers attack (known as Sweet32).
>>> https://access.redhat.com/security/cve/cve-2016-2183
>>> 
>>> I don't think this is a vulnerability of Pgpool-II itself. In fact
>>> other OSS projects except OpenSSL have not assigned CVE because of
>>> this.
>>> 
>>> However, I think implementing ssl_ciphers is a good thing for
>>> Pgpool-II because we could say "we are safer than before." Also some
>>> of other OSS projects have done something for this.
>>> 
>>> So I decided to implement ssl_ciphers and back patch to all supported
>>> branches. Usually we do not add new configuration parameters to minor
>>> releases but this time there's no other way to implement the feature,
>>> so I have to add a new parameter ssl_ciphers.
>>> 
>> 
>> I agree this is the right way to go, to make an exception in this case and
>> back-port
>> the patch to all supported branches.
>> 
>>> 
>>> The main changes to the code are fairly small (in src/utils/pool_ssl.c):
>>> 
>>> +       /* set up the allowed cipher list */
>>> +       error = SSL_CTX_set_cipher_list(cp->ssl_ctx,
>>> pool_config->ssl_ciphers);
>>>        SSL_RETURN_ERROR_IF((error != 1), "Setting allowed cipher list");
>>> +
>>> +       /* Let server choose order */
>>> +       SSL_CTX_set_options(cp->ssl_ctx, SSL_OP_CIPHER_SERVER_PREFERENCE);
>>> +
>>> 
>>> SSL_CTX_set_cipher_list() limits the cipher
>>> list. SSL_CTX_set_options() is needed to reject client's request for
>>> ciphers not in the cipher list.
>>> 
>>> Attached is the patch for master branch.
>>> 
>> 
>> The patch looks fine, One small comment is, do you think we should make
>> the SSL_CTX_set_options(cp->ssl_ctx, SSL_OP_CIPHER_SERVER_PREFERENCE); call
>> conditional with some new configuration parameter (similar to PostgreSQL's
>> ssl_prefer_server_ciphers config)
>> and set the default value of that parameter to "off", so that minor version
>> upgrades
>> keep the consistent behaviour, and users gets the option to use server or
>> client cipher preference.
> 
> Yeah, since we are going make releases for stable branches, keeping
> existent behavior is important. I agree with you.
> 
> Do you mind if I ask you to implement ssl_prefer_server_ciphers? If
> ok, I would like to push the patch as proposed (without
> ssl_prefer_server_ciphers), then you implement
> ssl_prefer_server_ciphers part on top of it.

Sure I will do that today after you push this path 

Thanks 
Best Regards 
Muhammad Usama 

> 
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp