[libvirt] [PATCH v2 1/2] NSS: Add explicit check to not report expired lease

[Nehal J Wani]

On Mon, Oct 3, 2016 at 1:19 PM, Michal Privoznik <mprivozn at redhat.com> wrote:
> On 30.09.2016 17:11, Nehal J Wani wrote:
>> The NSS module shouldn't rely on custom leases database to not have
>> entries for leases which have expired.
>>
>> Change-Id: Ic3e043003d33ded0da74696a1d27ed4967ddbfb8
>
> Oh, is this a result of some git hook script?

Yes, indeed. I forgot to remove it. My bad. #GerritChronicles
>
>> ---
>>  tools/nss/libvirt_nss.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
>> index 54c4a2a..cb3edf5 100644
>> --- a/tools/nss/libvirt_nss.c
>> +++ b/tools/nss/libvirt_nss.c
>> @@ -42,6 +42,7 @@
>>  #include "virlease.h"
>>  #include "viralloc.h"
>>  #include "virfile.h"
>> +#include "virtime.h"
>>  #include "virerror.h"
>>  #include "virstring.h"
>>  #include "virsocketaddr.h"
>> @@ -114,6 +115,8 @@ findLease(const char *name,
>>      ssize_t i, nleases;
>>      leaseAddress *tmpAddress = NULL;
>>      size_t ntmpAddress = 0;
>> +    time_t currtime;
>> +    long long expirytime;
>>
>>      *address = NULL;
>>      *naddress = 0;
>> @@ -161,6 +164,11 @@ findLease(const char *name,
>>      nleases = virJSONValueArraySize(leases_array);
>>      DEBUG("Read %zd leases", nleases);
>>
>> +    if ((currtime = time(NULL)) == (time_t) - 1) {
>> +        ERROR("Failed to get current system time");
>> +        goto cleanup;
>> +    }
>> +
>>      for (i = 0; i < nleases; i++) {
>>          virJSONValuePtr lease;
>>          const char *lease_name;
>> @@ -181,6 +189,16 @@ findLease(const char *name,
>>          if (STRNEQ_NULLABLE(name, lease_name))
>>              continue;
>>
>> +        if (virJSONValueObjectGetNumberLong(lease, "expiry-time", &expirytime) < 0) {
>> +            /* A lease cannot be present without expiry-time */
>> +            ERROR("expiry-time field missing for %s", name);
>> +            goto cleanup;
>> +        }
>> +
>> +        /* Do not report expired lease */
>> +        if (expirytime < (long long) currtime)
>
> I am putting here a debug message (which is no-op unless enabled) as it
> might help me in debugging in the future.

Maybe we should at that debug message at other places too, where we do
this, for example, in networkGetDHCPLeases()
>
>> +            continue;
>> +
>>          DEBUG("Found record for %s", lease_name);
>>          *found = true;
>>
>>
>
> ACK
>
> Michal
>
>



-- 
Nehal J Wani