[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 08/11] authz: add QAuthZList object type for an a
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PULL 08/11] authz: add QAuthZList object type for an access control list |
Date: |
Tue, 26 Feb 2019 07:04:51 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Eric Blake <address@hidden> writes:
> I missed reviewing this before the pull request, so comments here are
> best for a followup patch:
I procrastinated, same result. My apologies.
Followup or quick respin is up to you. I'd respin as long as the
changes are trivial.
> On 2/25/19 6:31 AM, Daniel P. Berrangé wrote:
>> From: "Daniel P. Berrange" <address@hidden>
>>
>> Add a QAuthZList object type that implements the QAuthZ interface. This
>> built-in implementation maintains a trivial access control list with a
>> sequence of match rules and a final default policy. This replicates the
>> functionality currently provided by the qemu_acl module.
>>
>
>> Reviewed-by: Marc-André Lureau <address@hidden>
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>> Tested-by: Philippe Mathieu-Daudé <address@hidden>
>> Signed-off-by: Daniel P. Berrange <address@hidden>
>> ---
>
>> +++ b/qapi/Makefile.objs
>> @@ -7,7 +7,7 @@ util-obj-y += qapi-util.o
>>
>> QAPI_COMMON_MODULES = block-core block char common crypto introspect
>> QAPI_COMMON_MODULES += job migration misc net rdma rocker run-state
>> -QAPI_COMMON_MODULES += sockets tpm trace transaction ui
>> +QAPI_COMMON_MODULES += sockets tpm trace transaction ui authz
>
> Let's keep this list alphabetically sorted (authz before block-core).
Yes, please.
>> +++ b/qapi/authz.json
>> @@ -0,0 +1,58 @@
>> +# -*- Mode: Python -*-
>> +#
>> +# QAPI authz definitions
>> +
>> +##
>> +# @QAuthZListPolicy:
>> +#
>> +# The authorization policy result
>> +#
>> +# @deny: deny access
>> +# @allow: allow access
>> +#
>> +# Since: 4.0
>> +##
>> +{ 'enum': 'QAuthZListPolicy',
>> + 'prefix': 'QAUTHZ_LIST_POLICY',
>> + 'data': ['deny', 'allow']}
>> +
>> +##
>> +# @QAuthZListFormat:
>> +#
>> +# The authorization policy result
Pasto?
>> +#
>> +# @exact: an exact string match
>> +# @glob: string with ? and * shell wildcard support
>
> Does it actually use glob() (in which case it also has [] glob support?)
>
>> +#
>> +# Since: 4.0
>> +##
>> +{ 'enum': 'QAuthZListFormat',
>> + 'prefix': 'QAUTHZ_LIST_FORMAT',
>> + 'data': ['exact', 'glob']}
>> +
>> +##
>> +# @QAuthZListRule:
>> +#
>> +# A single authorization rule.
>> +#
>> +# @match: a glob to match against a user identity
>
> Should this read 'a string or glob to match...' since...
>
>> +# @policy: the result to return if @match evaluates to true
>> +# @format: (optional) the format of the @match rule (default 'exact')
>
> ...format controls which of the two styles it is interpreted as? The
> use of '(optional)' is not required in the current QAPI doc generator,
> and in fact results in redundant output.
Please drop (optional).
>
>> +#
>> +# Since: 4.0
>> +##
>> +{ 'struct': 'QAuthZListRule',
>> + 'data': {'match': 'str',
>> + 'policy': 'QAuthZListPolicy',
>> + '*format': 'QAuthZListFormat'}}
>> +
>> +##
>> +# @QAuthZListRuleListHack:
>> +#
>> +# Not exposed via QMP; hack to generate QAuthZListRuleList
>> +# for use internally by the code.
>> +#
>> +# Since: 4.0
>> +##
>> +{ 'struct': 'QAuthZListRuleListHack',
>> + 'data': { 'unused': ['QAuthZListRule'] } }
>
> We keep on encountering these hacks; someday it would be nice to teach
> the QAPI generator a nicer way to do this. But not your problem.
- [Qemu-devel] [PULL 00/11] Merge authz core patches, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 02/11] qom: don't require user creatable objects to be registered, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 01/11] util: add helper APIs for dealing with inotify in portable manner, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 03/11] hw/usb: don't set IN_ISDIR for inotify watch in MTP driver, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 04/11] hw/usb: fix const-ness for string params in MTP driver, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 06/11] authz: add QAuthZ object as an authorization base class, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 05/11] hw/usb: switch MTP to use new inotify APIs, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 07/11] authz: add QAuthZSimple object type for easy whitelist auth checks, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 08/11] authz: add QAuthZList object type for an access control list, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 10/11] authz: add QAuthZPAM object type for authorizing using PAM, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 11/11] authz: delete existing ACL implementation, Daniel P . Berrangé, 2019/02/25
- [Qemu-devel] [PULL 09/11] authz: add QAuthZListFile object type for a file access control list, Daniel P . Berrangé, 2019/02/25
- Re: [Qemu-devel] [PULL 00/11] Merge authz core patches, Peter Maydell, 2019/02/26