-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Improve Best Practice Guide]: Security scanning with SCRUB, et. al. #25
Comments
@jpl-jengelke - as @awdtinio mentioned, please chat with @jeffreypon as he's experimented with SCRUB. Also - I think the security scanning with SCRUB ticket should really be delivered as a Jenkins plugin for people to quickly / easily adapt to their existing projects. Having a plug-and-play plugin would likely reduce the amount of time developers would have to spend getting SCRUB scanning initialized for their own CI. Take a look at this guide: https://www.jenkins.io/doc/developer/tutorial/create/ Thoughts? |
There's a Maven plugin that will integrate with the CodeSonar system, but of course Maven is mostly Java-specific. Developing a Jenkins plugin is a big task, so I don't think by any measure that it's doable with the resources at our disposal. However, we can offer a demonstration that is configurable and plug-and-play enough to cover all bases. That is what I am working on completing. |
+1'd by @ramesh-maddegoda, @jeffreypon, @pymonger |
@riverma FYI, @lylebarner helped us get this integrated into our CodeQL execution using GitHub Actions. here is an example: https://github.com/NASA-PDS/validate/blob/main/.github/workflows/codeql-analysis.yml#L65 |
Hi @jordanpadams - thanks for pointing us to @lylebarner and the GH action you all have built for validate. This is pretty cool. @jpl-jengelke - let's definitely work together and discuss with @jordanpadams and @lylebarner on synergizing their existing code with the goals of this ticket as we work on this during Q2. |
AMMOS currently setup to use Codesonar and sonarqube. Scrub outputs a .csv report in a specific format that the cybersecurity team can ingest into their VATT tool to perform adjudication. @lylebarner If new tools are added to scrub (i.e. CodeQL), does the scrub tool output the .csv results in the specific format that can be imported into the cybersecurity's VATT tool without any software changes? Or does scrub have to be modified for each tool to get it in the specific format? |
@jpl-jengelke Make sure to grab the latest version of scrub. There was a major update this past week that changed some of the .csv output formats. The cybersecurity team is requesting all scans be performed using the latest version of scrub now. |
|
There's another ticket we should probably merge into this one: #51 |
Some ideas discussed with @jpl-jengelke for this ticket:
|
Proposed implementation
flowchart LR
subgraph CI["CI Server (Exa.: Java)"]
C1[Compile]-->C2[Test]
C2-->C3[Package]
C3-->C4["Publish\n(Artifactory)"]
C2-.-> |"Success, Monthly"| C5[SCRUB]
end
subgraph GA["GitHub Action (Exa.: Python)"]
A1[Compile]-->A2[Test]
A2-->A3[Package]
A3-->A4[SCRUB]
A4-->A5["Release\n(PyPi)"]
end
|
Thanks for the write-up diagram @jpl-jengelke! Some questions:
|
@lylebarner - had a few questions for you:
|
There is an institutional Secure Coding Guideline that can be used for this. Generally this is considered a good starting place for identifying high priority security issues. This guide can be used to tailor down to a smaller set of CWEs. I can provide a link via email.
Florin Tudor has set up several other repos with SCRUB-based scanning on Jenkins. He should be able to provide you with more information or at least point you in the right direction. |
Timing. We can't add anything to extend the build times for VICAR products since the timeline is so unpredictable in my experience. Potentially this could eat 30+ minutes, and that would not meet users' level of service expectations. In current planning we wanted this to occur once a month, maybe quarterly, at most, just to avoid long build times. Now for a POC in SLIM we could do that, but we wouldn't want someone implementing it in a VICAR product right now. |
Thanks @lylebarner I'll check in with Florin about it. |
@jpl-jengelke would it be possible to perform SCRUB analysis in parallel with test activities? From a technical point of view, SCRUB only needs the source code itself (and compiled outputs such as .class/.jar files) to perform analysis. Not sure of the system resources that are required to run the test suite though. |
Yes, it is possible, though some accommodations must be made. One point of the test phase in the Maven lifecycle is to provide an exit point if tests fail, before packaging. So the SCRUB operations would be run against The tasks would be run in containers to provide a cleanroom environment that isolates them from the CI server setup. Another option is to run in a parallel job but that would likely require a second compilation.
Costs versus value. During a release cycle, code should be turned around fast. If we execute SCRUB at the test stage then potentially tens of minutes are added to what must be a fast-turnaround build-compile-release cycle. The artifact is not generated until the test cycle completes (not sure if I'd add a pass/fail test). There are options but for a process that generates a report intermittently, the accommodation is to run it at the end of packaging and delivery. I think we can more readily apply this to the testing stage for a Python deliverable that is completely generated and released using GitHub Actions. (Note: (rough definitions) packaging == convert
Part of the charge of this project is to make tools that support Lab processes and requirements. Because CodeQL is the successor to Semmle, and the Lab security reports are predicated on Semmle, we do need to support it. Otherwise, these are soft recommendations based on conventions (as understood) that we can re-evaluate with trade studies. For instance, SonarQube is a very Java-centric tool that is highly adopted and more critically implements an interface that combines a number of underlying code analytic tools, such as PMD, Jacoco, etc. |
Note, preliminarily modified design flow (above) to accommodate comments after our meeting. I'll be sending out a follow up invitation today. Thanks everyone! |
Thanks for the feedback and responses @jpl-jengelke! I think @lylebarner's thoughts on a potentially running scans in parallel to builds is really interesting. Perhaps it's better to run scans nightly / on a schedule as opposed to tying to build triggers? The length of the code scan issue you mentioned is the same problem that comes up often for running system regression tests - which can take hours. Thoughts? |
@jpl-jengelke: recommendation next steps for this ticket (two separate PR's):
|
See @lylebarner's ticket over at SCRUB to help with (ii): nasa/scrub#99 |
@jpl-jengelke - I think we have (1) from @lylebarner write-up - we can point to this. Do you have a working, generic, solution to document for (2)? Having the two in place - we should be able to close this ticket soon. |
Yes I do. I will publish it shortly within the Java Starter Kit. That will close a number of tickets simultaneously. |
That sounds great - thanks @jpl-jengelke! |
NASA-AMMOS/slim#25: Functional GitHub Actions-based SCRUB (CodeQL) analysis...
…r indentation, add registry entry. More to come ...
… there will be a semi-minor rewrite to clarify build requirements in the context of multiple languages. ...
…r indentation, add registry entry. More to come ...
… there will be a semi-minor rewrite to clarify build requirements in the context of multiple languages. ...
Checked for duplicates
Yes - I've already checked
Best Practice Guide
Continuous Testing
Best Practice Guide Sections
Describe the improvement
We'd like to integrate NASA's SCRUB into a continuous build system to make it easy for projects to automate security scanning, per NASA recommendations.
The text was updated successfully, but these errors were encountered: