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

Cyt 814 docker scout #193

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

Conversation

KendallHarterAtWork
Copy link
Collaborator

Summary

If merged this pull request will add support for running Docker Scout on Docker .tar and .tar.gz files

Copy link
Collaborator

@nightlark nightlark left a comment

Choose a reason for hiding this comment

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

Let's rename docker_file.py to docker_image.py to clarify that it is getting info from a docker image rather than a Dockerfile -- would be neat to get the original Dockerfile for an image, that would be like getting access to source code.

surfactant/filetypeid/id_magic.py Outdated Show resolved Hide resolved
surfactant/filetypeid/id_magic.py Outdated Show resolved Hide resolved
surfactant/filetypeid/id_magic.py Outdated Show resolved Hide resolved
surfactant/infoextractors/docker_file.py Outdated Show resolved Hide resolved
surfactant/infoextractors/docker_file.py Outdated Show resolved Hide resolved


# Function that extract_docker_info delegates to to actually run Docker scout
def run_docker(filename: str) -> object:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def run_docker(filename: str) -> object:
def run_docker_scout(filename: str) -> object:

Docker Scout is a plugin for Docker (that may not be installed by default), so let's clarify that this is running it rather than one of the commands built into docker.

# Function that extract_docker_info delegates to to actually run Docker scout
def run_docker(filename: str) -> object:
result = subprocess.run(
["docker", "scout", "sbom", "--format", "spdx", f"fs://{filename}"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

It could be interesting to also capture the results from running docker scout cves, and either add as another entry in the returned dictionary, or convert to openvex files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

docker scout cves requires being logged into Docker's website and may also require uploading the docker image (the documentation isn't very clear on this - it kind of just assumes you're using remote images)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at the docs, it seems to have it run on something not uploaded to DockerHub requires adding a prefix -- archive:// would probably be it for the docker tar files we are looking at.

If it gets run on a local file, does it still require being logged into Docker? Maybe this is something that would need to be a user configurable setting that defaults to off -- not really sure why it would need logging in though, other than if Docker is maintaining their own CVE/advisory database instead of using the public databases.

Comment on lines 274 to 275
# TODO: Is there a better way of doing this?
pm.register(docker_file)
Copy link
Collaborator

Choose a reason for hiding this comment

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

For "internal plugins", the import of the file and registering can be done in: https://github.com/LLNL/Surfactant/blob/main/surfactant/plugin/manager.py#L13

surfactant/cmd/generate.py Outdated Show resolved Hide resolved

def supports_file(filetype: str) -> bool:
return filetype in ("Docker archive tar", "Docker archive gzip")

Copy link
Collaborator

@nightlark nightlark May 10, 2024

Choose a reason for hiding this comment

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

Suggested change
def is_docker_scout_installed():
# Check that Docker Scout can be run unless it's disabled
result = subprocess.run(["docker", "scout", "--help"], capture_output=True, check=False)
if result.returncode != 0:
logger.warning(
"Install Docker Scout to scan containers for additional information"
)
# Check if Docker Scout is installed when this Python module gets loaded
disable_docker_scout = not is_docker_scout_installed()

The above should result in the check for Docker Scout happening just once when this module gets loaded (extract_docker_info() can check disable_docker_scout before calling run_docker_scout).

Another idea that came to mind is adding a "check_plugin_prereqs" hookspec that would let plugins that have choose to implement it do some checks when they first get loaded. That might be a more elegant solution, coupled with a future user settings for plugins feature.


I think in terms of UX the lack of Docker Scout shouldn't be a hard failure that requires disabling -- there are likely to be plenty of samples people analyze that don't have any Docker images. Ideally we'd know ahead of time that one of the files they'll be analyzing is a Docker image, and then warn them... a more realistic alternative to omniscience could be printing the warning in extract_docker_info() if/when a Docker file is encountered.


def is_docker_scout_installed():
# Check that Docker Scout can be run
result = subprocess.run(["docker", "scout", "--help"], capture_output=True, check=False)
Copy link
Collaborator

@nightlark nightlark May 24, 2024

Choose a reason for hiding this comment

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

From the CI tests, it looks like subprocess.run can sometimes throw a FileNotFoundError that should be caught (are there any other error types that can be thrown?) if the docker command isn't installed on a system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants