OpenCTI-Platform / connectors

OpenCTI Connectors
https://www.opencti.io
Apache License 2.0
389 stars 419 forks source link

Define a Volume for any r/w filesystem locations #2819

Open MaxwellDPS opened 1 month ago

MaxwellDPS commented 1 month ago

Description

In the import document connector the location used by _download_import_file() needs to be defined as a volume.

This poses a security issue if users decide to just not run with the root filesystem of a container due to having no context of this and turning it off.

More broadly k8s covers security contexts on runtime really well, I would recommend all containers be able to run non-root with a read only filesystem

Per Docker best practice is to make anyplace files are created at run time a volume.

You should use the VOLUME instruction to expose any database storage area, configuration storage, or files and folders created by your Docker container. You are strongly encouraged to use VOLUME for any combination of mutable or user-serviceable parts of your image.

Environment

  1. OS (where OpenCTI server runs): N/A
  2. OpenCTI version: All so far
  3. OpenCTI client: N/A
  4. Other environment details: N/A

Reproducible Steps

Steps to create the smallest reproducible scenario:

  1. deploy import connector with a readonly root filesystem

Expected Output

Proper volume definitions

Actual Output

Runtime errors if no volume is mounted

Additional information

https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-pod https://docs.docker.com/build/building/best-practices/#volume

romain-filigran commented 1 month ago

Need to assess whether it is necessary to write the file to disk. cc @richard-julien

MaxwellDPS commented 1 month ago

Same issue on threat fox, it isnt even using a directory and is writing to yield them. This would be easy to swap out for StringIO

https://github.com/OpenCTI-Platform/connectors/blob/master/external-import/threatfox/src/main.py#L301

This is a soild example of why this is bad, assume a k8s pod with a readonly FS, an emptyDir is a perfect use case to cache a csv. But since CSV_PATH is really just f"{BASE_PATH}/data.csv" this is a security issue as I cant lock down the file system. This gives me 2 options:

  1. Use an initcontainer to copy the python file to a volume, then mount the volume to the main container.
    • This doesnt fix the issue as the python is still r/w
  2. Allow the container to run R/W

One of 2 solutions is needed:

  1. Make volumes intentional and defined in the container image
  2. Just use memory - Avoid the disk like the plague we are in a container after all

This makes no sense

        try:
            zipped_file = io.BytesIO(data)
            with zipfile.ZipFile(zipped_file, "r") as zip_ref:
                with zip_ref.open("full.csv") as full_file:
                    csv_data = full_file.read()
        except zipfile.BadZipFile:
            # Treat as an unzipped CSV from /recent/
            csv_data = data

        with open(CSV_PATH, "wb") as fd:
            fd.write(csv_data)

        with open(CSV_PATH, "r", encoding="utf-8") as fd:
            yield from (line for line in fd if not line.startswith("#"))

Recommending this is changed to not use a generator as no memory saving is happening anyhow by using csv_data = full_file.read()

        try:
            zipped_file = io.BytesIO(data)
            with zipfile.ZipFile(zipped_file, "r") as zip_ref:
                with zip_ref.open("full.csv") as full_file:
                    csv_data = full_file.read()
        except zipfile.BadZipFile:
            # Treat as an unzipped CSV from /recent/
            csv_data = data

        return [
            line 
            for line in csv_data.split() 
            if not line.startswith("#")
       ]
richard-julien commented 3 weeks ago

Yes @romain-filigran, i think @MaxwellDPS is right, there is no need to write the file in some connectors. That could be improved