Implement a second version of the credentials passing API

Implement a second version of the credentials passing API

In version 2 of the API, initialize() returns the list of supported key
types and add() expects one of these key types to be specified. At the
moment, the only supported key type is KEY_TYPE_PASSWORD_PLAIN but this
can now be expanded without changing the API.

BUG=367847
TEST=Extended browser test

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

[bartfab (slow), 2014-04-28 17:32:25]

Hi Nikita,

Could you please review?

[Nikita (slow), 2014-04-29 11:01:51]

+Xiyuan

[Nikita (slow), 2014-04-29 11:13:13]

lgtm

[xiyuan, 2014-04-29 17:26:35]

https://codereview.chromium.org/258123002/diff/1/chrome/browser/resources/gai...
File chrome/browser/resources/gaia_auth/main.js (right):

https://codereview.chromium.org/258123002/diff/1/chrome/browser/resources/gai...
chrome/browser/resources/gaia_auth/main.js:51: passwordBytes_: null,
This seems to imply that it could contain binary data. How we are going to
handle the binary data? Would they be base64 encoded? Or we are going to use
typed array?

https://codereview.chromium.org/258123002/diff/1/chrome/browser/resources/gai...
chrome/browser/resources/gaia_auth/main.js:288: if
(!call.hasOwnProperty('requested_version')) {
nit: requested_version -> requestedVersion

[bartfab (slow), 2014-04-30 08:52:41]

https://codereview.chromium.org/258123002/diff/1/chrome/browser/resources/gai...
File chrome/browser/resources/gaia_auth/main.js (right):

https://codereview.chromium.org/258123002/diff/1/chrome/browser/resources/gai...
chrome/browser/resources/gaia_auth/main.js:51: passwordBytes_: null,
On 2014/04/29 17:26:35, xiyuan wrote:
> This seems to imply that it could contain binary data. How we are going to
> handle the binary data? Would they be base64 encoded? Or we are going to use
> typed array?

The plan is to use the format used by glibc2, see the "Notes" section here:

http://man7.org/linux/man-pages/man3/crypt.3.html

So it will be a string but one potentially containing magical characters. It
does seem like an odd way encoding but there is precedent for it (in glibc2) and
this is how Will designed the navigator principals API that the credentials
passing API is based on as closely as possible.

https://codereview.chromium.org/258123002/diff/1/chrome/browser/resources/gai...
chrome/browser/resources/gaia_auth/main.js:288: if
(!call.hasOwnProperty('requested_version')) {
On 2014/04/29 17:26:35, xiyuan wrote:
> nit: requested_version -> requestedVersion

Done.

[xiyuan, 2014-04-30 16:46:41]

lgtm

https://codereview.chromium.org/258123002/diff/1/chrome/browser/resources/gai...
File chrome/browser/resources/gaia_auth/main.js (right):

https://codereview.chromium.org/258123002/diff/1/chrome/browser/resources/gai...
chrome/browser/resources/gaia_auth/main.js:51: passwordBytes_: null,
On 2014/04/30 08:52:41, bartfab wrote:
> On 2014/04/29 17:26:35, xiyuan wrote:
> > This seems to imply that it could contain binary data. How we are going to
> > handle the binary data? Would they be base64 encoded? Or we are going to use
> > typed array?
> 
> The plan is to use the format used by glibc2, see the "Notes" section here:
> 
> http://man7.org/linux/man-pages/man3/crypt.3.html
> 
> So it will be a string but one potentially containing magical characters. It
> does seem like an odd way encoding but there is precedent for it (in glibc2)
and
> this is how Will designed the navigator principals API that the credentials
> passing API is based on as closely as possible.

Thanks for clarification. I ask because JS string defaults to UTF16 and it could
do char transcodings, thus not a good container for arbitrary binary data. We
are okay if it only holds printable ASCII. Maybe add a comment to make it more
clear?

[Jorge Lucangeli Obes, 2014-04-30 22:31:37]

https://codereview.chromium.org/258123002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/gaia_auth/main.js (right):

https://codereview.chromium.org/258123002/diff/20001/chrome/browser/resources...
chrome/browser/resources/gaia_auth/main.js:35: 'KEY_TYPE_PASSWORD_PLAIN',
Is this equivalent of what we're doing in v1?

[bartfab (slow), 2014-05-01 22:47:48]

https://codereview.chromium.org/258123002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/gaia_auth/main.js (right):

https://codereview.chromium.org/258123002/diff/20001/chrome/browser/resources...
chrome/browser/resources/gaia_auth/main.js:35: 'KEY_TYPE_PASSWORD_PLAIN',
On 2014/04/30 22:31:37, Jorge Lucangeli Obes wrote:
> Is this equivalent of what we're doing in v1?

Yes. For now, both v1 and v2 expect the password in plain text. In the future,
v2 will also support other algorithms while v1 will be limited to a plain-text
password forever.

Note that we have not publicly launched this API yet. We are under no obligation
to keep v1 of the API around as nobody depends on it. My plan is to make sure
that v2 works as expected and then to drop support for v1 entirely. Nobody will
miss it because nobody is using it yet. When we launch the feature, we will
launch v2 only.

[Jorge Lucangeli Obes, 2014-05-01 23:00:24]

https://codereview.chromium.org/258123002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/gaia_auth/main.js (right):

https://codereview.chromium.org/258123002/diff/20001/chrome/browser/resources...
chrome/browser/resources/gaia_auth/main.js:35: 'KEY_TYPE_PASSWORD_PLAIN',
On 2014/05/01 22:47:48, bartfab wrote:
> On 2014/04/30 22:31:37, Jorge Lucangeli Obes wrote:
> > Is this equivalent of what we're doing in v1?
> 
> Yes. For now, both v1 and v2 expect the password in plain text. In the future,
> v2 will also support other algorithms while v1 will be limited to a plain-text
> password forever.
> 
> Note that we have not publicly launched this API yet. We are under no
obligation
> to keep v1 of the API around as nobody depends on it. My plan is to make sure
> that v2 works as expected and then to drop support for v1 entirely. Nobody
will
> miss it because nobody is using it yet. When we launch the feature, we will
> launch v2 only.

IIUC the password is transmitted in plaintext, but the transport is secure
(HTTPS), right?

[bartfab (slow), 2014-05-02 06:29:53]

https://codereview.chromium.org/258123002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/gaia_auth/main.js (right):

https://codereview.chromium.org/258123002/diff/20001/chrome/browser/resources...
chrome/browser/resources/gaia_auth/main.js:35: 'KEY_TYPE_PASSWORD_PLAIN',
On 2014/05/01 23:00:24, Jorge Lucangeli Obes wrote:
> On 2014/05/01 22:47:48, bartfab wrote:
> > On 2014/04/30 22:31:37, Jorge Lucangeli Obes wrote:
> > > Is this equivalent of what we're doing in v1?
> > 
> > Yes. For now, both v1 and v2 expect the password in plain text. In the
future,
> > v2 will also support other algorithms while v1 will be limited to a
plain-text
> > password forever.
> > 
> > Note that we have not publicly launched this API yet. We are under no
> obligation
> > to keep v1 of the API around as nobody depends on it. My plan is to make
sure
> > that v2 works as expected and then to drop support for v1 entirely. Nobody
> will
> > miss it because nobody is using it yet. When we launch the feature, we will
> > launch v2 only.
> 
> IIUC the password is transmitted in plaintext, but the transport is secure
> (HTTPS), right?

The password is sent to two places:

1) The SAML IdP needs to send the password to its own auth server for
verification. It is the IdP's responsibility that this be done securely. This
has nothing to do with the API being introduced here. You are right that on
Chrome OS, we ensure that the IdP's communication with its own server uses
https. On other plaforms, the IdP could use http. It is their choice, not ours.

2) The IdP can send the password to Chrome / Chrome OS using the API we are
discussing here. That entire communication happens via postMessage(), so there
is no http/https or any other network communication involved. The password is
sent locally on the Chromebook, using HTML5 mechanisms. After the password has
reached the C++ layers of Chrome, it is salted and hashed.

[Jorge Lucangeli Obes, 2014-05-02 18:17:17]

On 2014/05/02 06:29:53, bartfab wrote:
>
https://codereview.chromium.org/258123002/diff/20001/chrome/browser/resources...
> File chrome/browser/resources/gaia_auth/main.js (right):
> 
>
https://codereview.chromium.org/258123002/diff/20001/chrome/browser/resources...
> chrome/browser/resources/gaia_auth/main.js:35: 'KEY_TYPE_PASSWORD_PLAIN',
> On 2014/05/01 23:00:24, Jorge Lucangeli Obes wrote:
> > On 2014/05/01 22:47:48, bartfab wrote:
> > > On 2014/04/30 22:31:37, Jorge Lucangeli Obes wrote:
> > > > Is this equivalent of what we're doing in v1?
> > > 
> > > Yes. For now, both v1 and v2 expect the password in plain text. In the
> future,
> > > v2 will also support other algorithms while v1 will be limited to a
> plain-text
> > > password forever.
> > > 
> > > Note that we have not publicly launched this API yet. We are under no
> > obligation
> > > to keep v1 of the API around as nobody depends on it. My plan is to make
> sure
> > > that v2 works as expected and then to drop support for v1 entirely. Nobody
> > will
> > > miss it because nobody is using it yet. When we launch the feature, we
will
> > > launch v2 only.
> > 
> > IIUC the password is transmitted in plaintext, but the transport is secure
> > (HTTPS), right?
> 
> The password is sent to two places:
> 
> 1) The SAML IdP needs to send the password to its own auth server for
> verification. It is the IdP's responsibility that this be done securely. This
> has nothing to do with the API being introduced here. You are right that on
> Chrome OS, we ensure that the IdP's communication with its own server uses
> https. On other plaforms, the IdP could use http. It is their choice, not
ours.
> 

Yeah, agreed. The discussion on whether we should push them to do better for
their users does not belong on this bug.

> 2) The IdP can send the password to Chrome / Chrome OS using the API we are
> discussing here. That entire communication happens via postMessage(), so there
> is no http/https or any other network communication involved. The password is
> sent locally on the Chromebook, using HTML5 mechanisms. After the password has
> reached the C++ layers of Chrome, it is salted and hashed.

lgtm

[bartfab (slow), 2014-05-05 12:30:50]

On 2014/04/30 16:46:41, xiyuan wrote:
> lgtm
> 
>
https://codereview.chromium.org/258123002/diff/1/chrome/browser/resources/gai...
> File chrome/browser/resources/gaia_auth/main.js (right):
> 
>
https://codereview.chromium.org/258123002/diff/1/chrome/browser/resources/gai...
> chrome/browser/resources/gaia_auth/main.js:51: passwordBytes_: null,
> On 2014/04/30 08:52:41, bartfab wrote:
> > On 2014/04/29 17:26:35, xiyuan wrote:
> > > This seems to imply that it could contain binary data. How we are going to
> > > handle the binary data? Would they be base64 encoded? Or we are going to
use
> > > typed array?
> > 
> > The plan is to use the format used by glibc2, see the "Notes" section here:
> > 
> > http://man7.org/linux/man-pages/man3/crypt.3.html
> > 
> > So it will be a string but one potentially containing magical characters. It
> > does seem like an odd way encoding but there is precedent for it (in glibc2)
> and
> > this is how Will designed the navigator principals API that the credentials
> > passing API is based on as closely as possible.
> 
> Thanks for clarification. I ask because JS string defaults to UTF16 and it
could
> do char transcodings, thus not a good container for arbitrary binary data. We
> are okay if it only holds printable ASCII. Maybe add a comment to make it more
> clear?

Done.

[bartfab (slow), 2014-05-05 12:30:56]

The CQ bit was checked by bartfab@chromium.org

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

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bartfab@chromium.org/258123002/40001

[commit-bot: I haz the power, 2014-05-05 16:17:34]

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:
  linux_chromium_chromeos_rel on tryserver.chromium

[bartfab (slow), 2014-05-05 16:19:01]

The CQ bit was unchecked by bartfab@chromium.org

[bartfab (slow), 2014-05-05 18:08:45]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-05-05 18:10:02]

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

[commit-bot: I haz the power, 2014-05-05 21:43:41]

Change committed as 268278