All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Cc: Valentino Paulon <valentino.paulon88@gmail.com>,
	qemu-devel@nongnu.org, yvugenfi@redhat.com, michael.roth@amd.com
Subject: Re: [PATCH] qga: Do not follow symlink in guest-ssh-* commands
Date: Mon, 10 Aug 2026 09:55:17 +0100	[thread overview]
Message-ID: <anmR9aKJcdtL_xTL@redhat.com> (raw)
In-Reply-To: <CAPMcbCoKduRuKdt8P6ULrc43uX_2z=hBp_N87yi8F4GHdVqPeA@mail.gmail.com>

On Mon, Aug 10, 2026 at 11:37:34AM +0300, Kostiantyn Kostiuk wrote:
> On Mon, Aug 10, 2026 at 11:20 AM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> 
> > On Mon, Aug 10, 2026 at 11:06:27AM +0300, Kostiantyn Kostiuk wrote:
> > > Hi Valentino,
> > >
> > > Sorry for the long delay.
> > > I have a question for you
> > >
> > > On Tue, Jul 14, 2026 at 7:10 PM Valentino Paulon <
> > > valentino.paulon88@gmail.com> wrote:
> > >
> > > > > > So, you propose to call seteuid/setegid before any I/O operation?
> > > > >
> > > > > Yes, specifically for the SSH commands, because they're unusual in
> > > > > that we're doing stuff on behalf of an unprivileged user.
> > > >
> > > > +1 on the euid/egid direction -- as the reporter that's the outcome I
> > > > was hoping for. It confines every step (read, mkdir, create, rename,
> > > > chown) to what the target user could already do himself, which removes
> > > > the whole TOCTOU class instead of chasing one instance of it.
> > > >
> > > > A few implementation details that tend to bite with this pattern, in
> > > > case they save a round-trip:
> > > >
> > > > - drop the supplementary groups too, not just egid: initgroups() (or
> > > > setgroups() with the user's list) while still root, otherwise the
> > > > agent's effective access won't match the user's in either direction
> > > >
> > >
> > > Technically, there is no limitation to running QGA in unprivileged mode.
> > > Normally, it is a system daemon, but this is not mandatory.
> > > In this case, we can not call initgroups because the caller must
> > > have CAP_SETGID.
> > > We can call initgroups only when QGA runs as root. What do you think?
> >
> > In any practical sense it is a system daemon given the set of commands
> > it is exposing. Thus, IMHO, deploying as an unprivileged user should
> > be considered a broken deployment. We could make that explicit by
> > refusing to launch.
> >
> 
> Makes sense, but we have one more issue with QGA unit tests in this case.
> They are called by an unprivileged user (build user), and initgroups broke
> it.
> Is it possible to enter the namespace automatically by the test engine?

We can't assume that is possible todo in all build/test environments.

Doing things as different user accounts feels pretty dubious for the
scope of a unit test too - that is really functional or integration
testing.

Can we somehow restructure the tests and/or agent code to avoid the
changing groups stuff in unit tests.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|



  reply	other threads:[~2026-08-10  8:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09 10:57 [PATCH] qga: Do not follow symlink in guest-ssh-* commands Kostiantyn Kostiuk
2026-07-14 10:37 ` Daniel P. Berrangé
2026-07-14 11:28   ` Kostiantyn Kostiuk
2026-07-14 11:30     ` Daniel P. Berrangé
2026-07-14 16:10       ` Valentino Paulon
2026-08-10  8:06         ` Kostiantyn Kostiuk
2026-08-10  8:20           ` Daniel P. Berrangé
2026-08-10  8:37             ` Kostiantyn Kostiuk
2026-08-10  8:55               ` Daniel P. Berrangé [this message]
2026-08-10  8:58                 ` Kostiantyn Kostiuk
2026-08-10  9:28                   ` Daniel P. Berrangé
2026-08-10  9:33                     ` Kostiantyn Kostiuk
2026-08-10  9:58                       ` Daniel P. Berrangé
2026-08-10 10:48                         ` Kostiantyn Kostiuk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=anmR9aKJcdtL_xTL@redhat.com \
    --to=berrange@redhat.com \
    --cc=kkostiuk@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=qemu-devel@nongnu.org \
    --cc=valentino.paulon88@gmail.com \
    --cc=yvugenfi@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.