Skip to content
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

New guide on container security best practices #156

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

riverma
Copy link
Collaborator

@riverma riverma commented May 7, 2024

Purpose

  • A best practice guide to help folks easily scan container (docker) related code repositories for vulnerabilities, automatically

Proposed Changes

  • [ADD] New best practice guide and associated files
  • [CHANGE] Yarn plugin added to help render code snippets

Issues

Testing

- MDX plugin to show snippets of code from external files
@riverma riverma self-assigned this May 7, 2024
@riverma riverma requested a review from a team May 7, 2024 19:13
@riverma riverma added most requested Highly requested by community members software lifecycle Process improvements involving developing, testing, integrating, deploying software labels May 7, 2024
Copy link

@nutjob4life nutjob4life left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor "here" hyperlink issue but otherwise looks great, reads great. And I learned about .mdx files!

@riverma
Copy link
Collaborator Author

riverma commented May 7, 2024

Minor "here" hyperlink issue but otherwise looks great, reads great. And I learned about .mdx files!

Thanks for reviewing this @nutjob4life! Much appreciated! Yeah - MDX is allowing these guides to get all fancy, with embedded code and additional features. Some interesting possibilities down-the-line!

Curious if the hyperlink issue you were seeing was related to this block or somewhere else?

@nutjob4life
Copy link

@riverma weird, my comment got dropped somehow.

Anyway, the issue is the hyperlinking of [here]. It's a pet peeve of mine. Hyperlinking the word "here" makes a tiny target (Section 508 issue) but also relatively free of context. Read more about it.

You can rework it by writing something like:

NOTE: you'll need a DockerHub account to run the `docker scout` tool.
Note that this command will compare a local scan's results with Docker's database.
[More information about Docker Scout is available](https://docs.docker.com/scout/quickstart/).

@riverma
Copy link
Collaborator Author

riverma commented May 8, 2024

Minor "here" hyperlink issue but otherwise looks great, reads great. And I learned about .mdx files!

@riverma weird, my comment got dropped somehow.

Anyway, the issue is the hyperlinking of [here]. It's a pet peeve of mine. Hyperlinking the word "here" makes a tiny target (Section 508 issue) but also relatively free of context. Read more about it.

You can rework it by writing something like:

NOTE: you'll need a DockerHub account to run the `docker scout` tool.
Note that this command will compare a local scan's results with Docker's database.
[More information about Docker Scout is available](https://docs.docker.com/scout/quickstart/).

Thanks for the clarification! Feedback incorporated 👍


# Container Security

<pre align="center">Comprehensive guide to scanning container images for security vulnerabilities using pre-commit hooks and automated repository scanning tools.</pre>
Copy link

@lewismc lewismc May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @riverma explicit mention of Docker in the title is probably necessary. Reasoning; there are many (OCI-compliant) alternatives to Docker which could also be scanned for security vulnerabilities.
That is unless you want to broaden the scope of this best practice outside of Docker. Just food for thought :)
Nice work.

Copy link
Collaborator Author

@riverma riverma May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @lewismc - great observation! I was wanting to keep this guide title general so that we could support content for the Docker alternatives as well down-the-line.

That being said (and we could get feedback from @NASA-AMMOS/slim-community here too) - which other containers should we support within this guide? By that I mean: which other container technologies are actually being used by your projects right now or will be in aspiration? True to the SLIM philosophy - we tend to make guides that are targeted towards solutions / technology by our community members, and as the community grows, we iterate and expand the scope. (CC @NASA-AMMOS/slim-community-member-leads)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Open Container Initiative (OCI) is a standard started and promoted by Docker, amongst others. Many on Lab are now using Podman, which reports to be OCI-compliant, in place of Docker. In fact, I believe it is mandated going forward on AWS for some teams. Here is more information on Podman origins.

Thinking off-the-grid, what about making it an OCI-compliant container security guide?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lewismc @jpl-jengelke - thought about your suggestions, discussed a bit with @lylebarner and ended up swapping to the grype toolkit, which is OGC compliant and not Docker specific. Moreover, it can scan non-containers as well.

yarn.lock Outdated Show resolved Hide resolved
@jpl-jengelke
Copy link
Contributor

jpl-jengelke commented May 9, 2024

I didn't engage a formal review, but added a number of comments. Hopefully they are helpful.

Also, I wanted to note there is no reason why we cannot have multiple container security guides, including a specific Docker container security guide.

@riverma
Copy link
Collaborator Author

riverma commented May 9, 2024

One suggestion from @ddalton-swe is to look at this tool (which is being utilized for some current projects): https://github.com/anchore/grype

@riverma
Copy link
Collaborator Author

riverma commented Jun 25, 2024

Thank you for the extensive review @jpl-jengelke . I’m going to try out an OCI complaint tool to support non-Docker containers, but if they are insufficient I’ll suggest with take @lewismc suggestion and make this a Docker specific guide for now and add in other scanning tools the community suggests for other container types later.

@riverma
Copy link
Collaborator Author

riverma commented Jul 3, 2024

@NASA-AMMOS/slim-community - I've made some updates to this PR to take into account feedback from @nutjob4life @jpl-jengelke @lewismc. Let me know if you have other thoughts!

See live rendering here: https://riverma.github.io/slim/docs/guides/software-lifecycle/security/dependency-vulnerability-scanning/

Copy link

@nutjob4life nutjob4life left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fabulous guide and I am looking forward to bringing Grype into my toolchain. Big time approval.

I did have some comments but they can largely be ignored.

One other comment: the image static/img/continuous-testing-image.png is included in the pull request but isn't referenced? Did I miss it?

Thanks for the guide!

grype dir:.
```

**⬇️ [.pre-commit-config.yml](.pre-commit-config.yml)**

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're going to call YAML files .yml, I will insist you call HTML files .htm 😈

Copy link
Collaborator Author

@riverma riverma Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hear you @nutjob4life! However, this is a "feature" of the pre-commit tool itself. It doesn't recognize yaml extensions by default, it is strictly looking for a .pre-commit.yml file in the local repository. There is a workaround (use the --config parameter), but its extra typing. See pre-commit/pre-commit#702

Copy link

@nutjob4life nutjob4life Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @riverma, unless my dyslexia is kicking in, I think that's reversed. The pre-commit tool looks specifically for .pre-commit-config.yaml (with the a).

The issue cited wanted support without the a (.yml) but the maintainers were all "nah bro".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nutjob4life - apologies! I keep making the mistake of citing things that actually contradict my argument 😂. I could’ve sworn I got an error message from pre-commit though. Let me do some digging and share feedback.

BTW what is your opinion on .jpg vs .jpeg? ;)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW what is your opinion on .jpg vs .jpeg? ;)

@riverma don't get me started 🤣

But I will leave you with this:

"GIF"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤯

@riverma
Copy link
Collaborator Author

riverma commented Jul 4, 2024

This is a fabulous guide and I am looking forward to bringing Grype into my toolchain. Big time approval.

I did have some comments but they can largely be ignored.

Thanks for the guide!

Thanks @nutjob4life - appreciate you taking the time and glad the guide is useful!

One other comment: the image static/img/continuous-testing-image.png is included in the pull request but isn't referenced? Did I miss it?

Looks like main wasn't merged fully in this branch. I've updated and changes like the above are no longer in the PR. Thanks for catching this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
most requested Highly requested by community members software lifecycle Process improvements involving developing, testing, integrating, deploying software
Projects
Status: 👀 In Review
Development

Successfully merging this pull request may close these issues.

None yet

4 participants