Re: Sourceware mitigating and preventing the next xz-backdoor

Alejandro Colomar via Gdb
2024-04-21T20:53:03Z

[-- Attachment #1: Type: text/plain, Size: 6759 bytes --]

On Sun, Apr 21, 2024 at 10:40:14PM +0200, Alejandro Colomar wrote:
> On Sun, Apr 21, 2024 at 05:30:52PM +0200, Mark Wielaard wrote:
> > Hi Alejandro,
>
> Hi Mark,
>
> I've reordered your message, to organize my response.
>
> > On Wed, Apr 10, 2024 at 06:30:42PM +0200, Alejandro Colomar wrote:
> > > It would also be interesting to require showing range-diffs between
> > > patch revisions. They make it much more difficult to introduce a
> > > vulnerability after a reviewer has turned its mins into approving the
> > > patch. Of course, the patch could go in if the submitter lies in the
> > > range-diff and the vuln is undetected, but then it can be verified a
> > > posteriory to prove that there was a lie.
> >
> > Could you give an example of using git range-diff?
>
> Sure!
>
> > Normally when asked for changes to a
> > patch (series) I do an git rebase -i (on the local branch I used to
> > develop the feature/bug fix) and split/commit all requested changes
> > and then sent the new patches with git send-email again.
>
> I do the same thing.
>
> > But I guess
> > to use/combine that with git range-diffs I should start creating new
> > local branches for each patch (series) in development?
>
> No; it's compatible with that workflow.
>
> > How do you go from
> > v1 of a patch (series) to a v2?
>
> I'll start first with how you go from nothing to v1, which will help
> explain how you go from v1 to v2.
>
> Let's say I have a branch 'unifont' for adding the Unifont font to the
> Linux man-pages' PDF book. I have it in a branch that applies on top of
> 'master'.
>
> $ git log --oneline --graph master..unifont;
> * 892a12470 (HEAD -> unifont, alx/contrib, contrib) share/mk/: build-fonts-unifont: Specify spacewidth in afmtodit(1)
> * d80376b08 share/mk/: build-pdf-book: Use Unifont
> * 7ec952012 share/mk/: build-fonts-unifont: Build UnifontR from unifont.otf
>
> I want to send that as a patch set (v1, of course, since it's the first
> send). I would already use range-diff when generating the patches:
>
> $ git format-patch -o ./patches master..HEAD \
> --range-diff=master -v1 --cover-letter;
> ./patches/v1-0000-cover-letter.patch
> ./patches/v1-0001-share-mk-build-fonts-unifont-Build-UnifontR-from-.patch
> ./patches/v1-0002-share-mk-build-pdf-book-Use-Unifont.patch
> ./patches/v1-0003-share-mk-build-fonts-unifont-Specify-spacewidth-i.patch
>
> Why would you do that in v1? It notes the commit IDs of the patches, so
> you can later check them when preparing v2; we'll come back to that.
> For now, see what this adds to the patch set:
>
> $ tail ./patches/v1-0000-cover-letter.patch ;
> create mode 100644 share/mk/build/fonts/unifont/pfa.mk
> create mode 100644 share/mk/configure/build-depends/fonts-unifont/unifont.otf.mk
>
> Range-diff against v0:
> -: --------- > 1: 7ec952012 share/mk/: build-fonts-unifont: Build UnifontR from unifont.otf
> -: --------- > 2: d80376b08 share/mk/: build-pdf-book: Use Unifont
> -: --------- > 3: 892a12470 share/mk/: build-fonts-unifont: Specify spacewidth in afmtodit(1)
> --
> 2.43.0
>
> You can see the IDs of the commits that form this patch set, which match
> the git-log(1) that I showed above. Now someone suggests a change. For
> this example, I wasn't happy with a commit message, and added a space to
> the third commit message subject line. The git-log(1) is now:
>
> $ git log --oneline --graph master..unifont
> * bc7fa7d92 (HEAD -> unifont) share/mk/: build-fonts-unifont: Specify space width in afmtodit(1)
> * d80376b08 share/mk/: build-pdf-book: Use Unifont
> * 7ec952012 share/mk/: build-fonts-unifont: Build UnifontR from unifont.otf
>
> Let's generate a v2 patch set, showing the range-diff against v1. We
> need to check the commit IDs of the first set, which can be found in the
> mailing list archives, thanks to the trick we used. The v1 range was
> 7ec952012^..892a12470. So we just pass that range:
>
> $ git format-patch -o ./patches/ master..HEAD \
> --range-diff=7ec952012^..892a12470 -v2 --cover-letter;
> ./patches/v2-0000-cover-letter.patch
> ./patches/v2-0001-share-mk-build-fonts-unifont-Build-UnifontR-from-.patch
> ./patches/v2-0002-share-mk-build-pdf-book-Use-Unifont.patch
> ./patches/v2-0003-share-mk-build-fonts-unifont-Specify-space-width-.patch
>
> The v2 cover letter shows the changes introduced since v1:
>
> $ tail -n20 ./patches/v2-0000-cover-letter.patch
> create mode 100644 share/mk/build/fonts/unifont/dit.mk
> create mode 100644 share/mk/build/fonts/unifont/pfa.mk
> create mode 100644 share/mk/configure/build-depends/fonts-unifont/unifont.otf.mk
>
> Range-diff against v1:
> 1: 7ec952012 = 1: 7ec952012 share/mk/: build-fonts-unifont: Build UnifontR from unifont.otf
> 2: d80376b08 = 2: d80376b08 share/mk/: build-pdf-book: Use Unifont
> 3: 892a12470 ! 3: bc7fa7d92 share/mk/: build-fonts-unifont: Specify spacewidth in afmtodit(1)
> @@ Metadata
> Author: Alejandro Colomar <alx@kernel.org>
>
> ## Commit message ##
> - share/mk/: build-fonts-unifont: Specify spacewidth in afmtodit(1)
> + share/mk/: build-fonts-unifont: Specify space width in afmtodit(1)
>
> Link: <https://lore.kernel.org/linux-man/ZiQ_mTQHPq3ig723@debian/T/#t>
> Suggested-by: "G. Branden Robinson" <branden@debian.org>
> --
> 2.43.0
>
> If too much time passes between the last time the commit was rebased and
> when you create the range diff, you might need to use tags, such as
> 'unifont-v1', to avoid git(1) collecting garbagge and removing those
> old commits. But if you send shortly after rebasing, you won't need
> that.
>
> Lately, I haven't sent many patches to mailing lists, except for trivial
> oneliners, so I don't remember an example where I used this, but I can
> point to several github PRs where I've used this to document all changes
> that have happened to my patch sets, even when the maintainer has edited
> them, to be fully transparent. For example:
> <https://github.com/neomutt/neomutt/pull/4221#issuecomment-2061341972>
> <https://github.com/neomutt/neomutt/pull/4221#issuecomment-2052533059>

And here's an example where the maintainer pushed a version different
from my last revision, because he rebased after applying other changes
to his branch, and I showed a last range diff to prove that he didn't
do anything weird with my patches:

<https://github.com/neomutt/neomutt/pull/4247#issuecomment-2061145881>

>
> Have a lovely day!
> Alex
>
> --
> <https://www.alejandro-colomar.es/>

--
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]