[FFmpeg-devel] rebasing security

[Michael Niedermayer]

Hi Timo

On Sun, Aug 03, 2025 at 10:01:42PM +0200, Timo Rothenpieler wrote:
> On 8/3/2025 9:02 PM, Michael Niedermayer wrote:
> > Hi
> > 
> > On Sun, Aug 03, 2025 at 05:31:39PM +0200, Michael Niedermayer wrote:
> > [...]
> > > The solutions are obvious:
> > > 1. ignore security and supply chain attacks
> > > 2. use merges not rebases on the server
> > > 3. rebase locally, use fast forward only
> > > 4. verify on server rebases
> > 
> > Maybe not everyone understood the problem. So let me try a different
> > explanation. Without any signatures.
> > 
> > In the ML workflow: (for simplicity we assume reviewer and commiter is the same person)
> > 1. someone posts a patch
> > 2. patch is locally applied or rebased
> > 3. commit is reviewed
> > 4. commit is tested
> > 5. commit is pushed
> > 
> > Here the only way to get bad code in, is through the reviewer
> > If the reviewer doesnt miss anything and his setup is not compromised
> > then what he pushes is teh reviewed code
> > 
> > if its manipulated after its pushed git should light up like a christmess tree
> > on the next "git pull --rebase"
> > 
> > 
> > With the rebase on webapp (gitlab or forgejo) workflow
> > 1. someone posts a pull request
> > 2. pr is reviewed
> > 3. pr is approved
> > 4. pr is rebased
> > 5. pr is tested
> > 6, pr is pushed
> > 
> > now here of course the same reviewer trust or compromised scenarios exist
> > but we also have an extra one and that is the server
> > because the server strips the signatures during rebase it can modify the
> > commit. And this happens after review and because a rebase was litterally
> > requested by the reviewer its not likely to be noticed as something out of
> > place

> If you as a pusher of commits want to sign them with your own key, you have
> to do that manually.
> There is no sane way for Forgjo to do that for you.

yes


>
> I can configure Forgejo to sign commits it itself generates, that is an
> option.

is there a disadvantage ?


> See here for how it can do it on merges.
> https://forgejo.org/docs/latest/admin/advanced/signing/#pull-request-merges

confusing, so many options


> 
> I think if I set it to "commitssigned", it'll check all commits in the PR
> against the users configured GPG/SSH key, and if they are all valid, it'll
> then sign them with the instance key whenever it needs to modify them for an
> operation.
> "twofa" would also be an option, cause it indicates that the author of that
> commit has some reasonably strong proof that they are them themselves.

yeah, I have not thought deeply about it, they seem to want to indicate
something by signing commmits.

To me signing my commits primarly is a way to say the commit was not tampered
with after I signed it.

thx


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
than the original author, trying to rewrite it will not make it better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250803/b89caaf7/attachment.sig>