Re: [Bug-wget] [PATCH] Allow to redefine ciphers list for OpenSSL

[Tomas Hozza]

----- Original Message -----
> On Friday 11 July 2014 04:30:04 Tomas Hozza wrote:
> > ----- Original Message -----
> > 
> > > On Thursday 10 July 2014 08:37:23 Tomas Hozza wrote:
> > > > ----- Original Message -----
> > > > 
> > > > > On Tuesday 08 July 2014 16:14:42 Petr Pisar wrote:
> > > > > > On Tue, Jul 08, 2014 at 10:00:24AM -0400, Tomas Hozza wrote:
> > > > > > > I'm afraid this is not suitable for us. We need to be able to
> > > > > > > define
> > > > > > > the
> > > > > > > policy somewhere in /etc, where the user is not able to change it
> > > > > > > (only
> > > > > > > the system administrator).
> > > > > > 
> > > > > > I hope can also prevent the user from running his own wget
> > > > > > executable,
> > > > > > or
> > > > > > ld-preloading modified OpenSSL library, or intercepting open(2)
> > > > > > calls
> > > > > > to
> > > > > > provide fake /etc file.
> > > > > > 
> > > > > > > Also the main intention to have a single place to set the policy
> > > > > > > for
> > > > > > > all
> > > > > > > system components, therefore wgetrc is not the right place for
> > > > > > > us.
> > > > > > 
> > > > > > What about to change wget to call OPENSSL_config(NULL) instead of
> > > > > > setting
> > > > > > some hard-coded preference string. Then you can teach OpenSSL to
> > > > > > load
> > > > > > your
> > > > > > /etc configuration instead of patching each application.
> > > > > > 
> > > > > > -- Petr
> > > > > 
> > > > > Tomas intention is to only change the (Wget hard-coded) cipher list
> > > > > for
> > > > > --secure-protocol=PFS. At least, that's what I understood so far.
> > > > 
> > > > It may seem so, but my intention was to be able to redefine any
> > > > occurrence
> > > > of explicitly hard-coded ciphers priority list. In openssl.c it was
> > > > only
> > > > in
> > > > the code that was executed if --secure-protocol=PFS was used.
> > > 
> > > In this case, you should use a name like --with-PFS-ciphers-list=LIST,
> > > because
> > > you are just changing the PFS hard-coded cipher list. Imagine, we add new
> > > -- secure-protocol options with hard-coded values like
> > > --secure-protocol=FOO
> > > --secure-protocol=BAR
> > > 
> > > In this case you have to create another patch with
> > > ./configure --with-ciphers-list-FOO
> > > and
> > > ./configure --with-ciphers-list-BAR
> > > since the meanings could be very different. This is why I think it makes
> > > sense
> > > to add 'PFS' to your ./configure option name.
> > 
> > I think you misunderstood me. My intention was NOT to handle PFS or any
> > other method specially. The intention is to replace ALL occurrences of
> > hard-coded ciphers priority list strings with value defined when running
> > ./configure.
> > 
> > That's why I don't want to introduce 4 new options, but a single one.
> 
> OK, than maybe it is a good idea to just patch OpenSSL code since there is
> just one occurrence of a hard-coded cipher string.
> For GnuTLS this is IMHO not the way to go, since you would make --secure-
> protocol=... doing simply nothing. There are users of Wget who needs to set
> the protocol (which is wired with ciphers, key exchange method, ... as I
> wrote
> earlier) for whatever reason.
> 
> So either you name the configure option again ...openssl... as you did in
> your
> first patch, and just patch OpenSSL code.
> Or with GnuTLS, we must inform the user about not being able to choose the
> protocol whenever he uses --secure-protocol (command line or config file).
> 
> Just patching OpenSSL code would need something like this
> 
> #ifdef OPENSSL_CIPHERS_LIST
>   /* Redhat request: setting cipher list at compile time
>   SSL_CTX_set_cipher_list (ssl_ctx, OPENSSL_CIPHERS_LIST);
> #elif
>   /* OpenSSL ciphers: https://www.openssl.org/docs/apps/ciphers.html
>    * Since we want a good protection, we also use HIGH (that excludes MD4
> ciphers and some more)
>    */
>   if (opt.secure_protocol == secure_protocol_pfs)
>     SSL_CTX_set_cipher_list (ssl_ctx,
> "HIGH:MEDIUM:!RC4:!SRP:!PSK:!RSA:address@hidden");
> #endif
> 
> So we are basically back to your patch #2 (+ you basically need to add the
> above).

Technically it is already done in version 2 of the patch. And it is also done 
exactly
the same way you're suggesting in the version 3. I agree it is more obvious and 
easier
to read way.

I'll wait a while for some more comments and if there are none I'll prepare 
hopefully
the final version of the patch.

Thanks.

Regards,
-- 
Tomas Hozza
Software Engineer - EMEA ENG Developer Experience

PGP: 1D9F3C2D
Red Hat Inc.                               http://cz.redhat.com