[PULL 0/4] Misc fixes patches

[PULL 0/4] Misc fixes patches

[Daniel P. Berrangé]

The following changes since commit 66a300a107ec286725bdc943601cbd4247b82158:

  Update version for v5.2.0-rc2 release (2020-11-17 22:58:10 +0000)

are available in the Git repository at:

  https://gitlab.com/berrange/qemu tags/misc-fixes-pull-request

for you to fetch changes up to c2aa8a3d7e5ce57fa3df310c9b7ca48fcbf9d4ad:

  authz-simple: Check that 'identity' property is set (2020-11-18 10:51:35 +0=
000)

----------------------------------------------------------------
Misc error reporting and checking fixes to authorization objects

----------------------------------------------------------------

Kevin Wolf (2):
  authz-pam: Check that 'service' property is set
  authz-simple: Check that 'identity' property is set

Markus Armbruster (2):
  authz-list-file: Fix file read error handling
  authz-list-file: Improve an error message

 authz/listfile.c |  6 +++++-
 authz/pamacct.c  |  6 ++++++
 authz/simple.c   | 14 ++++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

--=20
2.28.0

[PULL 1/4] authz-list-file: Fix file read error handling

[Daniel P. Berrangé]

From: Markus Armbruster <armbru@redhat.com>

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

qauthz_list_file_complete() is wrong that way: it passes @errp to
qauthz_list_file_complete() without checking for failure.  If it runs
into another failure, it trips error_setv()'s assertion.  Reproducer:

    $ qemu-system-x86_64 -nodefaults -S -display none -object authz-list-file,id=authz0,filename=
    qemu-system-x86_64: ../util/error.c:59: error_setv: Assertion `*errp == NULL' failed.
    Aborted (core dumped)

Fix it to check for failure.

Fixes: 55d869846de802a16af1a50584c51737bd664387
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 authz/listfile.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/authz/listfile.c b/authz/listfile.c
index 24feac35ab..1421e674a4 100644
--- a/authz/listfile.c
+++ b/authz/listfile.c
@@ -128,6 +128,9 @@ qauthz_list_file_complete(UserCreatable *uc, Error **errp)
     }
 
     fauthz->list = qauthz_list_file_load(fauthz, errp);
+    if (!fauthz->list) {
+        return;
+    }
 
     if (!fauthz->refresh) {
         return;
-- 
2.28.0

[PULL 2/4] authz-list-file: Improve an error message

[Daniel P. Berrangé]

From: Markus Armbruster <armbru@redhat.com>

When qauthz_list_file_load() rejects JSON values other than JSON
object with a rather confusing error message:

    $ echo 1 | qemu-system-x86_64 -nodefaults -S -display none  -object authz-list-file,id=authz0,filename=/dev/stdin
    qemu-system-x86_64: -object authz-list-file,id=authz0,filename=/dev/stdin: Invalid parameter type for 'obj', expected: dict

Improve to

    qemu-system-x86_64: -object authz-list-file,id=authz0,filename=/dev/stdin: File '/dev/stdin' must contain a JSON object

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 authz/listfile.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/authz/listfile.c b/authz/listfile.c
index 1421e674a4..da3a0e69a2 100644
--- a/authz/listfile.c
+++ b/authz/listfile.c
@@ -73,7 +73,8 @@ qauthz_list_file_load(QAuthZListFile *fauthz, Error **errp)
 
     pdict = qobject_to(QDict, obj);
     if (!pdict) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "obj", "dict");
+        error_setg(errp, "File '%s' must contain a JSON object",
+                   fauthz->filename);
         goto cleanup;
     }
 
-- 
2.28.0

[PULL 3/4] authz-pam: Check that 'service' property is set

[Daniel P. Berrangé]

From: Kevin Wolf <kwolf@redhat.com>

If the 'service' property is not set, we'll call pam_start() with a NULL
pointer for the service name. This fails and leaves a message like this
in the syslog:

qemu-storage-daemon[294015]: PAM pam_start: invalid argument: service == NULL

Make specifying the property mandatory and catch the error already
during the creation of the object.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 authz/pamacct.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/authz/pamacct.c b/authz/pamacct.c
index e67195f7be..c862d9ff39 100644
--- a/authz/pamacct.c
+++ b/authz/pamacct.c
@@ -84,6 +84,12 @@ qauthz_pam_prop_get_service(Object *obj,
 static void
 qauthz_pam_complete(UserCreatable *uc, Error **errp)
 {
+    QAuthZPAM *pauthz = QAUTHZ_PAM(uc);
+
+    if (!pauthz->service) {
+        error_setg(errp, "The 'service' property must be set");
+        return;
+    }
 }
 
 
-- 
2.28.0

[PULL 4/4] authz-simple: Check that 'identity' property is set

[Daniel P. Berrangé]

From: Kevin Wolf <kwolf@redhat.com>

If the 'identify' property is not set, we'll pass a NULL pointer to
g_str_equal() and crash. Catch the error condition during the creation
of the object.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 authz/simple.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/authz/simple.c b/authz/simple.c
index 18db0355f4..0597dcd8ea 100644
--- a/authz/simple.c
+++ b/authz/simple.c
@@ -65,11 +65,25 @@ qauthz_simple_finalize(Object *obj)
 }
 
 
+static void
+qauthz_simple_complete(UserCreatable *uc, Error **errp)
+{
+    QAuthZSimple *sauthz = QAUTHZ_SIMPLE(uc);
+
+    if (!sauthz->identity) {
+        error_setg(errp, "The 'identity' property must be set");
+        return;
+    }
+}
+
+
 static void
 qauthz_simple_class_init(ObjectClass *oc, void *data)
 {
     QAuthZClass *authz = QAUTHZ_CLASS(oc);
+    UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
 
+    ucc->complete = qauthz_simple_complete;
     authz->is_allowed = qauthz_simple_is_allowed;
 
     object_class_property_add_str(oc, "identity",
-- 
2.28.0

Re: [PULL 0/4] Misc fixes patches

[Peter Maydell]

On Wed, 18 Nov 2020 at 12:51, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The following changes since commit 66a300a107ec286725bdc943601cbd4247b82158:
>
>   Update version for v5.2.0-rc2 release (2020-11-17 22:58:10 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/berrange/qemu tags/misc-fixes-pull-request
>
> for you to fetch changes up to c2aa8a3d7e5ce57fa3df310c9b7ca48fcbf9d4ad:
>
>   authz-simple: Check that 'identity' property is set (2020-11-18 10:51:35 +0=
> 000)
>
> ----------------------------------------------------------------
> Misc error reporting and checking fixes to authorization objects
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM