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

Add more granular "Advanced" filtering #13

Closed
wants to merge 4 commits into from

Conversation

HazardousBackup
Copy link

linuxserver.io


  • [ x ] I have read the contributing guideline and understand that I have made the correct modifications

Description:

I've added a set of ENV variables derived directly from the latest Docker API and matching files for filtering using them.
The ADVANCED env variable selects for the new files which use the new variables.

Issues that I see with this current solution:

  • Duplication of ENV variables between the regular and advanced modes - solution would likely be just to manually remove the duplicates but I didn't at the time so that they'd keep the 1:1 match with the Docker API
  • I think it'd be preferable to create a system that automatically updates for any Docker API updates - I created a rough perl script that generates the variables from the API yaml but it's not fit for inclusion and I'm not sure something that automatically edits the dockerfile is desirable or would fit into the current build process
  • It might be desirable for someone more familiar with the Docker API to double check there is no issues with letting all version through the same filter.

Benefits of this PR and context:

Should close #12 and should tighten up the security of the proxy which currently allows more commands through than probably intended.

How Has This Been Tested?

I did some rough testing by launching a compose with a container built on this change and one that operated as a console.
I then manually sent some curl commands through to try and access a couple allowed and disallowed options.
This was not as thorough as I'd like and should be double checked before merging.

Source / References:

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for opening this pull request! Be sure to follow the pull request template!

@LinuxServer-CI
Copy link
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/socket-proxy/1.26.1-r0-pkg-96306a20-dev-a54a68ef120808b8882d6fd3b5e29e074330e392-pr-13/index.html
https://ci-tests.linuxserver.io/lspipepr/socket-proxy/1.26.1-r0-pkg-96306a20-dev-a54a68ef120808b8882d6fd3b5e29e074330e392-pr-13/shellcheck-result.xml

Tag Passed
amd64-1.26.1-r0-pkg-96306a20-dev-a54a68ef120808b8882d6fd3b5e29e074330e392-pr-13
arm64v8-1.26.1-r0-pkg-96306a20-dev-a54a68ef120808b8882d6fd3b5e29e074330e392-pr-13

@HazardousBackup
Copy link
Author

@thespad Does this seem like a proper fix/pull request?
Sorry for pinging, just the original issue this is for was posted 3 weeks ago without response so I'm not sure if it's been noticed.

@LinuxServer-CI
Copy link
Contributor

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/socket-proxy/1.26.2-r0-pkg-2d0d4a77-dev-21a51e50082657a29b184b1a7b3686209e867b05-pr-13/index.html
https://ci-tests.linuxserver.io/lspipepr/socket-proxy/1.26.2-r0-pkg-2d0d4a77-dev-21a51e50082657a29b184b1a7b3686209e867b05-pr-13/shellcheck-result.xml

Tag Passed
amd64-1.26.2-r0-pkg-2d0d4a77-dev-21a51e50082657a29b184b1a7b3686209e867b05-pr-13
arm64v8-1.26.2-r0-pkg-2d0d4a77-dev-21a51e50082657a29b184b1a7b3686209e867b05-pr-13

@thespad
Copy link
Member

thespad commented Sep 10, 2024

Closing as per comments on #12

@thespad thespad closed this Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

[FEAT] Increase granularity of lockdown options
3 participants