[libvirt] [PATCH 05/14] rpc: Be more precise in which cases the authentication is needed

[Marc Hartmayer]

On Fri, Dec 15, 2017 at 01:37 PM +0100, "Daniel P. Berrange" <berrange at redhat.com> 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 at linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
>> Reviewed-by: Bjoern Walk <bwalk at linux.vnet.ibm.com>
>> Reviewed-by: Stefan Zimmermann <stzi at linux.vnet.ibm.com>
>> ---
>>  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 at redhat.com
>> 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