Kiwi icon indicating copy to clipboard operation
Kiwi copied to clipboard

We need an overall plan and automation around security and inspection of dependencies

Open atodorov opened this issue 5 years ago • 5 comments

ATM we are running Snyk, Coverity and Bandit against our own code base. Coverity & Bandit against dependencies too which produces lots of issues, some are false positives others may need fixing.

OTOH some of the components we depend on (e.g. Patternfly) do not perform any kind of security testing. Snyk regularly reports about issues introduced by 2nd level dependencies pulled by Patternfly.

Patternfly itself uses (or just benefits from npm reporting that) npm audit and this can be seen in their Travis CI logs. This reports multiple issues however they do not fail Travis.

For Kiwi TCMS we need to have a clear visibility from all tools used against our code base directly and then all tools used against 1st level dependencies and the issues reported there.

All of this should be nicely reported inside Kiwi TCMS of course!

Then we also need a matrix report for tools/issues used inside all repositories of 1st level dependencies so we know what their status is and that their upstream has mechanisms for early security warnings on their side.

atodorov avatar Mar 19 '19 22:03 atodorov

What about the GitHub dependencies function?

image

Prome88 avatar Apr 23 '20 11:04 Prome88

Is this issue still valid and lacking the same things since Mar 2019?

zahari avatar Sep 02 '21 05:09 zahari

Looking at the GitHub Actions that we currently have, we have 3 that are for security checks - npm audit, bandit, and bandit_site_packages. They can be seen here https://github.com/kiwitcms/Kiwi/blob/master/.github/workflows/security.yml.

npm audit and bandit_site_packages seem to be set to allow_failure: true, because they are failing on master branch ATM https://github.com/kiwitcms/Kiwi/actions/runs/3564349473/jobs/5988201366 https://github.com/kiwitcms/Kiwi/actions/runs/3564349473/jobs/5988202855

bandit is scanning only our code and is currently green (I suppose its enforced).

npm audit

On the latest run of this job it looks like it failed, because of a single moderate severity vulnerability in one of our dependencies - simplemde. The report says that there is no fix available ATM.

bandit_site_packages

The bandit scan of our dependencies is failing with much more vulnerabilities found

Total issues (by severity):
		Undefined: 0
		Low: 3242
		Medium: 301
		High: 36

Fixing all these upstream will take a lot of time. I think it's worth investigating how much of these are actually affecting us. For example, if there is a vulnerability in a function in one of our dependencies, but we are not using this function we are not really vulnerable.

I think that for both bandit and npm audit, even if we fix all vulnerabilities, we should not set the rules to allow_failure: false, because that would mean that a vulnerability in a dependency that does not have an available fix will prevent us from pushing code/merging PRs.

I think the best thing we can do is - fail the job if there are vulnerabilities with fixes, but not fail it if there are vulnerabilities without fixes.

CodeQL

The GitHub CodeQL checkers is also reporting some potential vulnerabilities in our code. These are currently privately reported and not visible to the world. I think we should focus on fixing/investigating those, because they are in our code and we have full control over that.

New Idea: Image Scanning

I think an important thing that we should be doing but currently are not is scanning the container images we produce. Since many customers run Kiwi TCMS as a container it is beneficial for us to also have this information, because that type of scanning can expose vulnerable Linux packages that we ship inside our container. (Also, many scanners are also aware of the type of application that is running inside the container (java, python, etc.) and can even detect the python packages installed and scan them for CVEs. this check will probably produce similar results like bandit, but it will still be useful to have both results and be able to compare them)

We could use an open-source scanner like Trivy or Grype and integrate it into our GH workflows.

asankov avatar Nov 29 '22 21:11 asankov

I run an image scan of kiwitcms/kiwi:latest via Trivy and these are the results:

kiwitcms/kiwi (redhat 9.0)
Total: 131 (UNKNOWN: 0, LOW: 34, MEDIUM: 93, HIGH: 4, CRITICAL: 0)

Node.js (node-pkg)
Total: 8 (UNKNOWN: 0, LOW: 1, MEDIUM: 6, HIGH: 1, CRITICAL: 0)

/Kiwi/ssl/localhost.key (secrets)
Total: 1 (UNKNOWN: 0, LOW: 0, MEDIUM: 0, HIGH: 1, CRITICAL: 0)
HIGH: AsymmetricPrivateKey (private-key)

asankov avatar Nov 29 '22 22:11 asankov

Looking at the GitHub Actions that we currently have, we have 3 that are for security checks

Looking at the GitHub Actions that we currently have, we have 3 that are for security checks - npm audit, bandit, and bandit_site_packages.

There are more:

  • Snyk
  • CoverityScan

Fixing all these upstream will take a lot of time. I think it's worth investigating how much of these are actually affecting us.

Indeed it will. And investigating every single one of them will probably take also a very long time. Hence my proposal for such issues is:

  1. Report them upstream first so that upstream is aware
  2. Maybe help upstream onboarding with the same tools that we use
  3. Make adjustment where necessary, e.g. don't scan test related code or patch upstream packages not to ship tests, etc. Which approach is more preferable will also depend on what upstream feels comfortable with.

For example, if there is a vulnerability in a function in one of our dependencies, but we are not using this function we are not really vulnerable

I don't think there is an easy way to know for sure which functions we do use and which ones we don't without rigorous testing and exploration. OTOH we may start using a previously unused feature in a library so a blanket approach may be best in the general case.

npm audit

That will be largely fixed as part of the JavaScript and UI refactoring that I've started working on.

SimpleMDE

We've got an issue to replace it instead of fixing it. The security vulnerability that has been reported is actually fixed on our side by overriding a method.

CodeQL

Yes, that's new. It has to wait a bit b/c most of these reports are in the JavaScript code base which is under heavy modification ATM. But I agree that we have to fix them.

atodorov avatar Nov 30 '22 05:11 atodorov