Re: Sourceware mitigating and preventing the next xz-backdoor

[Alejandro Colomar <alx@kernel.org>]

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

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>

Have a lovely day!
Alex

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

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