Add --ssl-version-min and --ssl-version-fallback-min to chrome://flags.

Add --ssl-version-min and --ssl-version-fallback-min to chrome://flags.

That way, Android users get an opportunity to disable SSLv3.

BUG=426038

[Thiemo Nagel, 2014-10-21 16:46:43]

tnagel@chromium.org changed reviewers:
+ joaodasilva@chromium.org

[Thiemo Nagel, 2014-10-21 16:46:44]

Hi Joao,

may I take up your offer and ask you to test this on Android?

There's two relevant settings for each flag
--ssl-version-min={ssl3,tls1}
--ssl-version-fallback-min={ssl3,tls1}
yielding four combinations that I'd kindly ask you to test with each of the
following sites:

* https://www.imperialviolet.org:3333/
  SSLv3 only: This page should not be accessible with --ssl-version-min=tls1
  but it should work alright with --ssl-version-min=ssl3.  The value of
  --ssl-version-fallback-min should have no effect.
* https://poodle.io
  Fallback to SSLv3: This page should display a warning banner for
  --ssl-version-fallback-min=ssl3 in conjunction with --ssl-version-min=ssl3.
  For all other other combinations, the banner should read "Good News!".
* https://google.com/
  Should always work.

Note that the semantics of --ssl-version-fallback-min seems to be different that
its policy equivalent:  On the command line, --ssl-version-min is always
honoured, whereas the SSLVersionFallbackMin policy may override SSLVersionMin. 
(At least that's my understanding.)

Thank you!
Thiemo

[Joao da Silva, 2014-10-22 10:49:57]

The cases you've described work as expected except one: min ssl3 + fallback min
tls1 still shows a warning at https://poodle.io.

Code looks good to me. Check with security people if it's a good idea to have
these flags.

https://codereview.chromium.org/667183002/diff/20001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/667183002/diff/20001/chrome/app/generated_res...
chrome/app/generated_resources.grd:9616: +      <message
name="IDS_FLAGS_SSL_VERSION_MIN_DESCRIPTION" desc="min desc">
The desc attribute is used to give a hint to translators, to clarify what this
string is meant to convey. Can you make it a bit more descriptive?

https://codereview.chromium.org/667183002/diff/20001/chrome/browser/about_fla...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/667183002/diff/20001/chrome/browser/about_fla...
chrome/browser/about_flags.cc:504: 
single newline

https://codereview.chromium.org/667183002/diff/20001/chrome/browser/about_fla...
chrome/browser/about_flags.cc:2012: // histograms.xml and don't forget to run
AboutFlagsHistogramTest unit test.
What about this?

[Thiemo Nagel, 2014-10-22 16:30:54]

Hi Joao,

many thanks for your assistance!  Could you please take another look?

Thank you!
Thiemo

https://codereview.chromium.org/667183002/diff/20001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/667183002/diff/20001/chrome/app/generated_res...
chrome/app/generated_resources.grd:9616: +      <message
name="IDS_FLAGS_SSL_VERSION_MIN_DESCRIPTION" desc="min desc">
Yes, sure.  (Having them as they are was an oversight.)

https://codereview.chromium.org/667183002/diff/20001/chrome/browser/about_fla...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/667183002/diff/20001/chrome/browser/about_fla...
chrome/browser/about_flags.cc:504: 
On 2014/10/22 10:49:57, Joao da Silva wrote:
> single newline

Thanks.  Done.

https://codereview.chromium.org/667183002/diff/20001/chrome/browser/about_fla...
chrome/browser/about_flags.cc:2012: // histograms.xml and don't forget to run
AboutFlagsHistogramTest unit test.
On 2014/10/22 10:49:57, Joao da Silva wrote:
> What about this?

Good catch.  I had interpreted that as "upon adding a new command-line switch"
(which wouldn't apply to me), not as "upon new chrome://flags" entry.

[Joao da Silva, 2014-10-22 16:56:20]

The implementation looks good to me. Let's figure with the security people
whether this flag is worth adding or not; please get their approval too before
landing.

[Ryan Sleevi, 2014-10-22 17:09:36]

rsleevi@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Ryan Sleevi, 2014-10-22 17:09:36]

Not LGTM. Please do not land this. It is an explicit goal NOT to add this.

[Ryan Sleevi, 2014-11-04 20:58:05]

FYI: https://codereview.chromium.org/693963003/

After discussion with Darin and chrome leads, --ssl-version-min will be exposed
via flags, but *will* disappear over time. It's a short-term solution for M40.
-fallback-min will *not* be exposed, but will be controlled via Finch.