Re: [OE-core] [PATCH 2/2] cve-update-db-native: avoid partial updates
Marta Rybczynska escreveu no dia quarta, 4/01/2023 à(s) 15:28: > Hi Jose, Thanks for looking into that. The function is not a total duplicate: the difference is that the it always removes the db_tmp_file, not only if the journal file exists (Python code formatting!). >>> >>> Don't see on the first time that the db_tmp_file is always removed, I >>> need new glasses :) >>> >>> I was hesitating on this part a bit, because with the old path could be taken only in some specific situations: at the code update and if you share the DL_DIR and some of the builds use the old code, some the new version. I think we should keep both for now for safety. >>> >>> makes sense, thanks for your explanation. >>> >>> >> Well... I'll submit a v2 with comments to make sure nobody tries to >> optimize that piece :) >> > > And here it is (with a minor title change): > https://lists.openembedded.org/g/openembedded-core/message/175355 > Thanks, Jose > > Regards, > Marta > -- Best regards, José Quaresma -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#175501): https://lists.openembedded.org/g/openembedded-core/message/175501 Mute This Topic: https://lists.openembedded.org/mt/96002809/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 2/2] cve-update-db-native: avoid partial updates
> > >>> >>> Hi Jose, >>> Thanks for looking into that. The function is not a total duplicate: the >>> difference is that >>> the it always removes the db_tmp_file, not only if the journal file >>> exists (Python code >>> formatting!). >>> >> >> Don't see on the first time that the db_tmp_file is always removed, I >> need new glasses :) >> >> >>> >>> I was hesitating on this part a bit, because with the old path could be >>> taken only in some >>> specific situations: at the code update and if you share the DL_DIR and >>> some of the builds >>> use the old code, some the new version. I think we should keep both for >>> now for safety. >>> >> >> makes sense, thanks for your explanation. >> >> > Well... I'll submit a v2 with comments to make sure nobody tries to > optimize that piece :) > And here it is (with a minor title change): https://lists.openembedded.org/g/openembedded-core/message/175355 Regards, Marta -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#175486): https://lists.openembedded.org/g/openembedded-core/message/175486 Mute This Topic: https://lists.openembedded.org/mt/96002809/21656 Group Owner: openembedded-core+ow...@lists.openembedded.org Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 2/2] cve-update-db-native: avoid partial updates
On Tue, Jan 3, 2023 at 10:30 AM Jose Quaresma
wrote:
>
>
> Marta Rybczynska escreveu no dia segunda,
> 2/01/2023 à(s) 16:38:
>
>>
>>
>> On Mon, Jan 2, 2023 at 2:14 PM Jose Quaresma
>> wrote:
>>
>>> Hi Marta,
>>>
>>> Marta Rybczynska escreveu no dia segunda,
>>> 2/01/2023 à(s) 07:03:
>>>
The database update has been done on the original file. In case of
network connection issues, temporary outage of the NVD server or
a similar situation, the function could exit with incomplete data
in the database. This patch solves the issue by performing the update
on a copy of the database. It replaces the main one only if the whole
update was successful.
See https://bugzilla.yoctoproject.org/show_bug.cgi?id=14929
Reported-by: Alberto Pianon
Signed-off-by: Marta Rybczynska
---
.../recipes-core/meta/cve-update-db-native.bb | 81 +--
1 file changed, 56 insertions(+), 25 deletions(-)
diff --git a/meta/recipes-core/meta/cve-update-db-native.bb
b/meta/recipes-core/meta/cve-update-db-native.bb
index 642fda5395..89804b9e5c 100644
--- a/meta/recipes-core/meta/cve-update-db-native.bb
+++ b/meta/recipes-core/meta/cve-update-db-native.bb
@@ -21,6 +21,8 @@ CVE_DB_UPDATE_INTERVAL ?= "86400"
# Timeout for blocking socket operations, such as the connection
attempt.
CVE_SOCKET_TIMEOUT ?= "60"
+CVE_DB_TEMP_FILE ?= "${CVE_CHECK_DB_DIR}/temp_nvdcve_1.1.db"
+
python () {
if not bb.data.inherits_class("cve-check", d):
raise bb.parse.SkipRecipe("Skip recipe when cve-check class is
not loaded.")
@@ -32,19 +34,15 @@ python do_fetch() {
"""
import bb.utils
import bb.progress
-import sqlite3, urllib, urllib.parse, gzip
-from datetime import date
+import shutil
bb.utils.export_proxies(d)
-YEAR_START = 2002
-
db_file = d.getVar("CVE_CHECK_DB_FILE")
db_dir = os.path.dirname(db_file)
+db_tmp_file = d.getVar("CVE_DB_TEMP_FILE")
-cve_socket_timeout = int(d.getVar("CVE_SOCKET_TIMEOUT"))
-
-cleanup_db_download(db_file)
+cleanup_db_download(db_file, db_tmp_file)
# The NVD database changes once a day, so no need to update more
frequently
# Allow the user to force-update
@@ -62,9 +60,55 @@ python do_fetch() {
pass
bb.utils.mkdirhier(db_dir)
+if os.path.exists(db_file):
+shutil.copy2(db_file, db_tmp_file)
+
+if update_db_file(db_tmp_file, d) == True:
+# Update downloaded correctly, can swap files
+shutil.move(db_tmp_file, db_file)
+else:
+# Update failed, do not modify the database
+bb.note("CVE database update failed")
+os.remove(db_tmp_file)
+}
+
+do_fetch[lockfiles] += "${CVE_CHECK_DB_FILE_LOCK}"
+do_fetch[file-checksums] = ""
+do_fetch[vardeps] = ""
+
+def cleanup_db_download(db_file, db_tmp_file):
+"""
+Cleanup the download space from possible failed downloads
+"""
+if os.path.exists("{0}-journal".format(db_file)):
+# If a journal is present the last update might have been
interrupted. In that case,
+# just wipe any leftovers and force the DB to be recreated.
+os.remove("{0}-journal".format(db_file))
+
+if os.path.exists(db_file):
+os.remove(db_file)
+
+if os.path.exists("{0}-journal".format(db_tmp_file)):
+# If a journal is present the last update might have been
interrupted. In that case,
+# just wipe any leftovers and force the DB to be recreated.
+os.remove("{0}-journal".format(db_tmp_file))
+
+if os.path.exists(db_tmp_file):
+os.remove(db_tmp_file)
+
>>>
>>> It seems to me that this function is a duplication of the old version
>>> with an extra argument.
>>> So I think that using the old function version and call it with the
>>> proper argument does the same:
>>>
>>> cleanup_db_download(db_file)
>>> cleanup_db_download(db_tmp_file)
>>>
>>>
>> Hi Jose,
>> Thanks for looking into that. The function is not a total duplicate: the
>> difference is that
>> the it always removes the db_tmp_file, not only if the journal file
>> exists (Python code
>> formatting!).
>>
>
> Don't see on the first time that the db_tmp_file is always removed, I need
> new glasses :)
>
>
>>
>> I was hesitating on this part a bit, because with the old path could be
>> taken only in some
>> specific situations: at the code update and if you share the DL_DIR and
>> some of the builds
>> use the old code, some the new version. I think we should keep both for
>> now for safety.
>>
>
> makes sense, thanks for your
Re: [OE-core] [PATCH 2/2] cve-update-db-native: avoid partial updates
Marta Rybczynska escreveu no dia segunda, 2/01/2023
à(s) 16:38:
>
>
> On Mon, Jan 2, 2023 at 2:14 PM Jose Quaresma
> wrote:
>
>> Hi Marta,
>>
>> Marta Rybczynska escreveu no dia segunda,
>> 2/01/2023 à(s) 07:03:
>>
>>> The database update has been done on the original file. In case of
>>> network connection issues, temporary outage of the NVD server or
>>> a similar situation, the function could exit with incomplete data
>>> in the database. This patch solves the issue by performing the update
>>> on a copy of the database. It replaces the main one only if the whole
>>> update was successful.
>>>
>>> See https://bugzilla.yoctoproject.org/show_bug.cgi?id=14929
>>>
>>> Reported-by: Alberto Pianon
>>> Signed-off-by: Marta Rybczynska
>>> ---
>>> .../recipes-core/meta/cve-update-db-native.bb | 81 +--
>>> 1 file changed, 56 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/meta/recipes-core/meta/cve-update-db-native.bb
>>> b/meta/recipes-core/meta/cve-update-db-native.bb
>>> index 642fda5395..89804b9e5c 100644
>>> --- a/meta/recipes-core/meta/cve-update-db-native.bb
>>> +++ b/meta/recipes-core/meta/cve-update-db-native.bb
>>> @@ -21,6 +21,8 @@ CVE_DB_UPDATE_INTERVAL ?= "86400"
>>> # Timeout for blocking socket operations, such as the connection
>>> attempt.
>>> CVE_SOCKET_TIMEOUT ?= "60"
>>>
>>> +CVE_DB_TEMP_FILE ?= "${CVE_CHECK_DB_DIR}/temp_nvdcve_1.1.db"
>>> +
>>> python () {
>>> if not bb.data.inherits_class("cve-check", d):
>>> raise bb.parse.SkipRecipe("Skip recipe when cve-check class is
>>> not loaded.")
>>> @@ -32,19 +34,15 @@ python do_fetch() {
>>> """
>>> import bb.utils
>>> import bb.progress
>>> -import sqlite3, urllib, urllib.parse, gzip
>>> -from datetime import date
>>> +import shutil
>>>
>>> bb.utils.export_proxies(d)
>>>
>>> -YEAR_START = 2002
>>> -
>>> db_file = d.getVar("CVE_CHECK_DB_FILE")
>>> db_dir = os.path.dirname(db_file)
>>> +db_tmp_file = d.getVar("CVE_DB_TEMP_FILE")
>>>
>>> -cve_socket_timeout = int(d.getVar("CVE_SOCKET_TIMEOUT"))
>>> -
>>> -cleanup_db_download(db_file)
>>> +cleanup_db_download(db_file, db_tmp_file)
>>>
>>> # The NVD database changes once a day, so no need to update more
>>> frequently
>>> # Allow the user to force-update
>>> @@ -62,9 +60,55 @@ python do_fetch() {
>>> pass
>>>
>>> bb.utils.mkdirhier(db_dir)
>>> +if os.path.exists(db_file):
>>> +shutil.copy2(db_file, db_tmp_file)
>>> +
>>> +if update_db_file(db_tmp_file, d) == True:
>>> +# Update downloaded correctly, can swap files
>>> +shutil.move(db_tmp_file, db_file)
>>> +else:
>>> +# Update failed, do not modify the database
>>> +bb.note("CVE database update failed")
>>> +os.remove(db_tmp_file)
>>> +}
>>> +
>>> +do_fetch[lockfiles] += "${CVE_CHECK_DB_FILE_LOCK}"
>>> +do_fetch[file-checksums] = ""
>>> +do_fetch[vardeps] = ""
>>> +
>>> +def cleanup_db_download(db_file, db_tmp_file):
>>> +"""
>>> +Cleanup the download space from possible failed downloads
>>> +"""
>>> +if os.path.exists("{0}-journal".format(db_file)):
>>> +# If a journal is present the last update might have been
>>> interrupted. In that case,
>>> +# just wipe any leftovers and force the DB to be recreated.
>>> +os.remove("{0}-journal".format(db_file))
>>> +
>>> +if os.path.exists(db_file):
>>> +os.remove(db_file)
>>> +
>>> +if os.path.exists("{0}-journal".format(db_tmp_file)):
>>> +# If a journal is present the last update might have been
>>> interrupted. In that case,
>>> +# just wipe any leftovers and force the DB to be recreated.
>>> +os.remove("{0}-journal".format(db_tmp_file))
>>> +
>>> +if os.path.exists(db_tmp_file):
>>> +os.remove(db_tmp_file)
>>> +
>>>
>>
>> It seems to me that this function is a duplication of the old version
>> with an extra argument.
>> So I think that using the old function version and call it with the
>> proper argument does the same:
>>
>> cleanup_db_download(db_file)
>> cleanup_db_download(db_tmp_file)
>>
>>
> Hi Jose,
> Thanks for looking into that. The function is not a total duplicate: the
> difference is that
> the it always removes the db_tmp_file, not only if the journal file
> exists (Python code
> formatting!).
>
Don't see on the first time that the db_tmp_file is always removed, I need
new glasses :)
>
> I was hesitating on this part a bit, because with the old path could be
> taken only in some
> specific situations: at the code update and if you share the DL_DIR and
> some of the builds
> use the old code, some the new version. I think we should keep both for
> now for safety.
>
makes sense, thanks for your explanation.
Jose
>
> Kind regards,
> Marta
>
--
Best regards,
José Quaresma
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#175339):
Re: [OE-core] [PATCH 2/2] cve-update-db-native: avoid partial updates
On Mon, Jan 2, 2023 at 2:14 PM Jose Quaresma
wrote:
> Hi Marta,
>
> Marta Rybczynska escreveu no dia segunda,
> 2/01/2023 à(s) 07:03:
>
>> The database update has been done on the original file. In case of
>> network connection issues, temporary outage of the NVD server or
>> a similar situation, the function could exit with incomplete data
>> in the database. This patch solves the issue by performing the update
>> on a copy of the database. It replaces the main one only if the whole
>> update was successful.
>>
>> See https://bugzilla.yoctoproject.org/show_bug.cgi?id=14929
>>
>> Reported-by: Alberto Pianon
>> Signed-off-by: Marta Rybczynska
>> ---
>> .../recipes-core/meta/cve-update-db-native.bb | 81 +--
>> 1 file changed, 56 insertions(+), 25 deletions(-)
>>
>> diff --git a/meta/recipes-core/meta/cve-update-db-native.bb
>> b/meta/recipes-core/meta/cve-update-db-native.bb
>> index 642fda5395..89804b9e5c 100644
>> --- a/meta/recipes-core/meta/cve-update-db-native.bb
>> +++ b/meta/recipes-core/meta/cve-update-db-native.bb
>> @@ -21,6 +21,8 @@ CVE_DB_UPDATE_INTERVAL ?= "86400"
>> # Timeout for blocking socket operations, such as the connection attempt.
>> CVE_SOCKET_TIMEOUT ?= "60"
>>
>> +CVE_DB_TEMP_FILE ?= "${CVE_CHECK_DB_DIR}/temp_nvdcve_1.1.db"
>> +
>> python () {
>> if not bb.data.inherits_class("cve-check", d):
>> raise bb.parse.SkipRecipe("Skip recipe when cve-check class is
>> not loaded.")
>> @@ -32,19 +34,15 @@ python do_fetch() {
>> """
>> import bb.utils
>> import bb.progress
>> -import sqlite3, urllib, urllib.parse, gzip
>> -from datetime import date
>> +import shutil
>>
>> bb.utils.export_proxies(d)
>>
>> -YEAR_START = 2002
>> -
>> db_file = d.getVar("CVE_CHECK_DB_FILE")
>> db_dir = os.path.dirname(db_file)
>> +db_tmp_file = d.getVar("CVE_DB_TEMP_FILE")
>>
>> -cve_socket_timeout = int(d.getVar("CVE_SOCKET_TIMEOUT"))
>> -
>> -cleanup_db_download(db_file)
>> +cleanup_db_download(db_file, db_tmp_file)
>>
>> # The NVD database changes once a day, so no need to update more
>> frequently
>> # Allow the user to force-update
>> @@ -62,9 +60,55 @@ python do_fetch() {
>> pass
>>
>> bb.utils.mkdirhier(db_dir)
>> +if os.path.exists(db_file):
>> +shutil.copy2(db_file, db_tmp_file)
>> +
>> +if update_db_file(db_tmp_file, d) == True:
>> +# Update downloaded correctly, can swap files
>> +shutil.move(db_tmp_file, db_file)
>> +else:
>> +# Update failed, do not modify the database
>> +bb.note("CVE database update failed")
>> +os.remove(db_tmp_file)
>> +}
>> +
>> +do_fetch[lockfiles] += "${CVE_CHECK_DB_FILE_LOCK}"
>> +do_fetch[file-checksums] = ""
>> +do_fetch[vardeps] = ""
>> +
>> +def cleanup_db_download(db_file, db_tmp_file):
>> +"""
>> +Cleanup the download space from possible failed downloads
>> +"""
>> +if os.path.exists("{0}-journal".format(db_file)):
>> +# If a journal is present the last update might have been
>> interrupted. In that case,
>> +# just wipe any leftovers and force the DB to be recreated.
>> +os.remove("{0}-journal".format(db_file))
>> +
>> +if os.path.exists(db_file):
>> +os.remove(db_file)
>> +
>> +if os.path.exists("{0}-journal".format(db_tmp_file)):
>> +# If a journal is present the last update might have been
>> interrupted. In that case,
>> +# just wipe any leftovers and force the DB to be recreated.
>> +os.remove("{0}-journal".format(db_tmp_file))
>> +
>> +if os.path.exists(db_tmp_file):
>> +os.remove(db_tmp_file)
>> +
>>
>
> It seems to me that this function is a duplication of the old version with
> an extra argument.
> So I think that using the old function version and call it with the proper
> argument does the same:
>
> cleanup_db_download(db_file)
> cleanup_db_download(db_tmp_file)
>
>
Hi Jose,
Thanks for looking into that. The function is not a total duplicate: the
difference is that
the it always removes the db_tmp_file, not only if the journal file exists
(Python code
formatting!).
I was hesitating on this part a bit, because with the old path could be
taken only in some
specific situations: at the code update and if you share the DL_DIR and
some of the builds
use the old code, some the new version. I think we should keep both for now
for safety.
Kind regards,
Marta
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#175322):
https://lists.openembedded.org/g/openembedded-core/message/175322
Mute This Topic: https://lists.openembedded.org/mt/96002809/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [OE-core] [PATCH 2/2] cve-update-db-native: avoid partial updates
Hi Marta,
Marta Rybczynska escreveu no dia segunda, 2/01/2023
à(s) 07:03:
> The database update has been done on the original file. In case of
> network connection issues, temporary outage of the NVD server or
> a similar situation, the function could exit with incomplete data
> in the database. This patch solves the issue by performing the update
> on a copy of the database. It replaces the main one only if the whole
> update was successful.
>
> See https://bugzilla.yoctoproject.org/show_bug.cgi?id=14929
>
> Reported-by: Alberto Pianon
> Signed-off-by: Marta Rybczynska
> ---
> .../recipes-core/meta/cve-update-db-native.bb | 81 +--
> 1 file changed, 56 insertions(+), 25 deletions(-)
>
> diff --git a/meta/recipes-core/meta/cve-update-db-native.bb
> b/meta/recipes-core/meta/cve-update-db-native.bb
> index 642fda5395..89804b9e5c 100644
> --- a/meta/recipes-core/meta/cve-update-db-native.bb
> +++ b/meta/recipes-core/meta/cve-update-db-native.bb
> @@ -21,6 +21,8 @@ CVE_DB_UPDATE_INTERVAL ?= "86400"
> # Timeout for blocking socket operations, such as the connection attempt.
> CVE_SOCKET_TIMEOUT ?= "60"
>
> +CVE_DB_TEMP_FILE ?= "${CVE_CHECK_DB_DIR}/temp_nvdcve_1.1.db"
> +
> python () {
> if not bb.data.inherits_class("cve-check", d):
> raise bb.parse.SkipRecipe("Skip recipe when cve-check class is
> not loaded.")
> @@ -32,19 +34,15 @@ python do_fetch() {
> """
> import bb.utils
> import bb.progress
> -import sqlite3, urllib, urllib.parse, gzip
> -from datetime import date
> +import shutil
>
> bb.utils.export_proxies(d)
>
> -YEAR_START = 2002
> -
> db_file = d.getVar("CVE_CHECK_DB_FILE")
> db_dir = os.path.dirname(db_file)
> +db_tmp_file = d.getVar("CVE_DB_TEMP_FILE")
>
> -cve_socket_timeout = int(d.getVar("CVE_SOCKET_TIMEOUT"))
> -
> -cleanup_db_download(db_file)
> +cleanup_db_download(db_file, db_tmp_file)
>
> # The NVD database changes once a day, so no need to update more
> frequently
> # Allow the user to force-update
> @@ -62,9 +60,55 @@ python do_fetch() {
> pass
>
> bb.utils.mkdirhier(db_dir)
> +if os.path.exists(db_file):
> +shutil.copy2(db_file, db_tmp_file)
> +
> +if update_db_file(db_tmp_file, d) == True:
> +# Update downloaded correctly, can swap files
> +shutil.move(db_tmp_file, db_file)
> +else:
> +# Update failed, do not modify the database
> +bb.note("CVE database update failed")
> +os.remove(db_tmp_file)
> +}
> +
> +do_fetch[lockfiles] += "${CVE_CHECK_DB_FILE_LOCK}"
> +do_fetch[file-checksums] = ""
> +do_fetch[vardeps] = ""
> +
> +def cleanup_db_download(db_file, db_tmp_file):
> +"""
> +Cleanup the download space from possible failed downloads
> +"""
> +if os.path.exists("{0}-journal".format(db_file)):
> +# If a journal is present the last update might have been
> interrupted. In that case,
> +# just wipe any leftovers and force the DB to be recreated.
> +os.remove("{0}-journal".format(db_file))
> +
> +if os.path.exists(db_file):
> +os.remove(db_file)
> +
> +if os.path.exists("{0}-journal".format(db_tmp_file)):
> +# If a journal is present the last update might have been
> interrupted. In that case,
> +# just wipe any leftovers and force the DB to be recreated.
> +os.remove("{0}-journal".format(db_tmp_file))
> +
> +if os.path.exists(db_tmp_file):
> +os.remove(db_tmp_file)
> +
>
It seems to me that this function is a duplication of the old version with
an extra argument.
So I think that using the old function version and call it with the proper
argument does the same:
cleanup_db_download(db_file)
cleanup_db_download(db_tmp_file)
Jose
> +def update_db_file(db_tmp_file, d):
> +"""
> +Update the given database file
> +"""
> +import bb.utils, bb.progress
> +from datetime import date
> +import urllib, gzip, sqlite3
> +
> +YEAR_START = 2002
> +cve_socket_timeout = int(d.getVar("CVE_SOCKET_TIMEOUT"))
>
> # Connect to database
> -conn = sqlite3.connect(db_file)
> +conn = sqlite3.connect(db_tmp_file)
> initialize_db(conn)
>
> with bb.progress.ProgressHandler(d) as ph,
> open(os.path.join(d.getVar("TMPDIR"), 'cve_check'), 'a') as cve_f:
> @@ -82,7 +126,7 @@ python do_fetch() {
> except urllib.error.URLError as e:
> cve_f.write('Warning: CVE db update error, Unable to
> fetch CVE data.\n\n')
> bb.warn("Failed to fetch CVE data (%s)" % e.reason)
> -return
> +return False
>
> if response:
> for l in response.read().decode("utf-8").splitlines():
> @@ -92,7 +136,7 @@ python do_fetch() {
> break
> else:
> bb.warn("Cannot parse CVE metadata, update failed")
> -return
> +
