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

Add SecureDrop-specific metadata to buildinfo files #434

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Mar 28, 2023

dpkg will only export specific known environment variables into the
buildinfo file[1], but we want to track at least two more things:

  1. the Git commit of the package being built
  2. the Git commit of the securedrop-builder repository being used, as
    the bootstrap and wheels used affect the package output.

We can track those by capturing the values before the build process and
then manually adding them to the end of the buildinfo file. This also
means that builds must be done from a Git checkout, and cannot be from a
tarball, so that path now errors out.

While we're at it, look up the package filename by "parsing"
debian/files instead of trying to find it via find.

[1] https://git.dpkg.org/cgit/dpkg/dpkg.git/tree/scripts/Dpkg/BuildInfo.pm

I am also renaming PKG_GITREF to SD_PKG_GITREF, as this will be exported in the .buildinfo file so let's prefix it with "SD" to make it abundantly clear this is our environment variable and reduce the risk of collision.

There don't appear to be any other users of this besides humans, so I have not added in any backwards-compatibility support for the old name.

Test plan

  • Check out this PR. Run SD_PKG_GITREF=main make securedrop-proxy, verify the buildinfo file contains SD_BUILDER_GITREF and SD_PKG_GITREF with commits that correspond to this builder PR you just checked out and the current main branch of sd-proxy.
  • Add a dummy commit to your sd-builder checkout. Re-run SD_PKG_GITREF=main make securedrop-proxy, verify SD_BUILDER_GITREF changed to correspond to the new commit you just added.
  • Run git -C /tmp/securedrop-proxy checkout HEAD~1, note which commit you're now on. Run PKG_PATH=/tmp/securedrop-proxy make securedrop-proxy, verify SD_PKG_GITREF changed to the new commit you switched to.
  • Run PKG_VERSION=1 make securedrop-keyring, verify the buildinfo file contains SD_BUILDER_GITREF but not SD_PKG_GITREF.

@legoktm
Copy link
Member Author

legoktm commented Mar 28, 2023

One open question that I kind of discussed in #433 (comment) is at what layer should SD_BUILDER_GITREF be handled. It seems absurdly complex to try and have the build-debianpackage script re-clone the builder repo if it's the wrong version, my current thinking is we do it one layer higher and have the rebuild script take care of it.

But then should the build-debianpackage script error out if SD_BUILDER_GITREF is set and we're using the wrong builder? I think that would be a reasonable fail-safe.

@legoktm
Copy link
Member Author

legoktm commented Mar 31, 2023

This is basically ready for review, but lets land #427 first and then I'll rebase this.

This will be exported in the `.buildinfo` file so let's prefix it with
"SD" to make it abundantly clear this is our environment variable and
reduce the risk of collision.

There don't appear to be any other users of this besides humans, so I
have not added in any backwards-compatibility support for the old name.
dpkg will only export specific known environment variables into the
buildinfo file[1], but we want to track at least two more things:

 1) the Git commit of the package being built
 2) the Git commit of the securedrop-builder repository being used, as
    the bootstrap and wheels used affect the package output.

We can track those by capturing the values before the build process and
then manually adding them to the end of the buildinfo file. This also
means that builds must be done from a Git checkout, and cannot be from a
tarball; so that path now errors out.

It would be too complicated to have this script respect the new
$SD_BUILDER_GITREF by checking out a different version of the
repository, re-initializing the bootstrap, etc., so we leave that as a
responsiblity of the caller. But as a failsafe we still double-check
that the desired version of the builder repository is being used.

While we're at it, look up the package filename by "parsing"
debian/files instead of trying to find it via find.

[1]
https://git.dpkg.org/cgit/dpkg/dpkg.git/tree/scripts/Dpkg/BuildInfo.pm
@legoktm legoktm marked this pull request as ready for review April 4, 2023 18:29
@zenmonkeykstop zenmonkeykstop self-assigned this Apr 5, 2023
@cfm
Copy link
Member

cfm commented May 11, 2023

@legoktm and I discussed this in the DX working group just now. @zenmonkeykstop, I can pick this up, unless you're attached to it.

@cfm cfm self-assigned this May 11, 2023
@cfm cfm removed their assignment Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants