Allow option to ignore default route

Allow option to ignore default route

In OpenVPN, it is up to the client to decide whether or not to
create a default route through the OpenVPN server.  Even when the
server supplies a gateway address, it isn't always clear if the
intent is for the client to set a default route through this
gateway.

By default, ChromeOS creates a default route to the gateway.
This CL adds an ONC configuration option to allow ONC to override
this default behavior, thus providing the means for a "split
tunnel" configuration in situations where the route configuration
provided by the VPN server is amenable to this.  Supplying this
option does not override any explicit routes pushed by the server.

BUG=370460
R=pneubeck@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269942

[Paul Stewart, 2014-05-09 00:59:39]

A new version of service_constants.h needs to be pulled in order for this change
to build.  Can anyone point me at the procedure for that?  I tested this by
cherry-picking the change to service_constants.h in my local build.

[stevenjb, 2014-05-09 01:04:09]

I can do the DEPS roll first thing tomorrow. I'll include you on the review FYI.
lgtm

[pneubeck (no reviews), 2014-05-09 12:17:14]

https://codereview.chromium.org/275723002/diff/1/components/onc/docs/onc_spec...
File components/onc/docs/onc_spec.html (right):

https://codereview.chromium.org/275723002/diff/1/components/onc/docs/onc_spec...
components/onc/docs/onc_spec.html:1025: <dt
class="field">IgnoreDefaultRoute</dt>
could have been a positive setting like Use/Set/EnableDefaultRoute with default
'true', thereby reducing the number of negations.

Totally up to you.

https://codereview.chromium.org/275723002/diff/1/components/onc/docs/onc_spec...
components/onc/docs/onc_spec.html:1035: configurations where the VPN server
omits explicit default routes.
Reading this, it's not clear to me, how this option interacts with the
configuration pushed by the VPN server (e.g. with or with-out
'redirect-gateway').

Can this setting be used to locally override a pushed 'redirect-gateway'?
This sentence suggests 'no' but looking at your commit message of the Shill
change, this is the case.
Please clarify this sentence a bit more.

[Paul Stewart, 2014-05-09 13:25:09]

https://codereview.chromium.org/275723002/diff/1/components/onc/docs/onc_spec...
File components/onc/docs/onc_spec.html (right):

https://codereview.chromium.org/275723002/diff/1/components/onc/docs/onc_spec...
components/onc/docs/onc_spec.html:1025: <dt
class="field">IgnoreDefaultRoute</dt>
On 2014/05/09 12:17:14, pneubeck wrote:
> could have been a positive setting like Use/Set/EnableDefaultRoute with
default
> 'true', thereby reducing the number of negations.
> 
> Totally up to you.

I prefer it this way, since that preserves the default behavior when the flag is
left at its default.

https://codereview.chromium.org/275723002/diff/1/components/onc/docs/onc_spec...
components/onc/docs/onc_spec.html:1035: configurations where the VPN server
omits explicit default routes.
On 2014/05/09 12:17:14, pneubeck wrote:
> Reading this, it's not clear to me, how this option interacts with the
> configuration pushed by the VPN server (e.g. with or with-out
> 'redirect-gateway').
> 
> Can this setting be used to locally override a pushed 'redirect-gateway'?
> This sentence suggests 'no' but looking at your commit message of the Shill
> change, this is the case.
> Please clarify this sentence a bit more.

I'll add some verbiage.  The server configuration could push 'redirect-gateway'
on the client, which should override this parameter.

[Paul Stewart, 2014-05-09 13:31:09]

PTAL (additional redirect-gateway verbiage)

[pneubeck (no reviews), 2014-05-09 13:51:34]

lgtm

[Paul Stewart, 2014-05-09 17:26:55]

I have http://crosreview.com/199092 and http://crosreview.com/199056 to enforce
the gateway route.  This means my text in patch set 2 (the one pneubeck LGTMed)
is now correct.

[pneubeck (no reviews), 2014-05-09 18:03:01]

still lgtm
(note that you can also delete patchsets, just be careful that nobody already
started reviewing them)

[Paul Stewart, 2014-05-12 16:22:44]

The CQ bit was checked by pstew@chromium.org

[commit-bot: I haz the power, 2014-05-12 16:23:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pstew@chromium.org/275723002/60001

[Paul Stewart, 2014-05-12 16:24:35]

The CQ bit was unchecked by pstew@chromium.org

[Paul Stewart, 2014-05-12 18:12:35]

The CQ bit was checked by pstew@chromium.org

[Paul Stewart, 2014-05-12 18:12:46]

The CQ bit was unchecked by pstew@chromium.org

[Paul Stewart, 2014-05-12 18:14:34]

The CQ bit was checked by pstew@chromium.org

[commit-bot: I haz the power, 2014-05-12 18:14:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pstew@chromium.org/275723002/60001

[Paul Stewart, 2014-05-12 19:52:31]

The CQ bit was checked by pstew@chromium.org

[commit-bot: I haz the power, 2014-05-12 19:52:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pstew@chromium.org/275723002/80001

[commit-bot: I haz the power, 2014-05-12 23:31:32]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-05-13 00:51:31]

Change committed as 269942