#977674 Corrupt changes file when built with --source-only-changes

Package:
sbuild
Source:
sbuild
Submitter:
Mathias Behrle
Date:
2023-09-28 16:06:03 UTC
Severity:
important
Tags:
Blocked By:
Bug Title
981021

  1

devscripts: debsign fails to sign dsc if buildinfo was already signed

normal stable testing unstable over 5 years ago

#977674#5
Date:
2020-12-18 15:39:44 UTC
From:
To:
Dear Maintainer,

I am running a builder for the Tryton packages that is configured to provide
a changes as well as a source.changes file. The first used for the
upload to the reprepro mirror, the latter used to upload to Debian.

The results of the last builder run caused mismatches of checksums when
importing to reprepro. Indeed the control of the checksums prooved to be
wrong. I suspect the following lines in the build logs to be the
culprit:

"""
Signature with key 'mathiasb@m9s.biz' requested:
 signfile dsc /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1.dsc mathiasb@m9s.biz

 fixup_buildinfo /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1.dsc /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_amd64.buildinfo
 signfile buildinfo /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_amd64.buildinfo mathiasb@m9s.biz

 fixup_changes dsc /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1.dsc /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_amd64.changes
 fixup_changes buildinfo /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_amd64.buildinfo /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_amd64.changes
 signfile changes /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_amd64.changes mathiasb@m9s.biz

Successfully signed dsc, buildinfo, changes files
---> from here comes the problematic part from --source-only-changes

 unsignfile /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_amd64.buildinfo
 unsignfile /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1.dsc
 signfile dsc /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1.dsc mathiasb@m9s.biz

 fixup_buildinfo /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1.dsc /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_amd64.buildinfo
 signfile buildinfo /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_amd64.buildinfo mathiasb@m9s.biz

 fixup_changes dsc /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1.dsc /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_source.changes
 fixup_changes buildinfo /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_amd64.buildinfo /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_source.changes
 signfile changes /home/mathiasb/bin/tryton/debian_builder/results/sid/tryton-server_5.0.30-1_source.changes mathiasb@m9s.biz

Successfully signed dsc, buildinfo, changes files
"""


Indeed running without --sourice-only-changes produced correct changes files.

The options used for the sbuild run are
['sbuild', '--quiet', '--chroot=sid-amd64-sbuild', '--keyid=mathiasb@m9s.biz', '--source', '--mailfrom="Debian Autobuilder <sbuild@m9s.biz>"', '--mail-log-to=mathiasb@m9s.biz', '--sbuild-mode=user', '--no-apt-update', '--no-apt-upgrade', '--no-apt-distupgrade', '--lintian-opts=-i -v -I -E --pedantic', '--bd-uninstallable-explainer=apt', '--source-only-changes', '/home/mathiasb/bin/tryton/debian_builder/build/tryton-server']

I am attaching for reference two changes files, one with --source-only-changes, one
without.

#977674#8
Date:
2021-01-24 21:53:56 UTC
From:
To:
Hi,

sorry for the late reply.

Quoting Mathias Behrle (2020-12-18 16:39:44)

I'm not surprised that this bug exists. The codepath you are using doesn't get
any testing. The official buildds are not using source-only-changes and those
who do, do not sign the build result with sbuild.

I'm tagging this with "help" and "newcomer". The fix is is likely to modify the
function close_build_log() in lib/Sbuild/Build.pm. There is a part that calls
debsign differently if SOURCE_ONLY_CHANGES is active and that part is probably
broken somehow. A merge request against https://salsa.debian.org/debian/sbuild
is greatly appreciated.

Since you are using sbuild in a buildd context I think you can work around this
bug for now by either manually signing or manually mangling your changes files?

Thanks!

cheers, josch

#977674#15
Date:
2021-01-25 07:55:34 UTC
From:
To:
* Johannes Schauer Marin Rodrigues: " Re: Bug#977674: Corrupt changes file when
  built with --source-only-changes" (Sun, 24 Jan 2021 22:53:56 +0100):

Hi Josh,

Thanks for your analysis. As I am not familiar with perl at all I am not able to
do this merge request.

Probably I shall try this next time resp. I will try  with an old version of
sbuild.

IIRC this bug is a regression introduced (probably between August and September
2020). Before the signing process used to work for changes as well as for
source-only-changes. From my unexperienced view the following commit comes to my
attention
https://salsa.debian.org/debian/sbuild/-/commit/bafc5362e0bbf29c5604fcaa73454e0fd6d0e3fc
that could have caused the impact.

Cheers
Mathias

#977674#18
Date:
2021-01-25 14:25:45 UTC
From:
To:
Hi,

Quoting Mathias Behrle (2021-01-25 08:55:34)

that this is maybe a regression piqued my interest. I tried to reproduce the
problem you are seeing by running sbuild with --keyid, --source-only-changes
and --source. This is probably the combination that causes your problem.
Unfortunately, I was not able to reproduce your problem.

How did you verify that the checksums indeed are wrong? Did you try running
dscverify on the amd64 and source changes files?

I tried with a test package and it all checks out:

tmp/test-pkg_1.0_source.changes:
   validating test-pkg_1.0.dsc
   validating test-pkg_1.0.tar.xz
   validating test-pkg_1.0_amd64.buildinfo
All files validated successfully.

tmp/test-pkg_1.0_amd64.changes:
   validating test-pkg_1.0.dsc
   validating test-pkg_1.0.tar.xz
   validating test-pkg_1.0_all.deb
   validating test-pkg_1.0_amd64.buildinfo
All files validated successfully.

Also, maybe unrelated, why are you running sbuild with --source? Did you read
the part in the sbuild man page about the --source option?

Thanks!

cheers, josch

#977674#25
Date:
2021-01-25 16:45:29 UTC
From:
To:
Hi,

I got a bit further on this bug.

The problem is, that when you combine --source-only-changes with --keyid, then
debsign will be run twice (once for the normal changes file and once for the
source-only changes file) and both times with --re-sign.  This means, that the
second invocation will possibly also change the signature of files that were
already processed by the first invocation and this means that the checksum of
the first changes file doesn't match anymore.

To fix the problem, one might suggest to just run the second invocation of
debsign with --no-re-sign so that everything that is already signed does not
get changed and only those things that don't have a signature get signed.

But this triggers a bug in debsign where the dsc will not even be considered
for signing if the buildinfo was already signed. Consider this code from
debsign:

maybesign_buildinfo() {
[...]
    if check_already_signed "$buildinfo" "buildinfo"; then
       echo "Leaving current signature unchanged." >&2
       return
    fi

    if [ -n "$dsc" ]; then
	maybesign_dsc "$signas" "$remotehost" "$dsc"
	withtempfile buildinfo "$buildinfo" fixup_buildinfo "$dsc"
    fi
[...]

As you can see, the function will return immediately without checking the dsc
if the buildinfo is already signed.

This code was introduced in devscripts back in 2017, so you can see that I was
correct when I said that the sbuild codepath of combining --source-only-changes
with --keyid is indeed seldom used.

I reported this as devscripts bug #981021 but I suggest that you have a look
into it or I fear that the chances of $somebody doing the work for us are slim.

Thanks!

cheers, josch

#977674#30
Date:
2021-01-27 10:57:04 UTC
From:
To:
* Johannes Schauer Marin Rodrigues: " Re: Bug#977674: Corrupt changes file when
  built with --source-only-changes" (Mon, 25 Jan 2021 17:45:29 +0100):

Hi Josh,

Big thanks for taking a look.

That sounds exactly like the problem I have.

Yes, the real use case appeared with mandatory source only uploads.

On one side I need the source only changes for uploads to ftp.debian.org, on
the other side I need binaries with according changes files suitable for testing
and upload to the backports reprepro.

Will have a look ASAP.

Thanks to you!

Cheers
Mathias

#977674#33
Date:
2021-01-27 11:03:02 UTC
From:
To:
Hi,

Quoting Mathias Behrle (2021-01-27 11:57:04)

I think what most sbuild users do, is to run sbuild without --keyid and then
let dput do the signing.

Myself, I'm using "dgit push-source" to do my source-only uploads. I never run
sbuild with --source-only-changes because dgit will take care of doing all of
that for me.

Clearly, this bug has to be fixed. I just wanted to suggest some workaround for
you and explanations why this was only found so late.

Thanks!

cheers, josch

#977674#38
Date:
2023-09-28 15:41:38 UTC
From:
To:
Since the buildinfo file of an upload contains the checksums of the dsc, this
behaviour makes sense, as signing the dsc would break the buildinfo.  On the
other hand for the same reason, if the buildinfo is signed the dsc should
already be signed as well.  In the sbuild --source --source-only-changes case
it most certainly will be, because the first debsign invocation signed it.
What use case would using --no-re-sign for the second call break?