joebandenburg / libaxolotl-javascript

A JavaScript implementation of axolotl. Axolotl is a ratcheting forward secrecy protocol.
GNU Lesser General Public License v3.0
75 stars 13 forks source link

Remove Traceur, V8-incompatible syntax sugar to run in node directly #15

Open dcposch opened 8 years ago

dcposch commented 8 years ago

Node v4 supports most of ES6 by default now! This makes debugging easier.

Changes

joebandenburg commented 8 years ago

Unfortunately, this library is designed to be run by both Node.js and in the browser. Removing Traceur means that it won't run in the browser. It would still be possible via Webpack, but we'd certainly need to update the README to explain this.

dcposch commented 8 years ago

You're right... I thought it worked, but apparently only in Chrome, FF, and Edge, not in IE or Safari. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function*#Browser_compatibility

Annoying.

Curious, is anyone using libaxolotl in a webapp? Using it in a node program, an Electron app, or Chrome app seems reasonable, since all of those can be installed in a reasonably verifiable way (using shrinkwrap etc for reproducible builds, signing each release, etc).

Doing end-to-end encryption in an webapp, on the other hand, seems questionable. Every time you load the app, you're trusting the server to send you untampered-with code, at which point you might as well trust it to handle the messages as well.

joebandenburg commented 8 years ago

Curious, is anyone using libaxolotl in a webapp? Using it in a node program, an Electron app, or Chrome app seems reasonable, since all of those can be installed in a reasonably verifiable way (using shrinkwrap etc for reproducible builds, signing each release, etc).

That's a good point. I agree that it should not be used for online webapps currently - although I recently become aware that progress is being made in that area.

My concern is that there may be offline JavaScript environments out there that aren't ES6 compatible. As I said, though, it's perfectly reasonable to assume that they could use Babel or Traceur themselves so this library doesn't have to.

I can see the benefit, it would certainly simplify the build process for this library.

(Locally I've replaced them with standard,

What is standard?

I'm happy to drop JSCS and JSHint in favour of something better. I've used ESLint recently, which I like.

alax commented 8 years ago

@joebandenburg standard is http://standardjs.com/index.html

dcposch commented 8 years ago

standard uses ESHint under the hood, and is nice in that it requires no configuration. It's used by a bunch of projects including npm itself

If you prefer to keep the semicolons, I can use semistandard instead :)

dcposch commented 8 years ago

Subresource integrity is cool by the way, but doesn't provide a way to install a webapp securely. You still trust the server every time you use the app.

What it does provide is a way to use CDNs etc securely: instead of just including, say, <script src="https://some-cdn.biz/yolo">, you include <script src="https://somecdn.biz/yolo" integrity="<sha512-hash>" />, so you no longer have to trust the CDN to serve untampered code. The user still has to trust you to serve correct code each time they open the webapp.

joebandenburg commented 8 years ago

It seems to me that, as it stands, this pull request is changing two things at once:

  1. Making the code run natively in Node 4.2.
  2. Changing the style of the code.

Would you mind moving the style changes out of this pull request? I think I'm sold on 1 but not on 2.

jcbrand commented 8 years ago

Curious, is anyone using libaxolotl in a webapp? Using it in a node program, an Electron app, or Chrome app seems reasonable, since all of those can be installed in a reasonably verifiable way (using shrinkwrap etc for reproducible builds, signing each release, etc).

That's a good point. I agree that it should not be used for online webapps currently - although I recently become aware that progress is being made in that area.

For what it's worth, I'm keeping an eye on the development of this library because I'm considering using it to add OMEMO encryption support to converse.js which is a browser-JS XMPP chat client. OMEMO relies on Axolotl.

Converse.js is a purely client-side JS library that only communicates with an XMPP server and with no other backend web application. So this is a usecase where this library would be used purely in the browser.

Previously I've added OTR support (via otr.js ) to converse.js and it remains a relatively popular feature. People have since asked for OMEMO support as well.

Thanks and keep up the good work.

dcposch commented 8 years ago

@joebandenburg makes sense, fixed!

To get all Grunt tasks working again, I had to:

LMK if this looks reasonable, and also whether you want me to squash the commits.

dcposch commented 8 years ago
$ npm test && ./node_modules/grunt-cli/bin/grunt integration-test
> axolotl@1.3.0 test /home/dc/email/libaxolotl-javascript
> grunt test

Running "jshint:all" (jshint) task
>> 27 files lint free.

Running "jscs:all" (jscs) task
>> 26 files without code style errors.

Running "mochaTest:unitTests" (mochaTest) task

  ArrayBufferUtils
    areEqual
      ✓ returns true for equal arrays 
      ✓ returns false for arrays of unequal length 
      ✓ returns false for arrays of equal length but unequal values 
    concat
      ✓ concats all buffers into one large buffer if passed as varargs 
      ✓ concats all buffers into one large buffer if passed as an array 
    stringify
      ✓ converts an arrayBuffer to a string 
    parse
      ✓ converts a string to an arrayBuffer 

  FakeCrypto
    generateKeyPair
      ✓ returns key pair 
    calculateAgreement
      ✓ works 

  Axolotl
    ✓ is frozen 
    methods
      generateIdentityKeyPair
        ✓ returns value from generateKeyPair 
        ✓ calls generateKeyPair once 
      generateRegistrationId
        ✓ calls randomInt once 
        ✓ returns non-extended registration ids in the range [1, 16380] 
        ✓ returns extended registration ids in the range [1, MAX_INT] 
      generatePreKeys
        ✓ returns a list of pre keys of length count 
        ✓ calls generateKeyPair count times 
        ✓ returns generateKeyPair output 
        ✓ returns records with ascending ids start from start 
        ✓ wraps ids when they reach 2^24-2 
      generateLastResortPreKey
        ✓ returns a single pre key 
        ✓ calls generateKeyPair once 
      generateSignedPreKey
        ✓ returns a single pre key 
        ✓ calls generateKeyPair once 
        ✓ calls sign once 
        ✓ calls sign with correct arguments 
    communication
      ✓ accepts a simple exchange 
      ✓ accepts a simple multi message exchange 
      ✓ sends the first message as a PreKeyWhipserMessage 
      ✓ sends the second message as a WhipserMessage 
      ✓ accepts multiple messages sent by one party 
      ✓ accepts out of order message delivery (sub ratchet) 
      ✓ accepts a complicated message exchange (70ms)
      ✓ rejects messages that come after too many dropped messages 
      ✓ rejects duplicate PreKeyWhisperMessage delivery 
      ✓ rejects duplicate WhisperMessage delivery 
      ✓ rejects message with bad mac 
      ✓ rejects bad signedPreKey signature 
      ✓ rejects preKeyBundle if neither preKey nor signedPreKey are present 
      ✓ accepts optional oneTimePreKey 
      ✓ queues and eventually completes concurrent encryptMessage 
      ✓ queues and eventually completes concurrent decryptPreKeyWhisperMessage 
      ✓ queues and eventually completes concurrent decryptWhisperMessage 
      ✓ rejects version 2 of PreKeyWhisperMessage 
      ✓ accepts out of order message delivery (main ratchet) (45ms)
      ✓ rejects messages that are too far out of order (main ratchet) (57ms)
      ✓ returns the identity key and registration id when decrypting a PreKeyWhisperMessage 
      simultaneous session initiation
        ✓ handles both parties simultaneously initiating sessions 
        ✓ handles a dropped PreKeyWhisperMessage 
        ✓ handles a dropped first WhisperMessage 
        ✓ handles a long exchange of session disagreements (261ms)

  HKDF
    deriveSecretsWithSalt
      ✓ works with small inputs 
      ✓ works with large inputs 

  PromiseInterfaceDecorator
    ✓ throws if the passed in object doesn't implement the required interface 
    ✓ wraps the object's methods so that they return promises 

  Session
    ✓ is initially empty 
    ✓ clones the passed in state 
    addState
      ✓ puts the session onto the front of sessions array 
      ✓ removes the last element if the list is full 
    removeState
      ✓ removes the session from the list 
    mostRecentState
      ✓ returns the head of the list 

  SessionCipher
    decryptMessage
      ✓ rejects message with wrong version number 

  62 passing (926ms)

Done, without errors.
Running "mochaTest:integrationTests" (mochaTest) task

  axolotl integration node
    ✓ produces the expected output 

  1 passing (12ms)

Done, without errors.
dcposch commented 8 years ago

It seems to me that, as it stands, this pull request is changing two things at once:

  1. Making the code run natively in Node 4.2.
  2. Changing the style of the code.

@joebandenburg I've updated the PR to match the existing code style. Thoughts?