Cryptography code review wanted

Ionic Framework ionic-v3
1

Hi! I’m new to cryptography. I’m trying to write a provider that encrypts data (text or JSON) before it is sent to a server, such that the server cannot read the data. For now only the user himself should be able to decrypt the data, but he should be able to do so on different devices (and possibly in a PWA in the browser).

The private key is created from a password with PBKDF2. This key is then directly used to encrypt the data with AES-GCM. According to here there is no real benefit of using an intermediate content encryption key. Is this correct?

The encryption is done with js-jose and WebCryptoAPI. The encrypted data will be sent to the server as a JWE string together with the password salt. On the next device the user can get the JWE and the salt, create the same key with the password and decrypt the data.

Are there any major flaws in this design? Any comment/feedback will be appreciated.

Update for future visitors: key storage removed.

Here is the code I have so far:

import { Injectable } from '@angular/core';
import { Storage } from '@ionic/storage';

import { TextEncoder } from 'text-encoding';
import { Jose, JoseJWE, Encrypter, Decrypter } from 'jose-jwe-jws';

@Injectable()
export class CryptoProvider {

  private encoder = new TextEncoder();
  private encrypter: Encrypter;
  private decrypter: Decrypter;
  private alg: string = 'AES-GCM';
  private keyUsage: string[] = ['encrypt', 'decrypt'];

  constructor(private storage: Storage) {
    console.log('Hello CryptoProvider Provider');
  }

  encodeString(text: string): Uint8Array {
    return this.encoder.encode(text);
  }

  generateKey(password: string, salt: ArrayBufferView): PromiseLike<CryptoKey> {
    let passwordUtf8 = this.encodeString(password);
    return crypto.subtle.importKey('raw', passwordUtf8, {name: 'PBKDF2'}, false, ['deriveKey']).then((baseKey) => {
      let params = {
        name: 'PBKDF2',
        salt: salt,
        iterations: 5000,
        hash: 'SHA-256'
      }
      return crypto.subtle.deriveKey(params, baseKey, {name: this.alg, length: 256}, false, this.keyUsage)
    });
  }

  createNewKey(password: string): Promise<void> {
    let salt = crypto.getRandomValues(new Uint8Array(16));
    return this.storage.set('salt', salt).then(() => {
      return this.generateKey(password, salt).then((key) => {
        this.initCryptographer(key);
      });
    });
  }

  recreateKeyFromPassword(password: string): Promise<void> {
    return this.storage.get('salt').then((salt) => {
      return this.generateKey(password, salt).then((key) => {
        this.initCryptographer(key);
      });
    })
  }

  initCryptographer(key: CryptoKey): void {
    let cryptographer = new Jose.WebCryptographer();
    cryptographer.setKeyEncryptionAlgorithm('dir');
    this.encrypter = new JoseJWE.Encrypter(cryptographer,key);
    this.decrypter = new JoseJWE.Decrypter(cryptographer,key);
  }

  removeKey(): void {
    this.encrypter = null;
    this.decrypter = null;
  }

  // returns JWE: HEADER.KEY.IV.TEXT.INTEGRITY
  encryptText(plaintext: string): PromiseLike<string> {
    return this.encrypter.encrypt(plaintext);
  }

  decryptText(ciphertext: string): PromiseLike<string> {
    return this.decrypter.decrypt(ciphertext);
  }

}

Sorry for the long post. Thanks for reading!

2

I didn’t checked your code in details but thought I gonna share some overall thought, don’t know if they are relevant, hope so…

  1. About “The private key will be stored on the device” I’m not sure about it. Should your private key be considered as important as a password (for example)? If yes, then well I would advice not to save it locally. There is an interesting article from @joshmorony about basic security https://www.joshmorony.com/basic-security-for-ionic-cordova-applications/ and I tend to be agree with it.

  2. If not store on the device, maybe your app could make a first request thru https to your server where, with a token validation, your server serve then the private key respectively you will then save a token in the local storage

  3. Are you gonna ship your app in the US (for example)? If yes and of course I don’t know your business, maybe the cryptography is a must, but, isn’t going to be a bit more difficult to make your app being accepted by the App Store if you add another level of cryptography more than https? Of course that should be possible, but that will had complexity to the process I think…

3

I use scrypt instead of PBKDF2 for this purpose, but there’s nothing inherently wrong with your choice. Pretty much everything you describe looks solid to me with one glaring exception: don’t store the private key anywhere. When decrypting, require the user to enter the passphrase again and rerun the KDF.

One other trick I’ve found helpful when dealing with WebCrypto: wrap everything coming from it in Promise.resolve(), and that way you will have a proper zone-aware Promise for the rest of your app.

4

Thank you both for your comments. Much appreciated. I will remove the storing part then.

@rapropos What implementation of scrypt are you using? I haven’t found it in WebCryptoAPI.

5

scrypt-async.

1 Like
6

I wil.be trying your solutions soon :slight_smile:

1 Like
7

If I use the above approach of using user’s password as the encryption key, what is the encryption-decryption logic flow if a user forgets his/her password? How to decrypt (say, the encrypted local DB) if the user resets password?

8

The idea of the approach above is that absolutely nobody can decrypt the data except for the user with the password. So if the user forgets the password, there is no way of decrypting the data (apart from cracking the encryption). I guess in cryptography there is always a trade-off between security and convenience.

If the user still knows the old password, you can allow them to change the password by first decrypting everything with the old password and then encrypting it again with a new password.

9

Here’s a solution: