Implement Networking Private API CreateNetwork function on Windows.

Implement Networking Private API CreateNetwork function on Windows. Allows connection to hidden WiFi networks.
Defaults to TKIP encryption for WAPPSK and AES encryption for WAP2PSK. Exact encryption to use will have to be determined during Connect.

TBR=cpu@chromium.org for +third_party/libxml to DEPS.

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

[mef, 2013-12-06 19:00:58]

Hi guys,

please take a look. 

tbarzic@ - overall.
stevenjb@ - components/onc/onc_constants.*
afontan@ - components/wifi/wifi_service_win.cc

thanks,
-m

[stevenjb, 2013-12-06 23:45:32]

https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
File components/wifi/wifi_service.h (right):

https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
components/wifi/wifi_service.h:133: std::string password;
We should avoid storing wifi passwords (even in memory) if at all possible.

[mef, 2013-12-06 23:57:16]

On 2013/12/06 23:45:32, stevenjb wrote:
>
https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
> File components/wifi/wifi_service.h (right):
> 
>
https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
> components/wifi/wifi_service.h:133: std::string password;
> We should avoid storing wifi passwords (even in memory) if at all possible.

I agree, however the clear text password is needed in CreateNetwork on Windows
and in Connect on Mac.
I'm open to suggestions about better way to pass it in.

[mef, 2013-12-09 17:20:43]

On 2013/12/06 23:57:16, mef wrote:
> On 2013/12/06 23:45:32, stevenjb wrote:
> >
>
https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
> > File components/wifi/wifi_service.h (right):
> > 
> >
>
https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
> > components/wifi/wifi_service.h:133: std::string password;
> > We should avoid storing wifi passwords (even in memory) if at all possible.
> 
> I agree, however the clear text password is needed in CreateNetwork on Windows
> and in Connect on Mac.
> I'm open to suggestions about better way to pass it in.

On somewhat related note, what is expected usage of Network Properties
WiFi.Passphrase field?
I was thinking, that it could be used to mark (true/false) presence of
system-stored password in return of GetProperties method.

[stevenjb, 2013-12-09 17:56:38]

On 2013/12/09 17:20:43, mef wrote:
> On 2013/12/06 23:57:16, mef wrote:
> > On 2013/12/06 23:45:32, stevenjb wrote:
> > >
> >
>
https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
> > > File components/wifi/wifi_service.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
> > > components/wifi/wifi_service.h:133: std::string password;
> > > We should avoid storing wifi passwords (even in memory) if at all
possible.
> > 
> > I agree, however the clear text password is needed in CreateNetwork on
Windows
> > and in Connect on Mac.
> > I'm open to suggestions about better way to pass it in.
> 
> On somewhat related note, what is expected usage of Network Properties
> WiFi.Passphrase field?
> I was thinking, that it could be used to mark (true/false) presence of
> system-stored password in return of GetProperties method.

On Chrome OS we only use that property to set the passphrase, i.e. Chrome ->
Shill (connection manager). We never send the passphrase back to Chrome, and we
don't store the passphrase anywhere in Chrome. If possible we should do the same
thing on Windows and Mac, i.e. pass the passphrase in a set of configuration
properties when configuring / connecting (a Dictionary on Chrome OS), but don't
persist it once the configuration is completed.

[mef, 2013-12-09 18:03:10]

On 2013/12/09 17:56:38, stevenjb wrote:
> On 2013/12/09 17:20:43, mef wrote:
> > On 2013/12/06 23:57:16, mef wrote:
> > > On 2013/12/06 23:45:32, stevenjb wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
> > > > File components/wifi/wifi_service.h (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
> > > > components/wifi/wifi_service.h:133: std::string password;
> > > > We should avoid storing wifi passwords (even in memory) if at all
> possible.
> > > 
> > > I agree, however the clear text password is needed in CreateNetwork on
> Windows
> > > and in Connect on Mac.
> > > I'm open to suggestions about better way to pass it in.
> > 
> > On somewhat related note, what is expected usage of Network Properties
> > WiFi.Passphrase field?
> > I was thinking, that it could be used to mark (true/false) presence of
> > system-stored password in return of GetProperties method.
> 
> On Chrome OS we only use that property to set the passphrase, i.e. Chrome ->
> Shill (connection manager). We never send the passphrase back to Chrome, and
we
> don't store the passphrase anywhere in Chrome. If possible we should do the
same
> thing on Windows and Mac, i.e. pass the passphrase in a set of configuration
> properties when configuring / connecting (a Dictionary on Chrome OS), but
don't
> persist it once the configuration is completed.

That sounds good. In this particular CL the password property is used exactly
like that:
passed to the system as part of WLANProfile XML and discarded.

For future password slurping I'm planning to implement currently stubbed-out
method 
'VerifyAndEncryptCredentials' to encrypt password with devices public key before
passing it back to Chrome.


pass

[stevenjb, 2013-12-09 18:06:34]

https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
File components/wifi/wifi_service.h (right):

https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
components/wifi/wifi_service.h:133: std::string password;
On 2013/12/06 23:45:32, stevenjb wrote:
> We should avoid storing wifi passwords (even in memory) if at all possible.

If this is only used when configuring a network and not persisted, we should add
a comment to that effect.

[mef, 2013-12-09 18:54:51]

PTAL.

https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
File components/wifi/wifi_service.h (right):

https://codereview.chromium.org/105153002/diff/40001/components/wifi/wifi_ser...
components/wifi/wifi_service.h:133: std::string password;
On 2013/12/09 18:06:34, stevenjb wrote:
> On 2013/12/06 23:45:32, stevenjb wrote:
> > We should avoid storing wifi passwords (even in memory) if at all possible.
> 
> If this is only used when configuring a network and not persisted, we should
add
> a comment to that effect.

Done.

[stevenjb, 2013-12-09 18:59:16]

owner lgtm, but somebody familiar with the Windows implementation should review
this also.

[mef, 2013-12-09 19:07:34]

On 2013/12/09 18:59:16, stevenjb wrote:
> owner lgtm, but somebody familiar with the Windows implementation should
review
> this also.

Steven, thanks! Guys, could you take a look:

tbarzic@ - overall.
afontan@ - components/wifi/wifi_service_win.cc (WLANProfile XML creation, etc).

[tbarzic, 2013-12-10 19:42:39]

lgtm

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_ser...
File components/wifi/wifi_service.h (right):

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_ser...
components/wifi/wifi_service.h:146: bool UpdateFromValue(const
base::DictionaryValue& value);
can you add a comment that this updates only properties set in |value|

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_ser...
File components/wifi/wifi_service_win.cc (right):

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_ser...
components/wifi/wifi_service_win.cc:1516: &security_open,
instead of setting |security_open| in AuthEncryptionFromSecurity (feels that the
function does a bit more than just convert security properties, as its name
suggests), I'd do this after the function returns as:

bool security_open = authentication == kAuthenticationOpen &&
                     encryption == kEncryptionNone;

(feel free to keep it as it is if you disagree on semantics)

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_tes...
File components/wifi/wifi_test.cc (right):

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_tes...
components/wifi/wifi_test.cc:148: if (!password.empty()) {
nit: you don't need {}

[mef, 2013-12-10 20:51:42]

Toni, thanks! 

Do you have any comments in regards to AES vs TKIP encryption options?

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_ser...
File components/wifi/wifi_service.h (right):

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_ser...
components/wifi/wifi_service.h:146: bool UpdateFromValue(const
base::DictionaryValue& value);
On 2013/12/10 19:42:39, tbarzic wrote:
> can you add a comment that this updates only properties set in |value|

Done.

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_ser...
File components/wifi/wifi_service_win.cc (right):

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_ser...
components/wifi/wifi_service_win.cc:1516: &security_open,
On 2013/12/10 19:42:39, tbarzic wrote:
> instead of setting |security_open| in AuthEncryptionFromSecurity (feels that
the
> function does a bit more than just convert security properties, as its name
> suggests), I'd do this after the function returns as:
> 
> bool security_open = authentication == kAuthenticationOpen &&
>                      encryption == kEncryptionNone;
> 
> (feel free to keep it as it is if you disagree on semantics)

Done.

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_tes...
File components/wifi/wifi_test.cc (right):

https://codereview.chromium.org/105153002/diff/60001/components/wifi/wifi_tes...
components/wifi/wifi_test.cc:148: if (!password.empty()) {
On 2013/12/10 19:42:39, tbarzic wrote:
> nit: you don't need {}

Done.

[mef, 2013-12-13 15:03:23]

I've added TODO to determine proper WAP encryption (AES vs TKIP) during Connect.

I'm going to land this CL now, so extension could start using CreateNetwork
function, and will create another CL to change encryption if connect to newly
created network is failing.

[commit-bot: I haz the power, 2013-12-13 15:06:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mef@chromium.org/105153002/120001

[commit-bot: I haz the power, 2013-12-13 18:25:18]

Change committed as 240703