Skip to content
Snippets Groups Projects

Add support for additional DSA lists and CVE extends

Merged Emilio Pozuelo Monfort requested to merge pochu/security-tracker:wip/extends into master

This adds support for CVE extends, so that one can have a separate file which gets merged with CVE/list into the database.

It also allows to add more DSA-like lists. All of this while simplifying some code to avoid duplication (e.g. DLAFile).

The branch needs some squashing but I left as is for now as it may make it easier for review. In any case one can look at the 'changes' in the MR to get a squashed view of the changes.

The config file should probably be moved to a real config file

I'm only aware of one problem, which is marked with a FIXME and that I should fix before this gets merged.

To test or use this, one can just create a data/MY-CVE-EXTENDS/list file and add (bugs.CVEExtendFile, '/MY-CVE-EXTENDS/list') to lib/python/config.py

For another announce project (similar to DLA), just create data/MYSA/list and add it as (bugs.DSAFile, '/MYSA/list'). The changes to bin/gen-DSA help support a bin/gen-MYSA symlink which does the right thing.

@waldi can you take a look and let me know what you think of my changes?

Edited by Emilio Pozuelo Monfort

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • I would prefer a real config. Apart from that, I did not test it, but it looks okay.

  • Looks like a good start. Here are my suggestions.

    • Better than a configuration file, we might want a configuration directory so that we are sure that we will avoid merge conflict (we want to automate the merge with the upstream tracker so it's best to make sure of this). Defining the same distribution twice should be allowed and the last definition should take precedence.

    • Maybe the configuration files should merge also data from lib/python/dist_config.py?

    • We want to add some documentation in "doc/vendor-support.txt" or something like that. It should explain how to setup the tracker for a third party who wants to track security updates for a distribution derived from Debian. It should explain how to indicate the URLs where available packages are monitored.

    The last point is my main question. What should I change to tell the security tracker to monitor the wheezy distribution in another repository ?

  • For whatever triggers your updates, you need to make sure that the Packages file land in data/packages/${rel}__${archive}_${arch}_Packages (cf. lib/python/security_db.py). The security-tracker instance for https://security-tracker.debian.org does this by using cronjob and other triggers and using the Makefile in toplevel directory, where there are as well configured the mirror(s) to use for the Packages files download.

  • @carnil Thanks for those details. Just so that I don't miss anything, can you share the crontabs that you are using? What are the other triggers? Do you have some webhook to trigger an update when the git repository is updated? Is there some locking to avoid concurrent updates?

  • @hertzog

    @carnil Thanks for those details. Just so that I don't miss anything, can you share the crontabs that you are using?

    The crontab used is commited in the security-tracker-service repository:

    https://salsa.debian.org/security-tracker-team/security-tracker-service/blob/master/crontab/crontab

    What are the other triggers? Do you have some webhook to trigger an update when the git repository is updated? Is there some locking to avoid concurrent updates?

    Its improtant to note that you do not need exactly the same setup, you can do as well something with webhooks or other triggers. In our case we use additional mail triggered updates.

    The user is subscribed to the security announces lists and the commit mailinglists.

    If a DSA or DLA is released then a update-security is triggered.

    if a commit is done, then an update-list is triggered.

    (and processed by the cronjob).

    Concurrend updates are avoided by using a lock.

  • added 373 commits

    • 4f51c5f1...b63d0406 - 365 commits from branch security-tracker-team:master
    • 878a660f - Simplify DLAFile
    • a52388c7 - Merge DLAFile into DSAFile
    • 28499f92 - Move source list to a config file
    • 7cbb5d4d - Dynamically create announce queries
    • 70b31a29 - Add support for CUSTOMER bugs and CVE extends
    • de53ef7a - Simplify Extends support
    • 93c9bc5c - gen-DSA: allow other gen-* links
    • 32be9de6 - Rename CVECUSTOMERFile to CVEExtendFile

    Compare with previous version

  • added 6 commits

    • 0184f3b2 - Move source list to a config file
    • 8251dec7 - Dynamically create announce queries
    • 36571773 - Add support for CUSTOMER bugs and CVE extends
    • dbd95bfb - Simplify Extends support
    • 9002fdfe - gen-DSA: allow other gen-* links
    • 4dc4436a - Rename CVECUSTOMERFile to CVEExtendFile

    Compare with previous version

  • Updated the branch, this time with a real config.

    A config tree could be nice but changes to this file would be so infrequent that I didn't look at it yet. It could be done in a followup commit.

    I have cleaned up / squashed / reorder the commits. Some of the simplifications, and creating the config file are now happening before Bastian's "Add support for CUSTOMER bugs and CVE extends", that way that commit no longer hardcodes 'CUSTOMER-%' or the CVE/CUSTOMER etc. Adding those files to config.yaml would be up to the third party.

    Tested this by adding an additional CVEExtendFile and an additional DSAFile, with some fake data, then updating the DB with make all', then serving it make serve' and verifying that the pages are updated.

    I could squash the CVECUSTOMER -> CVEExtend rename, but in general I think this is ready to be merged.

  • Emilio Pozuelo Monfort
  • @pochu You might want to drop the WIP prefix if you think this is ready to be merged.

  • Ack, though Salvatore pointed out that this introduced a new dependency (yaml), and I then realised that we already depend on json so I could use it instead of yaml and avoid the new dependency. I will change that and then remove the WIP status.

  • @pochu started to look at your proposed changes. So far looks good so far, but I'm still going through, and have some questions.

    • Did you had a chance to look into moving the "configuration" into a json file so we can avoid adding yaml dependency?

    • Could you add as well a note on how to use the extends in doc/security-team.d.o/security_tracker (preferably I think as sort of appendix after the "Setting up a local testing instance" part). [addendum: or just another documentation file to document how to use use the extentions]

    • IMHO the limitations on the CVEExtend file should be documented as well. For instance it's not possible to add for a same CVE entries in data/CVE/list. So the main purpose of this file seems to be to add non-CVEified additional extends? Can you elaborate a bit on what is the intention on what should be supportable in the extends files?

    • Simlarly for the extends file, there is added support for TEMP-XXXX -XXXX identifier. Those are actually not very stable, so I wonder how much sense it would make to make the extends support it. In its current form a ha hash of the truncated description and a debian bug number if available. So assume there is a temporary entry, then it get a TEMP idenfier. But then someone reports to Debian bug to have an identifier, then the TEMP name changes (on pupose). Maybe in future we can add support for other identifiers, but so far the security trackers main key should remain a CVE to identify an issue, or as a fallback if thats not possible a Debian bug. Many TEMP entries which never got a CVE are propably as well disputable. Can you elaborate a bit on what should be supportable i nthose extends files and how it would be supposed to work?

    • gen-MYSA would need as well an according template in doc/MYSA.template and a data/mysa-needed.txt (or if that's not used then gen-DSA should handle missing data/*-needed.txt more gracefully). it works as it is right now, but trying it out without data/mysa-needed.list would error out a sed: can't read data/mysa-needed.txt: No such file or directory.

    Edited by Salvatore Bonaccorso
  • @pochu: just an example on what I mean with my issue iwth the CVEExtends. So i was under the impression you would like to support say adding in data/MY-CVEEXTENDS/list an entry like (assuming it is inteded say you support a release which is not anymore in Debian, the setup/fetching around is already in place and now you want to mark something as no-dsa for your case):

    CVE-2017-9951
            [jessie] - memcached <no-dsa> (Minor issue)

    but this CVE is already in data/CVE/list, then on building the database we get an error from data/MY-CVE-EXTENDS/list: 1: error: bug name CVE-2017-9951 is not unique. What do I miss?

    The "MYSA" part works without problem seen so far and the changes look okay.

  • Reading this, I can't help but think that we should be able to have multiple sources for CVE-related information, e.g. that CVE-2017-9951 could be in data/CVE/list but also data/CVE/jessie or, even better, data/CVE/CVE-2017-9951. The latter would open up the possibility to having one file per CVE entry, which would drastically reduce the load on the git data structures, because each CVE change would create a new or modify a small blob (instead of modifying a very large one like we're doing now).

    Of course, that's a major rearchitecture and would only be useful in the short term if we split the data/CVE/list file in multiple chunks and rebase the repository. But I think it could resolve some of the performance issues we're having with the repository right in the future as well...

    Just a thought...

  • added 336 commits

    • 4dc4436a...e76e94ca - 327 commits from branch security-tracker-team:master
    • 0b664197 - Simplify DLAFile
    • 375ba023 - Merge DLAFile into DSAFile
    • 0cb94dee - Move source list to a config file
    • dfcc9c12 - Dynamically create announce queries
    • 16d91b7d - Add support for CUSTOMER bugs and CVE extends
    • 028f59c7 - Simplify Extends support
    • 2bcf2883 - gen-DSA: allow other gen-* links
    • c1c3e525 - Rename CVECUSTOMERFile to CVEExtendFile
    • 06dc7209 - Document CVE extends support

    Compare with previous version

  • @carnil

    • Did you had a chance to look into moving the "configuration" into a json file so we can avoid adding yaml dependency?

    Done now! No extra dependencies, and removing the weird hack to find the config file.

    • Could you add as well a note on how to use the extends in doc/security-team.d.o/security_tracker (preferably I think as sort of appendix after the "Setting up a local testing instance" part). [addendum: or just another documentation file to document how to use use the extentions]

    Done!

    • IMHO the limitations on the CVEExtend file should be documented as well. For instance it's not possible to add for a same CVE entries in data/CVE/list. So the main purpose of this file seems to be to add non-CVEified additional extends? Can you elaborate a bit on what is the intention on what should be supportable in the extends files?

    Hopefully the documentation gives a little information. Mostly you can add information to existing CVEs. E.g. there is CVE-2018-0001 in CVE/list which affects python3.6, but you are still supporting python3.5 inside your organization, so you can add to CVE-EXTEND/list:

    CVE-2018-0001 - python3.5 [mydistro] - python3.5 (Can be fixed along with the next update)

    This way this CVE will appear on the security tracker for python3.5 with the relevant information, as well as the information in CVE/list

    • Simlarly for the extends file, there is added support for TEMP-XXXX -XXXX identifier. Those are actually not very stable, so I wonder how much sense it would make to make the extends support it. In its current form a ha hash of the truncated description and a debian bug number if available. So assume there is a temporary entry, then it get a TEMP idenfier. But then someone reports to Debian bug to have an identifier, then the TEMP name changes (on pupose). Maybe in future we can add support for other identifiers, but so far the security trackers main key should remain a CVE to identify an issue, or as a fallback if thats not possible a Debian bug. Many TEMP entries which never got a CVE are propably as well disputable. Can you elaborate a bit on what should be supportable i nthose extends files and how it would be supposed to work?

    The TEMP stuff was added by @waldi , I'm not sure how important it is. Maybe it wasn't there to reference TEMP-* issues in CVE/list, but to add your own new TEMP-* vulnerabilities (until you can get a CVE).

    • gen-MYSA would need as well an according template in doc/MYSA.template and a data/mysa-needed.txt (or if that's not used then gen-DSA should handle missing data/*-needed.txt more gracefully). it works as it is right now, but trying it out without data/mysa-needed.list would error out a sed: can't read data/mysa-needed.txt: No such file or directory.

    Yes, those need the templates if you are going to use gen-DSA (through a symlink). Otherwise you can add things manually.

    but this CVE is already in data/CVE/list, then on building the database we get an error from data/MY-CVE-EXTENDS/list: 1: error: bug name CVE-2017-9951 is not unique. What do I miss?

    Did you add the extend file to the config file as a CVEFile or as a CVEExtendFile ?

  • added 6 commits

    • 77190d32 - Dynamically create announce queries
    • ecbbab3e - Add support for CUSTOMER bugs and CVE extends
    • fafe4839 - Simplify Extends support
    • bf1f037a - gen-DSA: allow other gen-* links
    • b59cbe46 - Rename CVECUSTOMERFile to CVEExtendFile
    • a0c20580 - Document CVE extends support

    Compare with previous version

  • Emilio Pozuelo Monfort resolved all discussions

    resolved all discussions

  • Emilio Pozuelo Monfort unmarked as a Work In Progress

    unmarked as a Work In Progress

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading