On Fri, Dec 15, 2017 at 01:37 PM +0100, "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
> On Tue, Dec 12, 2017 at 12:36:27PM +0100, Marc Hartmayer wrote:
>> Additionally, use a whitelist model to decide whether authentication
>> is needed or not.
>
> Is this actually fixing any real problem, if so please document what
> the problem is.
It 'tells' the developer what we’re doing here. At first glance, it
wasn’t obvious to me how the authentication process works in
libvirt. This change doesn’t hurt someone, but at least it should be
easier for other developer to understand. Additionally, the default
return value is changing to True (=> client is not authenticated). So if
something may change at the authentication process (for example the
value for the enums - I know it’s unlikely to happen here…) we’re sure
that the user isn’t authenticated by accident.
>
> AFAICT, this is mostly just a case of painting the bikeshed a different
> colour, as both old & new code seem to have the same result in all cases ?
>
>>
>> Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxxxxxxx>
>> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
>> Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
>> Reviewed-by: Stefan Zimmermann <stzi@xxxxxxxxxxxxxxxxxx>
>> ---
>> src/rpc/virnetserverclient.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
>> index b454a3ff6992..0ee299e2d6ec 100644
>> --- a/src/rpc/virnetserverclient.c
>> +++ b/src/rpc/virnetserverclient.c
>> @@ -1512,10 +1512,10 @@ int virNetServerClientSendMessage(virNetServerClientPtr client,
>>
>> bool virNetServerClientNeedAuth(virNetServerClientPtr client)
>> {
>> - bool need = false;
>> + bool need = true;
>> virObjectLock(client);
>> - if (client->auth)
>> - need = true;
>> + if (client->auth == VIR_NET_SERVER_SERVICE_AUTH_NONE)
>> + need = false;
>> virObjectUnlock(client);
>> return need;
>> }
>> --
>> 2.13.4
>>
>> --
>> libvir-list mailing list
>> libvir-list@xxxxxxxxxx
>> https://www.redhat.com/mailman/listinfo/libvir-list
>
> 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 :|
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
