Problem/Motivation

Current implementation assumes extraction of zip archive before verification.

Extraction of an unvalidated zip-file might be a problem of itself (think zipbomb, invalid filenames).

Proposed resolution

Instead of using a single file (which has benefits from not needing to download as many files), use two. The csig file validates the zip file.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

heddn created an issue. See original summary.

drumm’s picture

Drupal.org’s in-place-updates being served no longer have the checksumlist.csig file. The same path as the .zip files, like https://www.drupal.org/in-place-updates/drupal/drupal-8.7.4-to-8.7.5.zip, with .csig appended, now serves the signed hash of the zip file. For example, https://www.drupal.org/in-place-updates/drupal/drupal-8.7.4-to-8.7.5.zip.csig.

drumm’s picture

FileSize
933.51 KB

To reduce HTTP requests, maybe using the zip archive comment to store the signing is possible. Attached is an example.

$ zipnote drupal-8.7.6-to-8.7.9.zip | grep -v '^@'
untrusted comment: verify with root.pub
RWTtDh2Yu5YZWeipXt0S7xIDidhiNXahsc1pLtRzC/TOh9Zg/GpAN9ZOr8zVchh1GbJL9PYl5dqfmF+ZgSvZ3BbLdDTa/ihmdgc=
2020-01-01
untrusted comment: TESTING build keypair - not for production use public key
RWRi8iT14BMp6VPfITGCJ9bpPB4IsIJHRmCdsSpolFZTwVpljz0UUuWa
untrusted comment: verify with sites/all/modules/drupalorg/drupalorg/temporary-keys/build.pub
RWRi8iT14BMp6ZnWax9iYynhxMKqFe4/kRwl29pjZp/1EbHREhifVs/qXU+mX5gMUgmuhlr1bHnW3EC4Y7hJij2U+kzxsqv5Ww4=
SHA256 (drupal-8.7.6-to-8.7.9.zip) = eb14f40fbe40ff22291e73a6a9d8593041252d8fa9b8e9e94af691190ca5ac6d

If PHP code can safely extract that note, strip the note from the file, and get the same eb14f40fb… shasum before extraction, this will work.

drumm’s picture

FileSize
933.72 KB

And a sample with a base64-encoded archive comment.

$ zipnote drupal-8.7.6-to-8.7.9.zip | grep -v '^@' | base64 -d
untrusted comment: verify with root.pub
RWTtDh2Yu5YZWeipXt0S7xIDidhiNXahsc1pLtRzC/TOh9Zg/GpAN9ZOr8zVchh1GbJL9PYl5dqfmF+ZgSvZ3BbLdDTa/ihmdgc=
2020-01-01
untrusted comment: TESTING build keypair - not for production use public key
RWRi8iT14BMp6VPfITGCJ9bpPB4IsIJHRmCdsSpolFZTwVpljz0UUuWa
untrusted comment: verify with sites/all/modules/drupalorg/drupalorg/temporary-keys/build.pub
RWRi8iT14BMp6ZnWax9iYynhxMKqFe4/kRwl29pjZp/1EbHREhifVs/qXU+mX5gMUgmuhlr1bHnW3EC4Y7hJij2U+kzxsqv5Ww4=
SHA256 (drupal-8.7.6-to-8.7.9.zip) = eb14f40fbe40ff22291e73a6a9d8593041252d8fa9b8e9e94af691190ca5ac6d
heddn’s picture

Status: Active » Needs review
FileSize
10.51 KB

We ran into issues adding the CSIG as a comment to the zip file. PHP vs posix ZIP doesn't save the file in the same format. And we have to strip the CSIG from the zip comment before hashing it. Otherwise it doesn't pass validation. But stripping it via PHP seems to rewrite the entire zip file in a slightly different format. Since the files are binary, it is hard to say exactly what is different. So we're back to downloading 2 distinct artifacts. One for the zip file and for the csig validation of the zip archive.

heddn’s picture

FileSize
1.32 KB
10.63 KB

heddn credited mbaynton.

heddn credited pwolanin.

heddn’s picture

Adding credit from slack discussions.

  • heddn committed d84e5fb on 8.x-1.x
    Issue #3093782 by heddn, drumm, David Strauss, mbaynton, pwolanin: Use...
heddn’s picture

Status: Needs review » Needs work

Landed this on 8.x and rolled an alpha4. Next up a 7.x backport and new alpha there too.

heddn’s picture

Status: Needs work » Needs review
FileSize
8.77 KB
heddn’s picture

Version: 8.x-1.x-dev » 7.x-1.x-dev
heddn’s picture

  • heddn committed 7d13c12 on 7.x-1.x
    Issue #3093782 by heddn, drumm, David Strauss, mbaynton, pwolanin: Use...
heddn’s picture

Status: Needs review » Fixed

heddn credited Heine.

heddn’s picture

7.x alpha3 is now also tagged.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.