Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider signing release source tarballs #3024

Closed
richvdh opened this issue Jan 22, 2017 · 20 comments
Closed

Consider signing release source tarballs #3024

richvdh opened this issue Jan 22, 2017 · 20 comments
Labels
A-Packaging Packaging, signing, releasing X-Release-Blocker

Comments

@richvdh
Copy link
Member

richvdh commented Jan 22, 2017

Packagers have asked that we pgp-sign source tarballs for our releases.

@richvdh
Copy link
Member Author

richvdh commented Jan 22, 2017

We already sign our git tags, and since the SHA of a git tag depends on its content, that carries the same information as a signed source tarball.

Verifying the content of the source tarball that github generates sounds like a significant extra step in our release process, and I am not enthusiastic about adding more steps to our already cumbersome release process - particularly given the information requested already appears to be available, and this appears to be working around deficiencies in downstream release tools/processes.

I'm open to being convinced, though...

@ArchangeGabriel
Copy link

@richvdh Unless I’m wrong, this means you’re asking to pull the whole git tree just for verifying one tag signature. This might be significantly larger in size than just the source release tarball.

Also, building a package from a git tag is not recommended generally, because then the tag could be changed upstream (so here in this case) without notice. Even if we do trust you in not mangling with this, we just prefer having this supplemental layer of security. ;)

@richvdh
Copy link
Member Author

richvdh commented Jan 22, 2017

@richvdh Unless I’m wrong, this means you’re asking to pull the whole git tree just for verifying one tag signature. This might be significantly larger in size than just the source release tarball.

You can get git to just clone the source for the tag in question, with --depth 1:

git clone --depth 1 https://github.com/vector-im/riot-web -b v0.9.6
git tag -v v0.9.6

Also, building a package from a git tag is not recommended generally, because then the tag could be changed upstream (so here in this case) without notice. Even if we do trust you in not mangling with this, we just prefer having this supplemental layer of security. ;)

I'm unclear why changing a tag (and resigning) is a bigger risk than changing the source tarball (and re-signing that)?

@ArchangeGabriel
Copy link

OK, wasn’t knowing about --depth, clear enhancement here. It still makes the build process depending on git however, something I would avoid ideally (for the sake of minimal dependencies), but even would that be fixed here riot build process still implies git anyway (at least for now, but eventually that’s also something I’d like to change — let’s keep it for another ticket).

I'm unclear why changing a tag (and resigning) is a bigger risk than changing the source tarball (and re-signing that)?

We hardcode checksums of source tarballs in our PKGBUILDs (see here), so would you change the tarball that would be detected by checksum not matching anymore. Unless you also try hard to get the same checksum, but then the sig file checksum would catch you.

I agree that git-pull-from-a-tag checksum is something that could once more be added in our packaging tool, but with git signatures verification not currently accepted (or even discussed by main devs AFAIK), I see this even less likely to land in ArchLinux’s makepkg. And I don’t know about other distros, but I doubt they are a lot with really better handling of this.

What I don’t understand is how much cumbersome this looks to you. It seems fairly simple to me, but maybe I’m missing something here.

@richvdh
Copy link
Member Author

richvdh commented Jan 22, 2017

We hardcode checksums of source tarballs in our PKGBUILDs (see here), so would you change the tarball that would be detected by checksum not matching anymore.

Hrm. You could equally well embed the sha of the git tag, so I guess this just comes down to tooling.

The only distro I know much about is Debian. As fair as I know they are happy to build from git tags; I think their approach is actually to maintain a fork of the upstream package.

What I don’t understand is how much cumbersome [this] looks to you. It seems fairly simple to me, but maybe I’m missing something here.

Actually I just saw the "alternative local workflow" at the bottom. That does indeed look simple.

@ArchangeGabriel
Copy link

Hrm. You could equally well embed the sha of the git tag, so I guess this just comes down to tooling.

Yes, that what my next paragraph was about. ;)

Actually I just saw the "alternative local workflow" at the bottom. That does indeed look simple.

Yes, and I would in fact think of it as the default one rather than alternative. Sorry not to have mentioned it explicitly, but glad you found out by ourself and seems to consider it as not too “cumbersome”. ;)

richvdh added a commit to matrix-org/matrix-js-sdk that referenced this issue Feb 4, 2017
When we are doing a signed release, also create a sig for the 'archive' tarball
which github creates for us.

Fixes element-hq/element-web#3024.
@richvdh
Copy link
Member Author

richvdh commented Feb 4, 2017

release 0.9.7 includes a signature for the source tarball, and I've opened a PR to include it in the release script for future releases, so this will be resolved.

However, on reflection, I'm not sure how far it is going to get you. The build process involves lots of other dependencies, which are downloaded by npm at build-time, and aren't signed by their respective authors.

@ArchangeGabriel
Copy link

@richvdh Thanks for this. :) I agree that the situation with npm is far from ideal, and I think it require fixing at npm level: after all, I expect from all package managers to handle signed packages, and npm, pip (dunno about the status of this one though) and the like should be no different from distro package managers in this regard. ;)

But having riot source release signed is still a gain, and having it already in place for the day npm will be fixed is a good thing. :)

I didn’t find your PR though, is the release script handled outside of this repo?

@richvdh
Copy link
Member Author

richvdh commented Feb 4, 2017

Yes, see matrix-org/matrix-js-sdk#351

@ArchangeGabriel
Copy link

Oh and just a thought, why did you add it -src in the name? Not a big concern, but makes it harder to download both file with a simple {,asc} at the end of the curl/wget line.

@richvdh
Copy link
Member Author

richvdh commented Feb 4, 2017

To make it obvious it is for the source, rather than the binary tarball.

@ArchangeGabriel
Copy link

OK, I kinda expected this answer, and I’ll live with that (since I’m not even sure the renaming part we use in our PKGBUILD would work with two files).

@ArchangeGabriel
Copy link

Nevermind, the paths are not even the same:

/archive/v${pkgver}.tar.gz
/releases/download/v${pkgver}/v${pkgver}-src.tar.gz.asc

So this require separate handling anyway. ;)

@richvdh richvdh closed this as completed Feb 6, 2017
@ArchangeGabriel
Copy link

@richvdh I don’t know how you’ve generated the one for release 0.9.7, especially what was different w.r.t. https://github.com/matrix-org/matrix-js-sdk/pull/351/files, but I’ve just figured out that every release since 0.9.8-rc.1 included has a mismatching sig for the release tarball (i.e. e.g. https://github.com/vector-im/riot-web/releases/download/v0.9.8-rc.1/v0.9.8-rc.1-src.tar.gz.asc does not verify https://github.com/vector-im/riot-web/archive/v0.9.8-rc.1.tar.gz).

I highly suspect --prefix="${project_name}-${release}/" to be the culprit, it should probably be --prefix="${tag}/" since that’s how GitHub generates it. Could you update your script, and fix the sig file for at least the current release (0.9.9)? Thanks!

@richvdh
Copy link
Member Author

richvdh commented May 14, 2017

@dbkr can we fix this for the next release?

@richvdh
Copy link
Member Author

richvdh commented May 18, 2017

It turns out that what is different is that @dbkr did the last couple of releases, on his Mac, and that his version of gzip generates slightly different things to the linux version, which apparently github use.

Releasing on linux isn't an option because building the electron artifacts is too hard there...

@ArchangeGabriel
Copy link

Hum… And I guess that generating the sigs for the source tarball (not the one with electron artifacts) on a different machine is too much burden? I guess so since in the current setup it’s just one more file generated by the release script, while here it would mean doing the release in two steps.

@t3chguy
Copy link
Member

t3chguy commented May 18, 2017

tested using GNU Gzip on a Mac (brew install gzip) and that matched the gzipped file to gzipped in Linux and in Windows (in WSL)

@richvdh
Copy link
Member Author

richvdh commented May 19, 2017

We believe this got fixed in matrix-org/matrix-js-sdk#438. Dave just released 0.9.10-rc.1 which has a good source signature.

@richvdh richvdh closed this as completed May 19, 2017
@ArchangeGabriel
Copy link

Nice, thank you. I especially like the way you where able to solve this with some sort of intermediate way between the two from the Debian wiki. Maybe that could be a nice addition to this one by the way, you might not be the only people with non GNU gzip or producing slightly different tarball. ;)

@jryans jryans added the A-Packaging Packaging, signing, releasing label Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Packaging Packaging, signing, releasing X-Release-Blocker
Projects
None yet
Development

No branches or pull requests

4 participants