Ignored Extension Vulnerabilities admin and maintainers experience

Development
#1

Hello all,

For XWiki 15.6RC1, we plan to list the security vulnerabilities of all the known extensions (only the installed extensions were listed in 15.5RC1).
And, this leads to some vulnerabilities being listed:

From what we know, those dependencies are not dangerous when it comes to the code we control (xwiki-commons, xwiki-rendering, xwiki-platform, and some contrib extensions). And, we want to present them as such to admins, to avoid some irrelevant anxiety :slight_smile: Of course we can’t draw the same conclusions for code we don’t control.

Naming

I’m currently using ignore or “false positive” to designate vulnerabilities we are aware of and consider as not dangerous.
I propose to use “analyzed extensions” as the default vocabulary (both for our source code and for UIs labels).

I’m currently using “Servlet” for the extensions that are not provided by the packaged XWiki instance or an installed extension.

Admin Experience

Some screenshots of my current PoC, to give you an idea of where I’m currently heading.

Screenshot 2023-07-11 at 16-12-58 Global Administration - XWiki

Screenshot 2023-07-11 at 16-13-11 Global Administration - XWiki

Screenshot 2023-07-11 at 16-13-33 Global Administration - XWiki

Screenshot 2023-07-11 at 16-22-20 Global Administration - XWiki

The extension vulnerabilities tab lists all extensions with at least one extension that is not marked as “analyzed”.
The ignored vulnerability (which should be renamed “analyzed vulnerabilities”) tab lists all extensions with only “analyzed” vulnerabilities.
The servlet vulnerabilities tab lists all extensions provided by the servlet engine, regardless of their “analyzed” status.

Note that “analyzed” vulnerabilities are visually distinguished by their smaller font size and the button on their right, allowing to access a modal screen providing the details of the analysis (fourth screenshot above).

Maintainer Experience

Maintainers are in charge of keeping up to date the analysis status of the vulnerabilities.
For now, I’m planning to expose the list of analyzed vulnerabilities using a json object (see below for an example), provided by a page in extension.xwiki.org (e.g., https://extensions.xwiki.org/xwiki/bin/view/Extension/Extension/Security/Code/AnalyzedVulnerabilities/), which would be only editable by members of XWikiCommittersGroup.

Currently, extensions are considered as:

  • vulnerable if no analyzed vulnerability can be found for at least one of its known vulnerabilities
  • “analyzed” otherwise (i.e., all its known vulnerabilities can be found)

Some remarks:

  • We’ll need to commit to maintain this list (and be reactive), this also mean being able to provide clear explanations of each analysis. This can imo be time consuming, and we’ll probably need to work as a group to make the first explanations clear to have good examples to follow in the future, and to give a first good impression to the first users of the feature.
  • The current design currently does not take into account the case where some extensions identify a vulnerability as impacting it while it was marked as analyzed by some other extensions. We can consider a few options:
    • remove the entry and all the explanations in the analyzed vulnerabilities source, but in this case all the existing explanations are lost
    • provide a way to explicit that an given extension is vulnerable, but this means providing the ability to bind an analyzed vulnerability to a given extension (including the version range where it is impacted)
    • ask the vulnerable extension to create an adhoc security advisory, which would itself reference the “analyzed” vulnerability, and explaining how to exploit the vulnerability in its own context
  • As for the source of vulnerabilities, admins will be able to point to their own source of analyzed vulnerabilities. Do we want to provide a way to allow maintainers to “inherit” from the main source of analyzed vulnerabilities (i.e., the one from e.x.o)?

Note: in a first step I propose to maintain the source of analyzed vulnerabilities as a static json (possibly with some velocity to avoid some explanations duplication), and to provide a way to declare analyzed vulnerabilities as XObjects on e.x.o in a second time

{
  "analyzedVulnerabilitiesMap": {
    "GHSA-2q8x-2p7f-574v": [
      {
        "source": "xwiki-platform",
        "explanation": "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Phasellus non pellentesque sem. Maecenas ultricies, nisi quis efficitur consectetur, libero enim blandit justo, at auctor nisi arcu congue odio. "
      },
      {
        "source": "some-extension",
        "explanation": "ivamus eget condimentum elit, in blandit turpis. Nullam suscipit eros vitae justo suscipit aliquam. Curabitur at nibh id elit eleifend condimentum eu a urna. Vivamus vel molestie nunc. Suspendisse porta porta quam, vel pulvinar magna vulputate a. In porta tincidunt dui. Cras tortor ante, interdum eget enim quis, posuere varius ligula."
      }
    ],
    "GHSA-3ccq-5vw3-2p6x": [
      {
        "source": "xwiki-platform",
        "explanation": "ivamus eget condimentum elit, in blandit turpis. Nullam suscipit eros vitae justo suscipit aliquam. Curabitur at nibh id elit eleifend condimentum eu a urna. Vivamus vel molestie nunc. Suspendisse porta porta quam, vel pulvinar magna vulputate a. In porta tincidunt dui. Cras tortor ante, interdum eget enim quis, posuere varius ligula."
      }
    ]
}

Let me know what you think.
Thanks,
Manuel

Security Policy Amendment to analyze vulnerabilities in dependencies
#2

Thanks for working on this!

Just small notes on the UI side: in the modal “xwiki-platform” and “some extension” is not really explicit. Might be better to have something like “Impacted module: xwiki-platform”. Also I’m wondering if it would make sense/if it’s easy to provide pills in the tab title with the total number of entries in the LD, so that admin quickly knows that there’s something to check.
Finally, the modal should probably always contain the same kind of warning you display on top of the LD, but as an introduction text for the analysis, since I’m pretty sure the warning on top of the modal might be discovered after opening the modal analysis…

This should be probably a new amendment of our Security Policy I think. I can try to make a draft of a proposal for it.

Sounds like a UC that could happen pretty fast.

And it’s probably not good: if Extension1 is impacted but not Extension2 you will mark all wikis having only Extension2 as impacted while they’re not.

Probably expensive but on the long run we certainly need to go there

I guess it’s an ok-ish short term option, but doing option 2 is probably easier now than later…

Well, on the long run it’s probably best to allow it, but I don’t think it’s a priority at all. This view is for Admins, so it’s already admins that will specify the main source of vulnerabilities: if they do that they’re already aware of the main source and can copy/paste or use scripts to get the data when needed. Clearly it’s already a very advanced use case to me.

Sounds good, especially since the schema might be unstable for a while.

#3

While writing Security Policy Amendment to analyze vulnerabilities in dependencies I realized that it could actually be useful to be able to provide an analysis of a vulnerability without considering it as ignored.
For example: a new vulnerability is found on an extension wildly used and we’re starting to get lots of requests about it but there’s no easy upgrade possible right away, and the impact is low. In such case, we shouldn’t ignore the vulnerability, but we should provide information that maybe a workaround is possible while waiting for the upgrade because it will take weeks to get it.

1 Like
#4

+1

Note sure to understand that part.

+1

I agree.

#5

I didn’t use the right word it seems, was thinking about badges in the tab title like shown in Components · Bootstrap

#6

ok, got it, some extra work but not definitely not difficult to implement.

#7

+1 to use “Analyzed” or maybe better “Reviewed” (this seems more use-friendly to me).

The reason why “Ignored” is not good to me is because we could be impacted by a vulnerability found in, say, one of Solr’s deps but ATM we cannot upgrade Solr for XYZ reason. So “Reviewed” seems better as it doesn’t imply we’re ignoring it, just that we’re blocked ATM and users/admins should know about it.

I don’t like “Servlet” too much since that has a specific meaning and the issue is not in Servlets but in Application Servers or Servlet Containers (see Web container - Wikipedia). So I’d use either “Servlet Container” or “Server” or “Application Server” or more generally “Environment”.

I think it would be good to have our build (and CI) fail when there’s an existing vulnerability that has not been reviewed. This could be achieved by a specific Maven plugin or by having a functional UI test checking that there’s no entry in the “Extension Vulnerabilities” tab. That would force us to address this topic and not do it as a best effort.

I don’t understand this. Since the JSON file you mention is global to all extensions and there’s a single file (ie the files are not located in the git repo of each extension, which could be interesting to have it under version control and close to the extension, with an exo page doing the aggregation), we can write whatever we want in that file, in the explanation field. If extension X is impacted, it can be written there.

Note that “analyzed” (I prefer “reviewed”) doesn’t mean it’s not a problem; it just means it’s been reviewing and either it’s not impacting XWiki or we cannot fix it ATM for some reason.

I’m probably missing something :slight_smile:

Yes, would be interesting to have.

I don’t understand what you mean by “source”. Does it mean the “impacted extension id”? The example shows xwiki-platform but that doesn’t exist as an extension id…

Thanks!

#8

+1

+1, as I mentioned in another chat “Servlet” is too technical, and, in the future, there are plenty of other environment security problems we could mention in the same tab which have nothing to do with the applicatiobn server (vulnerabilities in the Java version, EOF Linux distribution, etc.)

#9

I tried to add more details on the design page for UC103.

The things is, while the CVE is mapped to one or many extensionsId, extensions have dependencies.
And a given CVE can be safe for a whole set of extensions, but not one some specific cases.
So I think we need to be able to express that we reviewed a CVE globally for all of XS (hence the “xwiki-platform” used in the example, because it would be too difficult to exhaustively list all the extensions id), but not for some contrib extension.

For instance:

{
  "reviewedVulnerabilities": {
    "cve-id-123": [
       {
         "explanation": "This CVE is safe because XXX",
         "scope": "xwiki-platform"
         "impact": "safe",
       }, {
         "explanation": "This CVE is NOT safe because YYYY",
         "scope": "org.xwiki.contrib:my-app"
         "impact": "unsafe",
       }
    ]
  }
}

+1 for environment

This is UC106 :slight_smile:
We’ll need to discussion if we also want to activate it for contrib extensions when starting to work on that one.

#10

+1 for Reviewed (that’s what I used in the design page)

#11

In general +1.

I think it would be good if we considered right from the start that we might need to support multiple versions of this JSON file format for different versions of XWiki. We should at least express this in the URL that is requested by the client, which should also not depend on the specific chosen storage.

I’m not sure what to do regarding the way the data is generated. As long as we only have a few items, a manually maintained JSON file could be okay but we should make sure we don’t miss migrating to something more maintainable before it becomes too much work to migrate.

#12

+1, not sure to understand the last part of your sentence regarding storage though.

I think that part should be planned for the next roadmap or the one after.
I think this is good to have a small period of time where we maintain the json manually to get a better grasp of what’s really needed.
And then, move toward something more automated and reified as code in e.x.o.

#13

I mean don’t put the path to where we store the static JSON in the URL so we can change the fact that this is static JSON later.