Skip to content
This repository has been archived by the owner on Feb 25, 2019. It is now read-only.

Webcrypto #12

Merged
merged 26 commits into from
Apr 21, 2016
Merged

Webcrypto #12

merged 26 commits into from
Apr 21, 2016

Conversation

henrjk
Copy link
Contributor

@henrjk henrjk commented Mar 31, 2016

This is the PR for issue #7 with cleaned up history.
The history still reflects version increments made in my for that is from 0.2 to 0.2.5.
The commits since 0.2.5 are new to this PR mostly to adopt webcrypto-shim from npm which became available now.

This is a larger change which combines the work made overs some time.
In addition to switch the code to use the webcrypto API other
changes were made.
The following is a summary of these changes:

* Uses ES6 as source language which also replaces coffescript for tests.

* Currently the intent is to publish this under npm once it was reviewed
  and accepted into the mainline.

* Version 0.2.0 was introduced to indicate that the former API was
  broken. In particular all functions returning promises are now
  namespaced under Anvil.promise.

* A fallback mechanism is available to allow using crypto libraries
  when WebCrypto is not available or for testing on localhost.

* Claims are now checked

* Chrome replaces PhantomJS as karma test suite browser as PhantomJS does not
  support WebCrypto.

* Uses jspm for development and tests.

* Refactored duplicated code that was initially copied to support both
  angular.js and vanilla js. Differences in the code are now provider by
  the client setting adapter interfaces for HTTP, browser location and DOM
  access.

* Removed bower dependency.

* Removed production dependency of angular js and jquery.

* Removed dependencies on previously used crypto libraries:
- CryptoJS
- KJUR.jws.JWS
- sjcl

* New dependencies:
- jspm
Libs:
- tiny-emitter for events
- bows for logging (see commit messages below)
- text-encoder-lite (encode/decode str as utf-8, see below)
DEV:
- Q for promises (dev),
- q-xhr for HTTP access without jquery (dev)
- webcrypto-shim to support Safari and IE. Used via script tag.

The original work on this change was done over many commits which were
later combined. The following lists the  original headline and comments
for some of these commits. Although some of these might be outdated it
should mostly be useful to get further some details about has been
summarized above.

Significant original commits:

COMMIT: Add plain javascript version

This brings in the following new dependencies:

o Q for promises (https://github.com/kriskowal/q).
Bluebird seemed too large to me for the browser. Q seems very focused.
Angulars' $q service is inspired by Q albeit not the same.

o q-xhr for http requests (https://github.com/nathanboktae/q-xhr).
This is a backport from angular's $http service. It is quite
lightweight.

COMMIT: Handle disconnected popup in passwordless login.

The passwordless login method sends the user a link in an email. When
the
user follows this link a new browser window opens.

However the original popup window will not redirect and remains open
with a page allowing to resend the link.

This change has the popups parent window listens whether an
authentication is established. For this an 'authenticated' event
was introduced. The 'authenticated' event is emitted in response to a
local storage 'anvil.connect' event when the user is authenticated.
This reacts to an authentiation performed in another window or tab.

For the eventing a new dependency was added to `tiny-emitter`.
Here are a few alternative event emitters:
- https://github.com/Olical/EventEmitter
seems very elaborate, a bit too much weight for our scenario
- https://github.com/component/emitter
a similar alternative to tiny-emitter. More or less a coin-toss.
- https://github.com/jeromeetienne/microevent.js
seemed too microi (perhaps)!
- https://github.com/joaquimserafim/tiny-eventemitter.
not too popular. Might be an alternative too.

Presumably this could also be handled with the server. In this case the
'Sent mail' page would have to monitor whether server whether the
session is established. However one would want to avoid publishing the
email link to that window as one could easily steal it and allowing
anyone to login. Perhaps divulging the secret could be avoided similar
to
how OpenID Connect session management works.

Another disadvantage would be that the server would have to be either
polled or sending server events. Again it might be possible to do this
with the existing session events.

An advantage would be that this would allow the normal OpenID connect
authentication flow to happen.

One issue with this solution is that the page opened by the emailed link
activation may not self close as some browser may not allow this from a
script in this case. I did see this work in Chrome and Safari but not
Firefox.

COMMIT: Add support for better logging.

During development for sure there is some need to log how the client
interacts with connect.

While one might want a capability to submit client errors to a server,
this is not what this is for.

Instead this allows to have essentially console statement which are can
be enabled optionally by setting

`localStorage.debug = true`

in the browser dev tools console. To stop logging use

`delete localStorage.debug`

The logging library used is https://github.com/latentflip/bows.

I have read about https://github.com/visionmedia/debug which seems more
popular but it was mentioned that it does not link to the code in its
messages. This is not helpful in my opinion.

I played around a bit with bows and it pretty good to me.

COMMIT: Anvil.nonce and Anvil.sha256url to use web crypto.

Both Anvil.nonce and Anvil.sha256url now return promises.

Unfortunately this means that all consumers will have to adjust.
However apparently there is no real way around this fact.

This required implementing the following conversion or encodings for
ArrayBuffers:
1. encode/decode str as utf-8
2. convert buffer to base64url string
3. in the context of seriale base64 conversion to/from buffer
was needed.

Implementing utf-8 support should be relatively straightforward with
ES6. However I decided to use the npm:text-encoder-lite library instead
which is mentioned by MDN (https://developer.mozilla.org). It appears
that at the moment there is not yet widespread support in the browsers
for TextEncode/TextDecode and so this seemed like a pretty good
approach. However it might also be on option to piggy back on node
packages when using browserify or jspm. However I am not sure this is
desirable.

To install this library I had to shim it this should perhaps be
submitted to the jspm registry. See package.json for the override.

COMMIT: Anvil to use webcrypto except for JWT validation.

Methods changed were:
- callback
- authorize

Also started using ES6 promises and got rid of apiDefer which changes
methods:
- request

This also affected the JWT validators.

COMMIT: add verifyJWT function

There is an issue with the specific jwk used by our tests previously.
Apparently it is not fully compliant and Chrome rejects it.
Currently Firefox is more leniant here, but the Chrome folks aim to
push
Firefox to also implement stronger compliance here.

See the following related issues:
- OADA/rsa-pem-to-jwk#1
- https://code.google.com/p/chromium/issues/detail?id=383998

The key has been fixed by:
1. Decoding its n value to a hex string
"009e1b9b22bf7cba0430...e33a63"
2. Trim off the leading 00 byte:
"9e1b9b22bf7cba0430...e33a63"
3. Compute the base64url representation of this usingi
ab2base64urlstr(hex2ab(<step2-value>))
resulting in value:
"nhu...M6Yw"

COMMIT: callback to use webcrypto

remove crypto code using KJUR.jws.JWS

COMMIT: switch to window.crypto.getRandomValues(bytes)

COMMIT: Added webcrypto-shim to support Safari/IE?

Tested that this works with Safari 9.0.2 on OS-X. That is all
tests pass except the test which generates our own key and signs a
token. Note however that this capability is not actually needed to
consume tokens.

COMMIT: remove dependencies sjcl and jquery

jquery is no longer provided here as the plain js should do
as well. However currently only angular is tested.

Removes:
- "capaj/jspm-hot-reloader"
- "jquery": "github:components/[email protected]",
- "kriskowal/q": "github:kriskowal/[email protected]",
q is still used through "q": "npm:[email protected]",
- "sjcl": "npm:[email protected]",

COMMIT: Fix test isolation issue.

Failing test of:
describe('Check jwk sign verification', () => {
describe('self generated key', () => {
...
beforeEach(done => {

On OS X the signing fails and this casused
failure in the beforeEach done to be called twice.

While investigating change console.log to bows.

Split off extended test useful to gen tokens

The extended tests are not required for verifying jws tokens.
However in case one needs to generate new tokens the code can
be useful.
To observe the bows log statement one must set debug=true in
localStorage.

The extended tests can be run as follows:
1. npm run karma-browsers-extended
2. npm run karma-run

Currently the extended test is known to fail on [Safari 9.0.2
(Mac OS X
10.10.5)] and passes on
- [Firefox 43.0.0 (Mac OS X 10.10.0)]
- [Chrome 47.0.2526 (Mac OS X 10.10.5)]

COMMIT: Make Chrome browser for 'npm test'

webcrypto is not available on PhantomJS so it does not make
sense to
target it at the moment.
This may change when it is available.

COMMIT: remove one describe level in jwk tests

COMMIT: Work around MS Edge not taken use field well.

This could be an Edge bug. If so this should probably be
removed in the future.

See
https://connect.microsoft.com/IE/feedbackdetail/view/2242108/webcryptoapi-importing-jwk-with-use-field-fails

COMMIT: directories.lib src was needed for jspm17@beta

COMMIT: Use webcrypto-shim outside of npm.

webcrypto-shim currently is neither published to npm nor has a
release on github.
For now we are using it outside of package manage that is with
a script tag.

COMMIT: Refactor have module provide crypto ops needed.

This introduces an API implemented currently only by
encryptor-webcrypto which
is intended to support providing an alternative implementation
for example
based on the previous libraries used.

The mechanism on how to tell connect-js an alternative
implementation has not
yet been defined.

COMMIT: Improve jwsvalidator api and introduce fallback mechanism.

Fallback mechanism is in fallbacks.js. Fallbacks are only
used if subtle.crypto is not available.
For testing one can force fallbacks to be used if the origin
is at http://localhost

COMMIT: Now all promise functions are available under Anvil.promise.

Also upgraded version to 0.2.0 to indicate a breaking
change.

Update README.md except for usage section and remove stale
bower.json.

COMMIT: Update travis.ml and karma.conf.js

travis has not been tested yet. Presumably would happen once
a pull request is done

COMMIT: Add checking of claims
angular is still needed for testing.
The code from anvil-connect-angular has been moved to the example also.
In addition the Anvil#init now provides defaults for location and
window/document access. See domApis.js
Added a test for checking the nonce which requires both an access and id token.
The test data is now harvested from running against a connect server.

Also some tightening of the callback validation related code.

While writing new test needed to use done.fail(err) which
required updating our karma/jasmine dependency.
Avoid using Array.find or array.prototype.find. as IE 11 does not
support support the array.find() method. Alternatively a polyfill could
(see https://github.com/paulmillr/es6-shim) and
individual https://github.com/paulmillr/Array.prototype.find.

But found it easier to simply avoid this code.

IE 11 failed test when cookies are not stored.

The test that failed was:
''callback with bad signature for authorization response and jws-validator-decodeonly''

The symptom was that the cookie storing the encryption secret is not actually stored
and so deserialize() could not decrypt the session which then caused deserialize
to reset the session. The deserialise call is made when the localStorage event for
key 'anvil.connect' is triggered which is caused by calling serialize from the
Anvil.promise.callback method that the test is validating.

Now the reason the secret cooky was not stored is that the test mocks the time
so that the access token's time claims check out. However while this sets
an expire time of the cookie relative to the mocked time, it appears that IE
actually used the real time to let the cookie expire.

I have not checked whether this affected only IE. Another possibility is that
the timing of calling the promise handlers and the localStorage event
is different for the more modern browsers vs IE.
At any rate I don't think that deserialise should reset the session data.

Fixing this does change this test to succeed on all the tested browsers:

Safari 9.0.3 (Mac OS X 10.10.5): Executed 155 of 173 (skipped 18) SUCCESS (0.82 secs / 0.728 secs)
Firefox 43.0.0 (Mac OS X 10.10.0): Executed 155 of 173 (skipped 18) SUCCESS (0.904 secs / 0.781 secs)
Chrome 48.0.2564 (Mac OS X 10.10.5): Executed 155 of 173 (skipped 18) SUCCESS (0.748 secs / 0.674 secs)
IE 11.0.0 (Windows 10 0.0.0): Executed 155 of 173 (skipped 18) SUCCESS (0.776 secs / 0.787 secs)
Edge 13.10586.0 (Windows 10 0.0.0): Executed 155 of 173 (skipped 18) SUCCESS (1.894 secs / 1.801 secs)
jspm install is now called during npm postinstall.
This also means that users would not need to know
that jspm is involved in running the tests.
dist produced bundles which allow  usage by script tags.

Here is an example:
    <script src="node_modules/q/q.js"></script>
    <script src="node_modules/q-xhr/q-xhr.js"></script>
    <script src="node_modules/anvil-connect-js/dist/anvil-connect.src.js"></script>
    <script src="anvil-config-global.js"></script>
    <script src="scripts/rp.js"></script>

in rp.js:

Anvil = Anvil.default

function copy(dst, src) {
  for (var prop in src) {
    if (src.hasOwnProperty(prop))
      dst[prop] = src[prop];
  }
  return dst;
}

Anvil.init(copy({
    redirect_uri: anvilConfig.app_server + '/rp.html',
    scope: 'realm'
  }, anvilConfig), {
  http: {
    request: function (config) {
      return Q.xhr(config)
    },
    getData: function (response) {
      return response.data
    }
  }
});

Anvil.promise.deserialize().catch(function (err) {
  Anvil.reset()
});
Also added more logging to facilitate finding issues.
This is useful for angular to listen for logouts discovered via session management
so that it can invoke $rootScope.$apply().

Reverted a change when deserialize cannot restore a session to leave it alone.
Now instead it will set the session to an empty object and also update sessionState
from localStorage.
Furthermore it will emit the 'not-authenticated' event.

A subscriber may listen like so:

Anvil.on('not-authenticated, function(emptySession) {
   $rootScope.$apply()
})

Also reduced logging but left comments for checkSession to reduce noise.
webcrypto-shim did not have an npm published version before.
But recently vibornoff/webcrypto-shim#4
changed this.

To allow using this jspm I made some manual changes
to remove references to the master branch and then installed
with

jspm install npm:webcrypto-shim -o "{format: 'global'}"
@henrjk
Copy link
Contributor Author

henrjk commented Mar 31, 2016

Seems like the travis builds are hitting a rate limit.
Here is a gist https://gist.github.com/topheman/25241e48a1b4f91ec6d4 which may fix this.

The appropriate token must be set either by adding the encrypted
variable JSPM_GITHUB_AUTH_TOKEN or by setting it on the travis project
settings.
This must be done by the anvilresearch organization
See [update Firefox and Chromium · Issue #3475](travis-ci/travis-ci#3475)
@henrjk
Copy link
Contributor Author

henrjk commented Apr 1, 2016

I made changes for the travis to build to work. However this requires configuring a personal github access token which I can't do for the PR.
I created another branch in my fork and also setup a travis build there to make sure it works. This build is at https://travis-ci.org/henrjk/connect-js/builds/120051585

@henrjk
Copy link
Contributor Author

henrjk commented Apr 1, 2016

The new section Github rate limit fix for travis build describes how to have jspm authenticate with GitHub to avoid rate limits. Some of these steps have already been done in .travis.yml. However to make the PR build under the anvilresearch organisation the personal Github access token has to be created under that account and then encrypted using travis for this repository and not my fork.

Currently the .travis.yml file aims to logon with an empty token and so there are login errors in the build instead of rate limit error:

 Unauthorized response for GitHub API.
 Use jspm registry config github to reconfigure the credentials, or update them in your ~/.netrc file. 

As mentioned in my previous comment above this worked for my own travis builds and fork at https://travis-ci.org/henrjk/connect-js/builds/120051585

@tomkersten
Copy link
Contributor

@henrjk any idea how to fix the travis-ci issue on this? i glanced at it quickly and it looks like there was a conflict w/ fsevents, but didn't dig in at all...

@henrjk
Copy link
Contributor Author

henrjk commented Apr 14, 2016

@tomkersten The fsevents issue should be caused by the travis not being able to login into github. The PR is now configured to do so, but the corresponding github token must be created by the anvilresearch organisation (afaik). Authenticated github access is needed so that jspm does not fail with rate limits as the original builds did. See my comment from 13 days ago for more details.
Let me know if you have additional question.
I will be offline soon for today. Let me know if you have more questions.

@tomkersten
Copy link
Contributor

I'm sorry for not reading that just now...I had glanced over it when you posted it, but thought it had been taken care of. Will investigate.

@dmitrizagidulin
Copy link
Member

+1 to merging, since this is the right direction to go in.
(I understand that the TravisCI integration / key will need to be sorted out)

@christiansmith christiansmith merged commit 5600ee7 into anvilresearch:webcrypto-api Apr 21, 2016
@christiansmith
Copy link
Member

Sorry to everyone this has taken so long. I've been spread pretty thin. It's now merged into the webcrypto-api branch of this repo. We can work out any issues like travis integration in that branch.

I also want to verify how it works with OIDC Sessions as implemented by Anvil Connect before merging and releasing to npm.

Also, thank you @henrjk very much for your work on this. Huge step forward!

@dmitrizagidulin
Copy link
Member

Awesome, ok! Will be testing this branch, then.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants