PM: better no-dsa support in PTS

Project Management

Related Issues/Documents

See #2 (closed) and #3 (closed).

Additional Instructions

@carlespina let's use this issue to keep track of the progress being made.

added Project management label

Thanks all, this is really great. I plan to work on this later this week. I'll comment on this issue for significant progress, questions and mockups. If anyone wants any information at any time just get in touch indeed.

Progress report:

Details of nodsa: For the nodsa information in distro-tracker (this is a screenshot from https://tracker.debian.org/pkg/linux in buster):

In https://salsa.debian.org/carlespina/distro-tracker/-/tree/better-no-dsa-support we have (see the parenthesis after the CVE):

Or for bind9:

Last one, for bird:

The link goes to https://security-team.debian.org/security_tracker.html#issues-not-warranting-a-security-advisory but perhaps this information should be included there. Comments are welcome! (or adding a badge with a tooltip, etc. I think that a new panel wasn't the the ideal idea). For now I kept it simple.

next_point_release: I've made the following changes in security-tracker: it reads the data/next-point-update.txt, save them into a new sqlite3 table (during update-db), returns them in the JSON output (see this commit message for the format: carlespina/security-tracker@9c371431 ) (all in this branch, probably I'll squash before making a PR: https://salsa.debian.org/carlespina/security-tracker/-/commits/next-point-update/)

On the distro-tracker side of things: at the moment is ignoring issues in the release that has next_point_update as true. An example, for the package https://tracker.debian.org/pkg/nova:

CVE-2019-14433 is in https://salsa.debian.org/security-tracker-team/security-tracker/raw/master/data/next-point-update.txt , so in the new branch:

Another example is pdns: it has two issues, both in next-point-update, the section disappears.

If things look more or less fine I think that a missing thing might be a functional test for the nodsa information and I also need to test the next-oldstable-point-update.txt (I've just written the code but not properly tested).

If you have any quick comment besides here I also joined #debian-qa (cpina).

@carnil @jmm @seb please review if my suggestions below matches your desires.

I have a few comments:

You probably want to change the grouping of all the CVE. Have a section "X issues left for the package maintainer to handle" where you list all the CVE that are <nodsa> (i.e. not yet clarified as <ignored> or <postponed>) and those that are <postponed>. Then for each CVE, you indicate whether it needs to be triaged (for plain nodsa) or whether it needs to be fixed through a stable update (for postponed CVE). And here there should be a link to some place explaining how you triage and indicate that it's fixed through next point release

Then I would propose two supplementary sections that are only shown when there's something else to show, i.e. they are displayed when the action item exists but they would not trigger the creation of the action item:

• "Y issues that will be fixed with the next stable update": you list the issues from next-point-release
• "Z ignored issues": you list only the <ignored> issues

(Those changes get rid of the label "X issues skipped by the security team" which was part of the problem)

Note that we could possibly split the "X issues left for the package maintainer to handle" in two for more clarity but in truth when the package maintainer looks at it, he will never first requalify a CVE as postponed before deciding to fix it so that's why I grouped them.

I also think that removing "issues skipped [...]" is the way to go, and the dynamic sections described by @hertzog achieve that. @jmm, what do you think?

Agreed, suggestion by @hertzog sounds good to me! We can replace the "Please fix them" with a link to somewhere on security-team.debian.org where we detail the process. That's probably easier than having it all in the PTS (and makes it easiert to finetune the docs depending on later feedback). I can write something up in the next days.

AFAICT I've done the changes that we discussed. I'm adding the same screenshots as the other day with, I'm sure that some text strings need to be changed. If other changes are wanted (that I might have missed/miss-understood or that you realise now that would help) let me know, absolutely no problem.

Ignored issues (linux package, buster, at the bottom):

Postponed issue (bind9, stretch):

Needs triaging (bird, stretch):

Next point release example and needs triaging (nova, buster):

I've assumed that next_point_update issues might or might not be nodsa: they appear in two different lists (with a sentence like "X nodsa issue that will be fixed with the next stable update" or "X issue that will be fixed with the next stable update").

The next point release or ignored don't trigger the creation of the action item.

I need to push more code changes to Salsa, also the CI failed on salsa. But feel free to send any comments that you might have, or where links should point to, text changes or list grouping changes for anything that I might have missed or makes sense.

We really need to get rid of the "X ignored security issues in Y" in the title of the action item too. Maybe rename it to "X low-priority security issues in Y"? @jmm how does that sound? Or shall we just always show "X open security issues in Y" no matter whether we have only nodsa issues or not?

I've assumed that next_point_update issues might or might not be nodsa: they appear in two different lists (with a sentence like "X nodsa issue that will be fixed with the next stable update" or "X issue that will be fixed with the next stable update").

I don't think that such a split makes any sense. Also "nodsa" is really jargon of the security team that I'd rather not expose in plain english here.

(needs triaging)

The "(needs triaging)" link and the text after the list have the same goal, point the user towards a longer explanation. Do we want to keep that kind of duplication?

We really need to get rid of the "X ignored security issues in Y"

I really like removing this sentence - I'll wait for opinions on the alternatives that you've suggested.

I don't think that such a split makes any sense. Also "nodsa" is really jargon of the security team that I'd rather not expose in plain english here.

I'll remove this splitting. If a CVE is "next_point_update" it will appear in the "X issues will be fixed with the next stable update" regardless of the nodsa field. I'll probably do this (and any other changes) on Friday.

The "(needs triaging)" link and the text after the list have the same goal, point the user towards a longer explanation. Do we want to keep that kind of duplication?

I feel that it seems cleaner to have only the link after the text (too many links in the enumeration, and many pointing to the same address).

Regarding Raphäel suggestion about "We really need to get rid of the "X ignored security issues in Y" in the title of the action item too. Maybe rename it to "X low-priority security issues in Y"? @jmm how does that sound? Or shall we just always show "X open security issues in Y" no matter whether we have only nodsa issues or not?"

@jmm : would you like that I do new screenshots for this? (I don't want to spam all the thread with screenshots). Once I know this I might take new screenshots for the closer-to-the-final ones :-)

I think that I changed the other texts but I'll double check once we confirm this sentence.

Thanks!

@jmm / @hertzog / @seb : I thought of no harm if I create a merge request to move forward with code review: qa/distro-tracker!90 (merged) (update: flake8 and unit test failed in Salsa, I'll work on this tomorrow)

I can add, any time (before the merge, after the merge, after deployment), the link that we had discussed (#4 (comment 214861)).

Regarding the sentence change mentioned in #4 (comment 215214) : I changed it from "X ignored security issues in Y" to "X low-priority security issues in Y" for now. I can change the grouping if we want it.

Also, I can add screenshots in this thread. I don't want to flood it with too many of them but if it helps it's all good.

I guess that when the code changes are deployed there might be some more feedback (and small changes).

nice work! two minor comments:

"1 issue left for the maintainer to handle" and then the issue says "postponed; fixed through a stable update" -> I think this should be "postponed; to be fixed through a stable update"

similarily "1 nodsa that will be fixed with the next stable update" which I believe should be "'1 nodsa that should be fixed with the next stable update"

I've changed the texts, thanks!

I agree that with the new grouping of CVEs that all of you came up with (the task in general) it looks better organised, good job!

To keep the momentum, start getting feedback on that side, etc.: I've created a merge request for the security-tracker: security-tracker-team/security-tracker!77 (merged)

mentioned in merge request qa/distro-tracker!90 (merged)

I have merged and deployed @carlespina work on tracker.debian.org. @carnil @jmm can you see if you are happy with the result?

In particular, we are directing maintainers with issues to triage or handle in https://security-team.debian.org/security_tracker.html#issues-not-warranting-a-security-advisory but you seemed to be eager to provide a better place. Let us know if you have another URL and/or if you want to update some text.

Hi

Only had partially time yet to fully go through, but many many thanks for your work @carlespina!

It looks good and I very much like it how it is implemented, and think there is not need to reference for now another place for an explanation on no-dsa and their substates ignored and postponed. As said I was not able to fully go through.

But I noticed a glitch/issue while browsing trough some source packages on tracker.d.o, not sure where the bug exactly lies:

In https://tracker.debian.org/pkg/qemu for example, for some entries (the ones which do not have a CVE description yet?) the listing seems garbled mangled with json input:

in the '34 security issues in buster' section listing the buster issues, it looks like:

[...]

25 issues left for the package maintainer to handle:

    CVE-2019-12067: (postponed; to be fixed through a stable update) {'description': '', 'nodsa': 'Minor issue, revisit when fixed upstream', 'nodsa_reason': 'postponed', 'fixed_via_stable_update': True}
    CVE-2020-13253: (postponed; to be fixed through a stable update) sd_wp_addr in hw/sd/sd.c in QEMU 4.2.0 uses an unvalidated address, which leads to an out-of-bounds read during sdhci_write() operations. A guest OS user can crash the QEMU process.
    CVE-2020-14394: (postponed; to be fixed through a stable update) {'description': '', 'nodsa': 'Minor issue', 'nodsa_reason': 'postponed', 'fixed_via_stable_update': True}
    CVE-2020-15469: (postponed; to be fixed through a stable update) In QEMU 4.2.0, a MemoryRegionOps object may lack read/write callback methods, leading to a NULL pointer dereference.
    CVE-2020-15859: (postponed; to be fixed through a stable update) QEMU 4.2.0 has a use-after-free in hw/net/e1000e_core.c because a guest OS user can trigger an e1000e packet with the data's address set to the e1000e's MMIO address. 

To me it looks that those which do not have yet a description (which can be possible) instead it get attached the json fragments.

Is this correct?

Oops, this is really bad. Thanks for spotting it!

On the good side: I can reproduce it in my local environment and I'll look into this probably tonight or tomorrow the latest.

A fix in: qa/distro-tracker!91 (merged)

One word to define the bug: embarrassing.

@hertzog : I could have fixed the bug with a small change in the template but I preferred instead to make all the issue_information be the same type: a dictionary with description and optionally the other keys.

no, the word is hindsight, and in hindsight everything is easy, obvious and/or embarrassing.

and in real life I'd like to say: thank you very much for this very useful improvement in the first place, and 2nd, thank you too, for fixing up the little (or not so) glitches! :)

Thanks @carlespina, merged and deployed.

@holger : the "embarrassing" word was an internal joke between me and myself :-) (I had been bitten by a similar thing with a |default or linebreaksbr Django template). I agree with what you said.

Fun fact I didn't really like that issue_information could be a str or dictionary and it crossed my mind that it might bite me/us later. I can't remember why I didn't make the small change to consolidate it all to a dictionary in the first instance. It's good that we saw it was broken when it was recent code (easier to find the fix). On the positive side: I don't remember any other code that I wrote that I had a feeling that was going to bite me.

@hertzog : thanks for the quick merge and deployment!

Thanks very much @hertzog for merging and deploying it last Friday. It was a good start to the weekend!

I haven't seen any problems in mailing lists / #irc. As a reviewer of the task, is @seb happy enough as well?

It looks good to me, but I'll wait to hear from @carnil and @jmm as well.

It looks good to me, I tested a few packages and it all looks fine.

We should replace the link to https://security-team.debian.org/security_tracker.html#issues-not-warranting-a-security-advisory with something more specific. I've pushed a first WIP, will update this issue when ready.

mentioned in commit 326b8785

mentioned in merge request !4 (merged)

The issue is now completed: !4 (merged)

I just want to say thank you everybody (no specific order, all participants in this issue @jmm, @hertzog, @holger, @carnil, @seb for all your help, timely feedback, code review, making me feel part of the team, etc. I know that it's not necessarily easy to get a new person onboard.

It has been a very good experience and I'll stick around (and if any issues appeared related to this do not hesitate to ping me!). I actually have a Debian Stretch that I cannot update yet so I really appreciate Debian LTS :-)

I really enjoyed contributing to Debian using Django (and Python for the security-tracker) on this issue/mini project.

mentioned in commit 57b70d77

closed with merge request !4 (merged)

closed with commit 57b70d77

mentioned in commit 4c915243