* [PULL 1/4] authz-list-file: Fix file read error handling
2020-11-18 12:49 [PULL 0/4] Misc fixes patches Daniel P. Berrangé
@ 2020-11-18 12:49 ` Daniel P. Berrangé
2020-11-18 12:49 ` [PULL 2/4] authz-list-file: Improve an error message Daniel P. Berrangé
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-11-18 12:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Markus Armbruster
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PULL 2/4] authz-list-file: Improve an error message
2020-11-18 12:49 [PULL 0/4] Misc fixes patches Daniel P. Berrangé
2020-11-18 12:49 ` [PULL 1/4] authz-list-file: Fix file read error handling Daniel P. Berrangé
@ 2020-11-18 12:49 ` Daniel P. Berrangé
2020-11-18 12:49 ` [PULL 3/4] authz-pam: Check that 'service' property is set Daniel P. Berrangé
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-11-18 12:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Markus Armbruster
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PULL 3/4] authz-pam: Check that 'service' property is set
2020-11-18 12:49 [PULL 0/4] Misc fixes patches Daniel P. Berrangé
2020-11-18 12:49 ` [PULL 1/4] authz-list-file: Fix file read error handling Daniel P. Berrangé
2020-11-18 12:49 ` [PULL 2/4] authz-list-file: Improve an error message Daniel P. Berrangé
@ 2020-11-18 12:49 ` Daniel P. Berrangé
2020-11-18 12:49 ` [PULL 4/4] authz-simple: Check that 'identity' " Daniel P. Berrangé
2020-11-18 15:24 ` [PULL 0/4] Misc fixes patches Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-11-18 12:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PULL 4/4] authz-simple: Check that 'identity' property is set
2020-11-18 12:49 [PULL 0/4] Misc fixes patches Daniel P. Berrangé
` (2 preceding siblings ...)
2020-11-18 12:49 ` [PULL 3/4] authz-pam: Check that 'service' property is set Daniel P. Berrangé
@ 2020-11-18 12:49 ` Daniel P. Berrangé
2020-11-18 15:24 ` [PULL 0/4] Misc fixes patches Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-11-18 12:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PULL 0/4] Misc fixes patches
2020-11-18 12:49 [PULL 0/4] Misc fixes patches Daniel P. Berrangé
` (3 preceding siblings ...)
2020-11-18 12:49 ` [PULL 4/4] authz-simple: Check that 'identity' " Daniel P. Berrangé
@ 2020-11-18 15:24 ` Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2020-11-18 15:24 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: QEMU Developers
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
^ permalink raw reply [flat|nested] 6+ messages in thread