Decryption API and encrypted zip keys in tests

[GilchristTech]

I'm trying my hand at implementing file decryption in Unzipit. I already have some commits on my local system, including a ZIP compatibility check function which only adds ~450 bytes to the minified JavaScript file.

In the test data directory, there are two zip files with encrypted file records. If I can be provided the keys to these files, I can work on this feature within tests without modifying existing files. Otherwise, I would be happy to make new encrypted zip files for testing.

My use case for this prefers AES encryption, so that's what I'm going to focus on for the time being. I have an idea for how this would be fit with existing data structures, and it would look something like this:

import { unzip } from "@greggman/unzipit";
const {entries} = await unzip(url);

for (const [name, entry] of Object.entries(entries)) {
  const content = await entry.setKey("my-password").arrayBuffer();
  // ...
}

The setKey method would set a private property and return the ZipEntry itself, which would then make it simple to chain a reading method afterwards. For security purposes, I get the impression it would be best to ensure the key is disposed of in some way. In my opinion, any reading operation should dispose of the key from the ZipEntry, and another approach would be that a function like setKey would create a copy of the ZipEntry which tracks the key, and the garbage collector would erase the ZipEntry instance which contains the key.

Making the password part of ZipEntry instead of the archive has several benefits. The ZIP specification does not state that all files within an archive must have the same password or encryption method, and this would allow the user to account for this outside of a configuration more global to the archive. My hope is that it also is closer to the design focus of the library, putting the onus of determining the particulars of how to process the archive on the user. If this approach doesn't, I'm happy to try a different way of organizing decryption features.

[greggman]

Hey, thanks for looking into this.

What do you think about this design?

import { unzip } from "@greggman/unzipit";
import { decryption } from "???"; // could be from unzipit or not
const {entries} = await unzip(url);

for (const [name, entry] of Object.entries(entries)) {
  const content = await entry.decrypt(decryption("my-password")).arrayBuffer();
  // ...
}

I'm not sure that's the right design but ... decryption works by taking a blob of encrypted data and turning it into a blob of un-encrypted data

without encryption:

    compressesed-blob -> decompression -> decompressed-blob

with encryption

    encrypted-blob -> decryption -> un-encrypted-compressed blob -> decompression -> decompressed-blob 

So, from POV of unzipit, it doesn't really need to care about the decryption part. It just needs some function it can call that given and encrypted-blob, returns an unencrypted-blob

This way, there's no actually in-library connection to the decryption and it's easy to not included it if you don't care about supporting encryption.

To put it maybe in typescript terms, something like

type Decryptor = (entry: ZipEntry) => Blob | ArrayBuffer | Promise<Blob | ArrayBuffer>;

class ZipEntry {
  decrypt(fn: Decryptor): ZipEntry;
}

In the example at the top decryption returns a Decryptor which is a function that will decrypt the entry and then return a new ZipEntry that references the decrypted blob

[GilchristTech]

Hi, thanks for getting back to me!

I really like your idea of importing the decryptor from a separate module! Something along those lines would probably be the best approach. It would allow the module to remain tiny, but if other dependencies need to be introduced for fulfilling parts of the spec, it gives the user the ability. My take on how to build off this concept is that instead of the user calling a decryptor function, Unzipit starts with an empty registry of decryptors and other modules register their decryptors to ZIP version numbers. ZIP local file headers specify a encryption methods using the version needed to extract and general purpose bit flag fields, as far as I can tell; and those could be used to index the decryptors.

My thinking is that this approach keeps the compiled script tiny, keeps decryption algorithm code outside of Unzipit, makes it easy for the user to get through their files without having to figure out a much bigger API (for the user, it only adds one method and the need to import another module, I think), and also allows extension for specific features.

Reconciling multiple methods of decryption from a single decryptor function becomes more complicated as more methods are possible to encounter on different iterations. For that reason, I think it would be best for the ZipEntry.arrayBuffer() and ZipEntry.blob() methods to do some sort of check to figure out how to handle encryption. But the module doesn't have to contain any decryption logic itself, just "direct the traffic" to the right place.

I've been brainstorming and putting notes inside unzipit.ts in some uncommited changes, roughly this (abridged):
unzipit.ts

const decryptors_version Array<Decryptor> = [];  // benchmarked (v8): Sparse arrays are
const decryptors_algid Array<Decryptor>   = [];  // probably faster here.

export function addDecryptor (version: number, algid: number, d: Decryptor): void {
  // The version number is a 16-bit integer which denotes both
  // the major and minor version numbers. See APPNOTE.txt section 4.4.3
  // TODO: figure out if this is handling the AlgID fields correctly (section 7.2) 

  if (!Number.isInteger(version) || version < 0 || version > 0xFFFF) {
    throw new TypeError("Invalid ZIP version number");

  } else if (!Number.isInteger(version) || version < 0 || version > 0xFFFF) {
    throw new TypeError("Invalid ZIP AlgId");

  } else {
    decryptors_version[version] = d;
    decryptors_algid[algid] = number;
  }
}

export class ZipEntry {
  private _encryptionKey: string | null;

  constructor(reader: Reader, rawEntry: RawEntry) {
    this._encryptionKey = null;
  }

  // returns a promise that returns an ArrayBuffer for this entry
  async arrayBuffer(): Promise<ArrayBuffer> {
    // Transfer ownership of the encryption key to this function
    const key: string | null = this._encryptionKey;
    this._encryptionKey = null;

    if (!this.encrypted) {
      return await readEntryDataAsArrayBuffer(this._reader, this._rawEntry);

    } else if (key == null) {
      throw new Error(`File ${this.name} cannot be read; file is encrypted and no key was provided`);

    } else if (!(this.versionNeededToExtract in decryptors)) {
      const v = this.versionNeededToExtract;

      // Aside: maybe a custom zip-related error type would be
      // good to have? That way the user could try/catch cases
      // like this to skip files, while allowing other logical
      // errors which may be the result of bugs.

      // TODO: it appears to me that fields which specify that
      // a file is encrypted exist in multiple places. Translate
      // Bits from the general purpose bit flag into compression
      // algorithms, and check support for those. 

      throw new Error(
        `File ${this.name} requires ZIP v${v/10 >> 0}.${v % 10} to extract, but a decryptor does not exist for that version`
      );

    } else {
      // readEntryDataAsArrayBuffer exists, but handles
      // decompression. Maybe split it into some smaller
      // functions? It and readEntryDataAsBlob share the same
      // couple lines at the end, and there's a note about
      // refactoring. I'd be happy to work on that as I make my
      // way through figuring out the best way to read this data,
      // if you would like.
      const buf = await readEntryDataAsArrayBufferRaw(this._reader, this._rawEntry);

      // If decryptors are async, maybe they could be put into
      // web workers? For AES encryption, I'm using SubtleCrypto,
      // which I believe requires the use of asynchronous calls.
      return await decryptors[this.versionNeededToExtract](buf);
    }
  }

  // sets the encryption key for this file until the entry file content
  // is read. Returns this ZipEntry
  useKey(key: string): ZipEntry {
    this._encryptionKey = key;
    return this;
  }
}

Other modules would then add implementations for different encryption functionality into Unzipit like this:
unzipit-aes.js

import { addDecryptor } from "@greggman/unzipit"; // peer dependency, maybe?

async function decryptor (zip_entry, maybe_a_data_view) {
  // ...
}

addDecryptor(51, 0x660E, decryptor); // AES 128
addDecryptor(51, 0x660F, decryptor); // AES 192
addDecryptor(51, 0x6610, decryptor); // AES 256

// I think these different algorithms check the extra data field to discern which one to use, if memory serves. The logic for that could be in the decryptor.

And that way, a user who wants to add processing processing features would only need to import the extension module, causing calls to addDecryptor to be run:
user-code.js

import { unzip } from "@greggman/unzipit";
import "@greggman/unzipit/encryption/aes";

const {entries} = await unzip(url);

for (const [name, entry] of Object.entries(entries)) {
  const content = await entry.useKey("my-password").arrayBuffer();
  // ...
}

Having a registry of decryptors has another benefit, potentially. In version 6.2.0 of the ZIP spec (2004), PKWARE introduced the encryption for the central directory structure. This feature would allow the central directory to be decrypted after parsing the related structures. This is something which exists in some structures next to the encrypted central directory, I believe.

Also, a system like this registry could be used to add other compression methods.

What do you think?

[greggman]

Does unzip need to register anything? I can imagine the decryptor like this

export function decryptor(password: string) {
  return function(entry: ZipEntry) : ZipEntry {
     if (usesZipCrypto(entry)) { 
       ... decrypt with zipCrypto ...
     } else if (usesAESCTR(entry)) {
       ... decrypt with AES-CTR ...
     }
  }
}

If you want implement it moduarly you can separate implementations

import { uzesZipCrypto, decryptZipCrypto } from './zip-crypto.js';
import { usesAESCTR, decryptAESCTR } from './aes-ctr.js';

export function decryptor(password: string) {
  return function(entry: ZipEntry) : ZipEntry {
     if (usesZipCrypto(entry)) { 
       return decryptZipCrypto(entry);
     } else if (usesAESCTR(entry)) {
       return decryptAESCTR(entry);
     }
  }
}

We could add another hook for encrypted centeral directories but those are extemely rare in practice.