[2/2] cve-update-nvd2-native: Add an age threshold for incremental update

Add a new variable "CVE_DB_INCR_UPDATE_AGE_THRES", which can be used to
specify the maximum age of the database for doing an incremental update
For older databases, a full re-download is done.

With a value of "0", this forces a full-redownload.

Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
---
 .../meta/cve-update-nvd2-native.bb            | 20 +++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

On Wed, 13 Mar 2024, 16:15 Yoann Congal, <yoann.congal@smile.fr> wrote:

> Add a new variable "CVE_DB_INCR_UPDATE_AGE_THRES", which can be used to
> specify the maximum age of the database for doing an incremental update
> For older databases, a full re-download is done.
>
> With a value of "0", this forces a full-redownload.
>
> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
> ---
>  .../meta/cve-update-nvd2-native.bb            | 20 +++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/meta/recipes-core/meta/cve-update-nvd2-native.bb
> b/meta/recipes-core/meta/cve-update-nvd2-native.bb
> index f21c139aa5..d565887498 100644
> --- a/meta/recipes-core/meta/cve-update-nvd2-native.bb
> +++ b/meta/recipes-core/meta/cve-update-nvd2-native.bb
> @@ -26,6 +26,12 @@ NVDCVE_API_KEY ?= ""
>  # Use a negative value to skip the update
>  CVE_DB_UPDATE_INTERVAL ?= "86400"
>
> +# CVE database incremental update age threshold, in seconds. If the
> database is
> +# older than this threshold, do a full re-download, else, do an
> incremental
> +# update. By default: the maximum allowed value from NVD: 120 days
> (120*24*60*60)
> +# Use 0 to force a full download.
> +CVE_DB_INCR_UPDATE_AGE_THRES ?= "10368000"
> +
>  # Number of attempts for each http query to nvd server before giving up
>  CVE_DB_UPDATE_ATTEMPTS ?= "5"
>
> @@ -172,18 +178,24 @@ def update_db_file(db_tmp_file, d, database_time):
>
>      req_args = {'startIndex' : 0}
>
> -    # The maximum range for time is 120 days
> -    # Force a complete update if our range is longer
> -    if (database_time != 0):
> +    incr_update_threshold = int(d.getVar("CVE_DB_INCR_UPDATE_AGE_THRES"))
> +    if database_time != 0:
>          database_date = datetime.datetime.fromtimestamp(database_time,
> tz=datetime.timezone.utc)
>          today_date = datetime.datetime.now(tz=datetime.timezone.utc)
>          delta = today_date - database_date
> -        if delta.days < 120:
> +        if incr_update_threshold == 0:
> +            bb.note("CVE database: forced full update")
> +        elif delta < datetime.timedelta(seconds=incr_update_threshold):
>              bb.note("CVE database: performing partial update")
> +            # The maximum range for time is 120 days
> +            if delta > datetime.timedelta(days=120):
> +                bb.error("CVE database: Trying to do an incremental
> update on a larger than supported range")
>              req_args['lastModStartDate'] = database_date.isoformat()
>              req_args['lastModEndDate'] = today_date.isoformat()
>          else:
>              bb.note("CVE database: file too old, forcing a full update")
> +    else:
> +        bb.note("CVE database: no preexisting database, do a full
> download")
>
>      with bb.progress.ProgressHandler(d) as ph,
> open(os.path.join(d.getVar("TMPDIR"), 'cve_check'), 'a') as cve_f:
>
> --
>

I find naming of the variable misleading, sourcing like FORCE_FULL_DOWNLOAD
would be more obvious. Also, I'm not sure we need a variable if the user
can just delete the database file and force the download this way. The 120
days case is special, because its a limit set in the spec.

Regards,
Marta

>

Le jeu. 14 mars 2024 à 13:14, Marta Rybczynska <rybczynska@gmail.com> a
écrit :

>
>
> On Wed, 13 Mar 2024, 16:15 Yoann Congal, <yoann.congal@smile.fr> wrote:
>
>> Add a new variable "CVE_DB_INCR_UPDATE_AGE_THRES", which can be used to
>> specify the maximum age of the database for doing an incremental update
>> For older databases, a full re-download is done.
>>
>> With a value of "0", this forces a full-redownload.
>>
>> Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
>> ---
>>  .../meta/cve-update-nvd2-native.bb            | 20 +++++++++++++++----
>>  1 file changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/meta/recipes-core/meta/cve-update-nvd2-native.bb
>> b/meta/recipes-core/meta/cve-update-nvd2-native.bb
>> index f21c139aa5..d565887498 100644
>> --- a/meta/recipes-core/meta/cve-update-nvd2-native.bb
>> +++ b/meta/recipes-core/meta/cve-update-nvd2-native.bb
>> @@ -26,6 +26,12 @@ NVDCVE_API_KEY ?= ""
>>  # Use a negative value to skip the update
>>  CVE_DB_UPDATE_INTERVAL ?= "86400"
>>
>> +# CVE database incremental update age threshold, in seconds. If the
>> database is
>> +# older than this threshold, do a full re-download, else, do an
>> incremental
>> +# update. By default: the maximum allowed value from NVD: 120 days
>> (120*24*60*60)
>> +# Use 0 to force a full download.
>> +CVE_DB_INCR_UPDATE_AGE_THRES ?= "10368000"
>> +
>>  # Number of attempts for each http query to nvd server before giving up
>>  CVE_DB_UPDATE_ATTEMPTS ?= "5"
>>
>> @@ -172,18 +178,24 @@ def update_db_file(db_tmp_file, d, database_time):
>>
>>      req_args = {'startIndex' : 0}
>>
>> -    # The maximum range for time is 120 days
>> -    # Force a complete update if our range is longer
>> -    if (database_time != 0):
>> +    incr_update_threshold = int(d.getVar("CVE_DB_INCR_UPDATE_AGE_THRES"))
>> +    if database_time != 0:
>>          database_date = datetime.datetime.fromtimestamp(database_time,
>> tz=datetime.timezone.utc)
>>          today_date = datetime.datetime.now(tz=datetime.timezone.utc)
>>          delta = today_date - database_date
>> -        if delta.days < 120:
>> +        if incr_update_threshold == 0:
>> +            bb.note("CVE database: forced full update")
>> +        elif delta < datetime.timedelta(seconds=incr_update_threshold):
>>              bb.note("CVE database: performing partial update")
>> +            # The maximum range for time is 120 days
>> +            if delta > datetime.timedelta(days=120):
>> +                bb.error("CVE database: Trying to do an incremental
>> update on a larger than supported range")
>>              req_args['lastModStartDate'] = database_date.isoformat()
>>              req_args['lastModEndDate'] = today_date.isoformat()
>>          else:
>>              bb.note("CVE database: file too old, forcing a full update")
>> +    else:
>> +        bb.note("CVE database: no preexisting database, do a full
>> download")
>>
>>      with bb.progress.ProgressHandler(d) as ph,
>> open(os.path.join(d.getVar("TMPDIR"), 'cve_check'), 'a') as cve_f:
>>
>> --
>>
>
>
Hi Marta,

Thanks for your review!

If you had not noticed, this patch has been merged here
https://git.yoctoproject.org/poky/commit/?id=19f27037b2b785673c8f68f19ea783856f732e4d
And was used this morning by AB
https://autobuilder.yoctoproject.org/typhoon/#/builders/138/builds/1326 =>
It does not work as expected, I'm working on it.

I find naming of the variable misleading, sourcing like FORCE_FULL_DOWNLOAD
> would be more obvious.
>
I agree FORCE_FULL_DOWNLOAD is more obvious. But, I've chosen a more
nuanced variable to allow a customised workflow like the one we want on AB:
fulldownload once a day, no update for other daily cve-check runs.


> Also, I'm not sure we need a variable if the user can just delete the
> database file and force the download this way.
>

It is not usually expected of users to manually modify the content of
DL_DIR. Here, I feel like a variable is cleaner and (once documented), will
be more easy for users.


> The 120 days case is special, because its a limit set in the spec.
>

Yes, that what this hunk test:
+            # The maximum range for time is 120 days
+            if delta > datetime.timedelta(days=120):
+                bb.error("CVE database: Trying to do an incremental update
on a larger than supported range")
maybe the error message is not clear enough?

Regards,