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

Official Docker images #92

Closed
wants to merge 3 commits into from
Closed

Conversation

sk4la
Copy link

@sk4la sk4la commented Oct 18, 2019

Added Dockerfile(s) for vol.py, volshell.py and development/pdbparse-to-json.py.

@ikelos
Copy link
Member

ikelos commented Oct 18, 2019

Thanks for these @sk4la we'll hopefully get a chance to review them soon (we're just getting our PR handling policy in place internally), but one point I'd like to make is that pdbparse-to-json is an external tool that was used during development and wasn't really intended for mainstream use. We'd recommend that you call volatility/framework/symbols/windows/pdbconv.py if you want a standalone tool to convert pdb files. That uses the same internals that volatility uses itself for generating json data from pdbs, and will see updates in the future (which is unlikely to happen with things in development). Things in there are more for posterity and interest than much else... 5;)

@sk4la
Copy link
Author

sk4la commented Oct 20, 2019

Hey @ikelos, thanks for the heads up! Seems like it'd be beneficial to add a paragraph about this in the documentation. I'll fix the images as soon as I can.

@ikelos
Copy link
Member

ikelos commented Oct 20, 2019

Hiya, did you want something about not using files in the development directory? We've already got this: https://volatility3.readthedocs.io/en/latest/symbol-tables.html#windows-symbol-tables which mentions which tool to use for manual generation, but it can be reformatted if it's unclear?

@sk4la
Copy link
Author

sk4la commented Oct 20, 2019

I just pushed the changes.

@ikelos : I may have missed this part, nevermind then :)

@ikelos
Copy link
Member

ikelos commented Oct 20, 2019

Cool, thanks! 5:) Hopefully we'll get full-buy in on our proposed review policy soon and then we can start processing some of these requests. Thanks for your patience! 5:)

@ikelos ikelos assigned ikelos and npetroni and unassigned ikelos Nov 13, 2019
@npetroni
Copy link

Hi, sorry again for the delay on this. @ilch1 has agreed to pick this up and see it through, we agree there is value in including something like this in the repository. I'll assign it to him and he will pick up the conversation.

@ilch1
Copy link

ilch1 commented Dec 2, 2019

First, thank you for your contribution, and apologies for taking so long to get back to you. We agree that this is a useful capability. I have reviewed the proposed change and have a few questions. The first question is do you think we need multiple Dockerfiles/images? The current implementation adds Dockerfiles for pdbconv, volatility, and volshell images. The only thing that is different between volatility and volshell images are the ENTRYPOINT and CMD. Do you think it makes sense combining these Dockerfiles/images?

@sk4la
Copy link
Author

sk4la commented Jan 9, 2020

Hey @ilch1,

One of the advantages of Docker is to embed an application with its own dependencies, which is the reason why I initially created one Dockerfile per tool. To give an example, executing vol.py does not have the same purpose and/or need the same dependencies as pdbconv.py, although both are part of the same framework. Also, using vol.py requires Python packages that are of no use to pdbconv.py (ex. yara-python), which would therefore not be included in the Docker image for this particular tool.

This one-tool-one-image configuration also made a better use of the entrypoint functionality provided natively by Docker, and made much clearer the purpose and versioning of this particular image (ex. volatility/shell:3.2.3b or volatility/pdbconv:3.0.1a), as one could update functionnality in one utility without affecting the others. If all tools were embedded in the same image, it would not be possible to correlate the Docker image's version number for that specific tool.

To that extent, I would even suggest creating an official account on Docker Hub for the Volatility Foundation on which one could push new images without having to explicit the "Volatility context" (ex. volatility/shell:3.0.1a, volatility/stuff:latest).

That being said, it is of course possible to create a wrapper script that encompasses all the aforementioned tools (see this script for a real-world example), which I honestly find more of a hassle since you often have to consult the repository to get the correct syntax for that particular wrapper, etc.

Hope I made it clearer to you.

@ilch1
Copy link

ilch1 commented Feb 12, 2020

Hi @sk4la,

We had quite a bit of internal discussion about your PR and docker. We definitely appreciate your contribution and think that being able to run volatility via docker is a nice capability. The main challenge is how to best integrate volatility3 with docker.

The general consensus is that having the symbols be part of an image is not quite right, both in terms of volatility and docker. Volatility has the ability to download symbols during analysis. Having the symbols reside in the image would cause re-download of the symbols each time. The ZIP files with the symbols for each OS are not all-inclusive. Users may want to provide symbols curated for their specific analysis. Including all of the symbols from the 3 ZIP files causes the docker images to be 2 GB in size! Omitting them results in an image that is < 100 MB. In the end, we felt that the symbols should be provide at run-time via a volume (same as a memory image). This enables preserving symbols that get downloaded and allows the users to provide custom symbols that may not be included in the ZIP files.

I agree with your description of the benefits of having multiple images. I feel that can still be accomplished while taking advantage of the shared dependencies for each image. This results in producing distinct images that share multiple docker file-system layers, thus saving space. My examination of the built docker images from the PR (with docker inspect) showed no such sharing and close to 8 GB of used space (2 GB for 2 intermediate images and 2 GB for 2 final images).

I implemented the proposed design on a branch (https://github.com/volatilityfoundation/volatility3/tree/docker). See what you think :).

@sk4la
Copy link
Author

sk4la commented Feb 13, 2020

Totaly agree with this philosophy @ilch1. The sole reason for which I embedded the symbols in the images was because I needed them on an air-gaped environment, and didn't want to rely on an intricate setup (basically downloading the symbols beside and mounting them in the containers to the right directory).

An important point though: Docker's continuous integration (CI) pipeline (see this page for the details) needs one Dockerfile per image, which means you won't be able to use the automated builds from Docker Hub with this configuration, in case you want to automate your image builds/deployments (which is quite handy). Hence the original arborescence I pushed.
Plus, I feel that in the long run, having a single Dockerfile for the three images might frighten some people only needing one and thus slow the adoption of your images.

@ilch1
Copy link

ilch1 commented Feb 25, 2020

Thanks for the feedback, @sk4la. We haven't decided if we will be using Docker's CI pipeline for Volatility images. If we do, then I agree that having one Dockerfile could be an issue. The Volatility3 repository has a lot of development and we wouldn't want to publish new images until we have some confidence in the stability of the codebase. That could mean manually publishing new docker images with a volatility release and not using Docker's CI pipeline. Still TBD.

When pulling a new image from Dockerhub, I rarely look at the Dockerfile that was used to build that image. Sometimes those Dockerfiles are not even available. Thus, we're not overly concerned about the impact of having one Dockerfile on the adoption of the images.

@ikelos ikelos changed the base branch from master to develop January 6, 2021 21:12
@sk4la
Copy link
Author

sk4la commented Dec 30, 2021

Closing the PR since this is now outdated. For those interested in this feature, please check out my own repository.

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

Successfully merging this pull request may close these issues.

4 participants