[PULL][PATCH 0/1] Do setuid before git operations

[PULL][PATCH 0/1] Do setuid before git operations

[Jack (Szu-Chieh) Wang]

From: "Jack (Szu-Chieh) Wang" <jackwa@google.com>

/etc/build information missing is because the new git thinks it's
unsafe to do git operations if the directory has a different uid from
the executing process'. However, there are still cases that we are
building the image with a different uid, making git reporting false
alarms. This patch identifies the cases by attempting to setuid to the
owner of the git directory and if this works, it should be the
expected git repo thus safe to do git operations.

The following changes since commit 7b48f329aedc7fcb277302ba6ff167f22e5b1f22:

  scripts/autobuilder-worker-prereq-tests: add additional limit testing (2022-05-11 19:59:44 +0100)

are available in the Git repository at:

  git://git.yoctoproject.org/poky-contrib jackwang/setuid-before-git-op
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=jackwang/setuid-before-git-op

Jack Wang (1):
  Do setuid before git operations

 meta/classes/metadata_scm.bbclass | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.36.0.550.gb090851708-goog

[PATCH 1/1] Do setuid before git operations

[Jack (Szu-Chieh) Wang]

After 2.35.2, git operations aren't considered safe when running in
directories owned by other users.
This patch attempts to setuid to the uid of the directory owner before
doing the git operations.

Signed-off-by: Jack (Szu-Chieh) Wang <jackwa@google.com>
---
 meta/classes/metadata_scm.bbclass | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/meta/classes/metadata_scm.bbclass b/meta/classes/metadata_scm.bbclass
index 47cb969b8d..bb2f494533 100644
--- a/meta/classes/metadata_scm.bbclass
+++ b/meta/classes/metadata_scm.bbclass
@@ -22,18 +22,22 @@ def base_get_metadata_svn_revision(path, d):
 
 def base_get_metadata_git_branch(path, d):
     import bb.process
+    import os
+    stat_info = os.stat(path)
 
     try:
-        rev, _ = bb.process.run('git rev-parse --abbrev-ref HEAD', cwd=path)
+        rev, _ = bb.process.run('git rev-parse --abbrev-ref HEAD', cwd=path, preexec_fn=lambda : os.setuid(stat_info.st_uid))
     except bb.process.ExecutionError:
         rev = '<unknown>'
     return rev.strip()
 
 def base_get_metadata_git_revision(path, d):
     import bb.process
+    import os
+    stat_info = os.stat(path)
 
     try:
-        rev, _ = bb.process.run('git rev-parse HEAD', cwd=path)
+        rev, _ = bb.process.run('git rev-parse HEAD', cwd=path, preexec_fn=lambda : os.setuid(stat_info.st_uid))
     except bb.process.ExecutionError:
         rev = '<unknown>'
     return rev.strip()
-- 
2.36.0.550.gb090851708-goog

Re: [poky] [PULL][PATCH 0/1] Do setuid before git operations

[Richard Purdie]

On Fri, 2022-05-13 at 10:03 +0000, Jack (Szu-Chieh) Wang via
lists.yoctoproject.org wrote:
> From: "Jack (Szu-Chieh) Wang" <jackwa@google.com>
> 
> /etc/build information missing is because the new git thinks it's
> unsafe to do git operations if the directory has a different uid from
> the executing process'. However, there are still cases that we are
> building the image with a different uid, making git reporting false
> alarms. This patch identifies the cases by attempting to setuid to the
> owner of the git directory and if this works, it should be the
> expected git repo thus safe to do git operations.

This should have been fixed in master or in the process of being fixed
on the stable branches by the git wrapper we've recently merged?

https://git.yoctoproject.org/poky/commit/?id=4d7383aefb391a5a998454c70feb96127951ca0a

and several other related commits.

Which release are you seeing that with?

Cheers,

Richard

Re: [poky] [PULL][PATCH 0/1] Do setuid before git operations

[Jack (Szu-Chieh) Wang]

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

Hi Richard,

Thanks for your prompt reply.
We're using the dunfell branch and the git issues remain.
I see the patch is only on master not on dunfell, is this correct?

Sincerely,
Jack


On Fri, May 13, 2022 at 6:14 PM <richard.purdie@linuxfoundation.org> wrote:

> On Fri, 2022-05-13 at 10:03 +0000, Jack (Szu-Chieh) Wang via
> lists.yoctoproject.org wrote:
> > From: "Jack (Szu-Chieh) Wang" <jackwa@google.com>
> >
> > /etc/build information missing is because the new git thinks it's
> > unsafe to do git operations if the directory has a different uid from
> > the executing process'. However, there are still cases that we are
> > building the image with a different uid, making git reporting false
> > alarms. This patch identifies the cases by attempting to setuid to the
> > owner of the git directory and if this works, it should be the
> > expected git repo thus safe to do git operations.
>
> This should have been fixed in master or in the process of being fixed
> on the stable branches by the git wrapper we've recently merged?
>
>
> https://git.yoctoproject.org/poky/commit/?id=4d7383aefb391a5a998454c70feb96127951ca0a
>
> and several other related commits.
>
> Which release are you seeing that with?
>
> Cheers,
>
> Richard
>

[-- Attachment #2: Type: text/html, Size: 2132 bytes --]

Re: [poky] [PULL][PATCH 0/1] Do setuid before git operations

[Richard Purdie]

On Fri, 2022-05-13 at 18:19 +0800, Jack (Szu-Chieh) Wang wrote:
> Hi Richard,
> 
> Thanks for your prompt reply.
> We're using the dunfell branch and the git issues remain.
> I see the patch is only on master not on dunfell, is this correct?

We have a backport policy which means we fix on master, then fixes
cascade to the stable maintained releases. There is a review request
out here:

http://cgit.openembedded.org/openembedded-core-contrib/log/?h=stable/dunfell-next

which includes the patches and should hopefully merge in the next few
days.

Cheers,

Richard

Re: [poky] [PULL][PATCH 0/1] Do setuid before git operations

[Jack (Szu-Chieh) Wang]

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

I see, thanks a lot for your help and your efforts.

Sincerely,
Jack


On Fri, May 13, 2022 at 6:22 PM <richard.purdie@linuxfoundation.org> wrote:

> On Fri, 2022-05-13 at 18:19 +0800, Jack (Szu-Chieh) Wang wrote:
> > Hi Richard,
> >
> > Thanks for your prompt reply.
> > We're using the dunfell branch and the git issues remain.
> > I see the patch is only on master not on dunfell, is this correct?
>
> We have a backport policy which means we fix on master, then fixes
> cascade to the stable maintained releases. There is a review request
> out here:
>
>
> http://cgit.openembedded.org/openembedded-core-contrib/log/?h=stable/dunfell-next
>
> which includes the patches and should hopefully merge in the next few
> days.
>
> Cheers,
>
> Richard
>

[-- Attachment #2: Type: text/html, Size: 1400 bytes --]