[PATCH 1/2] qga/win32: Remove change action from MSI installer

Konstantin Kostiuk posted 2 patches 2 years, 11 months ago
Maintainers: Konstantin Kostiuk <kkostiuk@redhat.com>, Michael Roth <michael.roth@amd.com>
There is a newer version of this series
[PATCH 1/2] qga/win32: Remove change action from MSI installer
Posted by Konstantin Kostiuk 2 years, 11 months ago
resolves: rhbz#2167436
fixes: CVE-2023-0664

Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
---
 qga/installer/qemu-ga.wxs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 51340f7ecc..feb629ec47 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -31,6 +31,7 @@
       />
     <Media Id="1" Cabinet="qemu_ga.$(var.QEMU_GA_VERSION).cab" EmbedCab="yes" />
     <Property Id="WHSLogo">1</Property>
+    <Property Id="ARPNOMODIFY" Value="yes" Secure="yes" />
     <MajorUpgrade
       DowngradeErrorMessage="Error: A newer version of QEMU guest agent is already installed."
       />
--
2.25.1
Re: [PATCH 1/2] qga/win32: Remove change action from MSI installer
Posted by Philippe Mathieu-Daudé 2 years, 11 months ago
On 20/2/23 18:41, Konstantin Kostiuk wrote:
> resolves: rhbz#2167436

"You are not authorized to access bug #2167436."

> fixes: CVE-2023-0664

This commit description is rather scarce...

I understand you are trying to fix a CVE, but we shouldn't play
the "security by obscurity" card. How can the community and
distributions know this security fix is enough with the bare
"Remove change action from MSI installer" justification?
Can't we do better?

> Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
> ---
>   qga/installer/qemu-ga.wxs | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> index 51340f7ecc..feb629ec47 100644
> --- a/qga/installer/qemu-ga.wxs
> +++ b/qga/installer/qemu-ga.wxs
> @@ -31,6 +31,7 @@
>         />
>       <Media Id="1" Cabinet="qemu_ga.$(var.QEMU_GA_VERSION).cab" EmbedCab="yes" />
>       <Property Id="WHSLogo">1</Property>
> +    <Property Id="ARPNOMODIFY" Value="yes" Secure="yes" />
>       <MajorUpgrade
>         DowngradeErrorMessage="Error: A newer version of QEMU guest agent is already installed."
>         />
> --
> 2.25.1
> 
>
Re: [PATCH 1/2] qga/win32: Remove change action from MSI installer
Posted by Daniel P. Berrangé 2 years, 11 months ago
On Tue, Feb 21, 2023 at 09:15:15AM +0100, Philippe Mathieu-Daudé wrote:
> On 20/2/23 18:41, Konstantin Kostiuk wrote:
> > resolves: rhbz#2167436
> 
> "You are not authorized to access bug #2167436."
> 
> > fixes: CVE-2023-0664
> 
> This commit description is rather scarce...
> 
> I understand you are trying to fix a CVE, but we shouldn't play
> the "security by obscurity" card. How can the community and
> distributions know this security fix is enough with the bare
> "Remove change action from MSI installer" justification?
> Can't we do better?

Yes, commit messages should always describe the problem being
solved directly. Bug trackers usually make people wade through
piles of irrelevant comments & potentially misleading blind
alleys during the back & forth of triage. The important info
needs to be distilled down and put in the commit message,
concisely describing the problem faced. Bug tracker links have
been known to bit-rot too.

The commit message needs to focus on /why/ the change was made,
much more than describing /what/ was changed.

> > Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
> > ---
> >   qga/installer/qemu-ga.wxs | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> > index 51340f7ecc..feb629ec47 100644
> > --- a/qga/installer/qemu-ga.wxs
> > +++ b/qga/installer/qemu-ga.wxs
> > @@ -31,6 +31,7 @@
> >         />
> >       <Media Id="1" Cabinet="qemu_ga.$(var.QEMU_GA_VERSION).cab" EmbedCab="yes" />
> >       <Property Id="WHSLogo">1</Property>
> > +    <Property Id="ARPNOMODIFY" Value="yes" Secure="yes" />
> >       <MajorUpgrade
> >         DowngradeErrorMessage="Error: A newer version of QEMU guest agent is already installed."
> >         />

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH 1/2] qga/win32: Remove change action from MSI installer
Posted by Konstantin Kostiuk 2 years, 11 months ago
On Tue, Feb 21, 2023 at 10:15 AM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 20/2/23 18:41, Konstantin Kostiuk wrote:
> > resolves: rhbz#2167436
>
> "You are not authorized to access bug #2167436."
>
> > fixes: CVE-2023-0664
>
> This commit description is rather scarce...
>
> I understand you are trying to fix a CVE, but we shouldn't play
> the "security by obscurity" card. How can the community and
> distributions know this security fix is enough with the bare
> "Remove change action from MSI installer" justification?
> Can't we do better?
>

This patch is part of the fix. I remove the 'change' button because
the installer has no components to choose from and the installer
always installs everything.

The second patch removes the interactive command shell.


>
> > Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
> > ---
> >   qga/installer/qemu-ga.wxs | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> > index 51340f7ecc..feb629ec47 100644
> > --- a/qga/installer/qemu-ga.wxs
> > +++ b/qga/installer/qemu-ga.wxs
> > @@ -31,6 +31,7 @@
> >         />
> >       <Media Id="1" Cabinet="qemu_ga.$(var.QEMU_GA_VERSION).cab"
> EmbedCab="yes" />
> >       <Property Id="WHSLogo">1</Property>
> > +    <Property Id="ARPNOMODIFY" Value="yes" Secure="yes" />
> >       <MajorUpgrade
> >         DowngradeErrorMessage="Error: A newer version of QEMU guest
> agent is already installed."
> >         />
> > --
> > 2.25.1
> >
> >
>
>
Re: [PATCH 1/2] qga/win32: Remove change action from MSI installer
Posted by Mauro Matteo Cascella 2 years, 11 months ago
Hi Philippe,

On Tue, Feb 21, 2023 at 9:15 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 20/2/23 18:41, Konstantin Kostiuk wrote:
> > resolves: rhbz#2167436
>
> "You are not authorized to access bug #2167436."

Please refer to https://bugzilla.redhat.com/show_bug.cgi?id=2167423.
It should now be accessible.

> > fixes: CVE-2023-0664
>
> This commit description is rather scarce...
>
> I understand you are trying to fix a CVE, but we shouldn't play
> the "security by obscurity" card. How can the community and
> distributions know this security fix is enough with the bare
> "Remove change action from MSI installer" justification?
> Can't we do better?

CCing Brian Wiltse, who originally found and reported this issue.

Reported-by: Brian Wiltse <brian.wiltse@live.com>

> > Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
> > ---
> >   qga/installer/qemu-ga.wxs | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> > index 51340f7ecc..feb629ec47 100644
> > --- a/qga/installer/qemu-ga.wxs
> > +++ b/qga/installer/qemu-ga.wxs
> > @@ -31,6 +31,7 @@
> >         />
> >       <Media Id="1" Cabinet="qemu_ga.$(var.QEMU_GA_VERSION).cab" EmbedCab="yes" />
> >       <Property Id="WHSLogo">1</Property>
> > +    <Property Id="ARPNOMODIFY" Value="yes" Secure="yes" />
> >       <MajorUpgrade
> >         DowngradeErrorMessage="Error: A newer version of QEMU guest agent is already installed."
> >         />
> > --
> > 2.25.1
> >
> >
>

-- 
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
Re: [PATCH 1/2] qga/win32: Remove change action from MSI installer
Posted by Yan Vugenfirer 2 years, 11 months ago
Reviewed-by: Yan Vugenfirer <yvugenfi@redhat.com>

On Mon, Feb 20, 2023 at 7:41 PM Konstantin Kostiuk <kkostiuk@redhat.com> wrote:
>
> resolves: rhbz#2167436
> fixes: CVE-2023-0664
>
> Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
> ---
>  qga/installer/qemu-ga.wxs | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> index 51340f7ecc..feb629ec47 100644
> --- a/qga/installer/qemu-ga.wxs
> +++ b/qga/installer/qemu-ga.wxs
> @@ -31,6 +31,7 @@
>        />
>      <Media Id="1" Cabinet="qemu_ga.$(var.QEMU_GA_VERSION).cab" EmbedCab="yes" />
>      <Property Id="WHSLogo">1</Property>
> +    <Property Id="ARPNOMODIFY" Value="yes" Secure="yes" />
>      <MajorUpgrade
>        DowngradeErrorMessage="Error: A newer version of QEMU guest agent is already installed."
>        />
> --
> 2.25.1
>