Merge pull request #897 from zgroza/remove_v1_wbn_sign

Remove v1 integrity block format support from wbn-sign

.gitignore

@@ -14,3 +14,4 @@ draft-yasskin-dispatch-web-packaging.xml
 /lib
 loading.html
 subresource-loading.html
+.vscode/

js/sign/README.md

@@ -110,8 +110,6 @@ There are the following command-line flags available:
 - (optional) `--output <filePath>` (`-o <filePath>`)  
   which takes the path to the wanted signed web bundle output. Default:
   `signed.swbn`.
-- (optional) `--version <version>`  
-  which can be either `v1` or `v2`, defaulting to `v1`. Sets the integrity block format.
 - (required if more than one key is provided) `--web-bundle-id <web-bundle-id>`  
   which takes the `web-bundle-id` to be associated with the web bundle.
 
@@ -130,7 +128,6 @@ wbn-sign \
 -o ~/path/to/signed-webbundle.swbn \
 -k ~/path/to/ed25519key.pem \
 -k ~/path/to/ecdsa_p256key.pem
---version v2 \
 --web-bundle-id \
   amfcf7c4bmpbjbmq4h4yptcobves56hfdyr7tm3doxqvfmsk5ss6maacai
 ```
@@ -194,6 +191,9 @@ environment variable named `WEB_BUNDLE_SIGNING_PASSPHRASE`.
 
 ## Release Notes
 
+### v0.2.2
+- BREAKING CHANGE: Removed support for v1 integrity block format.
+
 ### v0.2.1
 
 - Moved is_v2 to the last and optional (defaulting to true) argument of

js/sign/package.json

@@ -1,6 +1,6 @@
 {
   "name": "wbn-sign",
-  "version": "0.2.1",
+  "version": "0.2.2",
   "description": "Signing tool to sign a web bundle with integrity block",
   "homepage": "https://github.com/WICG/webpackage/tree/main/js/sign",
   "main": "./lib/wbn-sign.cjs",

js/sign/src/cli-sign.ts

@@ -19,9 +19,6 @@ const program = new Command()
 
 function readOptions() {
   return program
-    .addOption(
-      new Option('--version <version>').choices(['v1', 'v2']).default('v2')
-    )
     .requiredOption(
       '-i, --input <file>',
       'input web bundle to be signed (required)'
@@ -37,30 +34,10 @@ function readOptions() {
     )
     .option('--web-bundle-id <web-bundle-id>', 'web bundle ID (only for v2)')
     .action((options) => {
-      switch (options.version) {
-        case 'v1':
-          {
-            if (options.privateKey.length > 1) {
-              throw new Error(
-                `It's not allowed to specify more than one private key for v1 signing.`
-              );
-            }
-            if (options.webBundleId) {
-              throw new Error(
-                `It's not allowed to specify --web-bundle-id for v1 signing.`
-              );
-            }
-          }
-          break;
-        case 'v2':
-          {
-            if (options.privateKey.length > 1 && !options.webBundleId) {
-              throw new Error(
-                `--web-bundle-id must be specified if there's more than 1 signing key involved.`
-              );
-            }
-          }
-          break;
+      if (options.privateKey.length > 1 && !options.webBundleId) {
+        throw new Error(
+          `--web-bundle-id must be specified if there's more than 1 signing key involved.`
+        );
       }
     })
     .parse(process.argv)
@@ -80,10 +57,9 @@ export async function main() {
     ? options.webBundleId
     : new WebBundleId(privateKeys[0]).serialize();
   const signer = new IntegrityBlockSigner(
-    webBundle,
+    Uint8Array.from(webBundle),
     webBundleId,
     privateKeys.map((privateKey) => new NodeCryptoSigningStrategy(privateKey)),
-    /*is_v2=*/ options.version === 'v2'
   );
   const { signedWebBundle } = await signer.sign();
   greenConsoleLog(`${webBundleId}`);

js/sign/src/signers/integrity-block-signer.ts

@@ -2,7 +2,6 @@ import crypto, { KeyObject } from 'crypto';
 import * as cborg from 'cborg';
 import {
   INTEGRITY_BLOCK_MAGIC,
-  VERSION_B1,
   VERSION_B2,
 } from '../utils/constants.js';
 import { checkDeterministic } from '../cbor/deterministic.js';
@@ -22,22 +21,18 @@ type IntegritySignature = {
 };
 
 export class IntegrityBlockSigner {
-  // `webBundleId` is ignored if `is_v2` is false.
   constructor(
     private readonly webBundle: Uint8Array,
     private readonly webBundleId: string,
     private readonly signingStrategies: Array<ISigningStrategy>,
-    private readonly is_v2: boolean = true
   ) {}
 
   async sign(): Promise<{
     integrityBlock: Uint8Array;
     signedWebBundle: Uint8Array;
   }> {
     const integrityBlock = this.obtainIntegrityBlock().integrityBlock;
-    if (this.is_v2) {
-      integrityBlock.setWebBundleId(this.webBundleId);
-    }
+    integrityBlock.setWebBundleId(this.webBundleId);
 
     const signatures = new Array<IntegritySignature>();
     for (const signingStrategy of this.signingStrategies) {
@@ -104,7 +99,7 @@ export class IntegrityBlockSigner {
         'IntegrityBlockSigner: Re-signing signed bundles is not supported yet.'
       );
     }
-    return { integrityBlock: new IntegrityBlock(this.is_v2), offset: 0 };
+    return { integrityBlock: new IntegrityBlock(), offset: 0 };
   }
 
   calcWebBundleHash(): Uint8Array {
@@ -172,14 +167,9 @@ export class IntegrityBlock {
   private attributes: Map<string, string> = new Map();
   private signatureStack: IntegritySignature[] = [];
 
-  constructor(private readonly is_v2: boolean = false) {}
+  constructor() {}
 
   setWebBundleId(webBundleId: string) {
-    if (!this.is_v2) {
-      throw new Error(
-        'setWebBundleId() is only available for v2 integrity blocks.'
-      );
-    }
     this.attributes.set('webBundleId', webBundleId);
   }
 
@@ -188,27 +178,15 @@ export class IntegrityBlock {
   }
 
   toCBOR(): Uint8Array {
-    if (this.is_v2) {
-      return cborg.encode([
-        INTEGRITY_BLOCK_MAGIC,
-        VERSION_B2,
-        this.attributes,
-        this.signatureStack.map((integritySig) => {
-          // The CBOR must have an array of length 2 containing the following:
-          // (0) attributes and (1) signature. The order is important.
-          return [integritySig.signatureAttributes, integritySig.signature];
-        }),
-      ]);
-    } else {
-      return cborg.encode([
-        INTEGRITY_BLOCK_MAGIC,
-        VERSION_B1,
-        this.signatureStack.map((integritySig) => {
-          // The CBOR must have an array of length 2 containing the following:
-          // (0) attributes and (1) signature. The order is important.
-          return [integritySig.signatureAttributes, integritySig.signature];
-        }),
-      ]);
-    }
+    return cborg.encode([
+      INTEGRITY_BLOCK_MAGIC,
+      VERSION_B2,
+      this.attributes,
+      this.signatureStack.map((integritySig) => {
+        // The CBOR must have an array of length 2 containing the following:
+        // (0) attributes and (1) signature. The order is important.
+        return [integritySig.signatureAttributes, integritySig.signature];
+      }),
+    ]);
   }
 }

js/sign/src/utils/constants.ts

@@ -13,5 +13,4 @@ export const PUBLIC_KEY_ATTRIBUTE_NAME_MAPPING = new Map<SignatureType, string>(
 export const INTEGRITY_BLOCK_MAGIC = new Uint8Array([
   0xf0, 0x9f, 0x96, 0x8b, 0xf0, 0x9f, 0x93, 0xa6,
 ]); // 🖋📦
-export const VERSION_B1 = new Uint8Array([0x31, 0x62, 0x00, 0x00]); // 1b\0\0
 export const VERSION_B2 = new Uint8Array([0x32, 0x62, 0x00, 0x00]); // 2b\0\0

js/sign/tests/integrity-block-signer_test.js

@@ -10,7 +10,7 @@ import url from 'url';
 const __dirname = path.dirname(url.fileURLToPath(import.meta.url));
 const TEST_WEB_BUNDLE_HASH =
   '95f8713d382ffefb8f1e4f464e39a2bf18280c8b26434d2fcfc08d7d710c8919ace5a652e25e66f9292cda424f20e4b53bf613bf9488140272f56a455393f7e6';
-const EMPTY_INTEGRITY_BLOCK_HEX = '8348f09f968bf09f93a6443162000080';
+const EMPTY_INTEGRITY_BLOCK_HEX = '8448f09f968bf09f93a64432620000a080';
 const TEST_ED25519_PRIVATE_KEY = `-----BEGIN PRIVATE KEY-----
 MC4CAQAwBQYDK2VwBCIEIB8nP5PpWU7HiILHSfh5PYzb5GAcIfHZ+bw6tcd/LZXh
 -----END PRIVATE KEY-----`;
@@ -66,11 +66,11 @@ describe('Integrity Block Signer', () => {
     const contents = fs.readFileSync(file);
     const signer = new wbnSign.IntegrityBlockSigner(
       contents,
-      /*webBundleId=*/ webBundleId,
+      /*webBundleId=*/ webBundleId ||
+        new wbnSign.WebBundleId(privateKeys[0]).serialize(),
       privateKeys.map(
         (privateKey) => new wbnSign.NodeCryptoSigningStrategy(privateKey)
-      ),
-      /*is_v2=*/ !!webBundleId
+      )
     );
     return signer;
   }
@@ -120,10 +120,11 @@ describe('Integrity Block Signer', () => {
     );
 
     const decoded = cborg.decode(cbor);
-    expect(decoded.length).toEqual(3);
+    expect(decoded.length).toEqual(4);
     expect(decoded[0]).toEqual(constants.INTEGRITY_BLOCK_MAGIC);
-    expect(decoded[1]).toEqual(constants.VERSION_B1);
-    expect(decoded[2]).toEqual([]);
+    expect(decoded[1]).toEqual(constants.VERSION_B2);
+    expect(decoded[2]).toEqual({});
+    expect(decoded[3]).toEqual([]);
   });
 
   it('calculates the hash of the web bundle correctly.', () => {
@@ -173,7 +174,7 @@ describe('Integrity Block Signer', () => {
       const hexHashString =
         /*64*/ '0000000000000040' +
         TEST_WEB_BUNDLE_HASH +
-        /*16*/ '0000000000000010' +
+        /*17*/ '0000000000000011' +
         EMPTY_INTEGRITY_BLOCK_HEX +
         attributesCborHex;
 
@@ -195,18 +196,29 @@ describe('Integrity Block Signer', () => {
       const sigAttr = {
         [utils.getPublicKeyAttributeName(keypair.publicKey)]: rawPubKey,
       };
+      const webBundleId = new wbnSign.WebBundleId(
+        keypair.privateKey
+      ).serialize();
+
+      const ibWithoutSignatures = new wbnSign.IntegrityBlock();
+      ibWithoutSignatures.setWebBundleId(webBundleId);
+
       const dataToBeSigned = signer.generateDataToBeSigned(
         signer.calcWebBundleHash(),
-        new wbnSign.IntegrityBlock().toCBOR(),
+        ibWithoutSignatures.toCBOR(),
         cborg.encode(sigAttr)
       );
 
       const ib = cborg.decode((await signer.sign()).integrityBlock);
-      expect(ib.length).toEqual(3);
+      expect(ib.length).toEqual(4);
+
+      const [magic, version, attributes, signatureStack] = ib;
 
-      const [magic, version, signatureStack] = ib;
       expect(magic).toEqual(constants.INTEGRITY_BLOCK_MAGIC);
-      expect(version).toEqual(constants.VERSION_B1);
+      expect(version).toEqual(constants.VERSION_B2);
+      expect(attributes).toEqual({
+        webBundleId: webBundleId,
+      });
       expect(signatureStack.length).toEqual(1);
       expect(signatureStack[0].length).toEqual(2);
 
@@ -253,7 +265,7 @@ describe('Integrity Block Signer', () => {
     expect(signatureStack.length).toEqual(keyPairs.length);
     signatureStack.map((entry) => expect(entry.length).toEqual(2));
 
-    const ibWithoutSignatures = new wbnSign.IntegrityBlock(/*is_v2=*/ true);
+    const ibWithoutSignatures = new wbnSign.IntegrityBlock();
     ibWithoutSignatures.setWebBundleId(webBundleId);
 
     for (let i = 0; i < keyPairs.length; i++) {