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 no-new-privileges to SecurityOptions returned by /info #45320

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

akerouanton
Copy link
Member

- What I did

Add no-new-privileges to SecurityOptions returned by /info

Partially fixes #45311.

- How to verify it

$ docker info 2>/dev/null | grep -A 4 "Security Options"
Security Options:
 seccomp
  Profile: builtin
 cgroupns
 no-new-privileges

- Description for the changelog

  • The API now indicates no-new-privileges is globally enabled.

@thaJeztah
Copy link
Member

Looks like the docs on this field are a bit skimpy;
https://docs.docker.com/engine/api/v1.42/#tag/System/operation/SystemInfo

Perhaps some of that was intentional (to prevent only partially documenting things), I think we could improve it though, if we know what things can appear there (we probably can).

I'm good with keeping that for a follow-up, but I would suggest;

  • Add this new option to the examples section for the field;

    moby/api/swagger.yaml

    Lines 5243 to 5259 in 3d0bdfa

    SecurityOptions:
    description: |
    List of security features that are enabled on the daemon, such as
    apparmor, seccomp, SELinux, user-namespaces (userns), and rootless.
    Additional configuration options for each security feature may
    be present, and are included as a comma-separated list of key/value
    pairs.
    type: "array"
    items:
    type: "string"
    example:
    - "name=apparmor"
    - "name=seccomp,profile=default"
    - "name=selinux"
    - "name=userns"
    - "name=rootless"
  • I THINK for this one it's fine to not gate it by API version (as the list of items to expect is not a hard contract). That said, we could do so if we want to avoid users depending on the API to check if it's supported (and some servers with the same API version would return it, and others not) ❓ (any thoughts on that?)
  • Add a mention in the API history. If we decide not to version it, then it's good to call that out in the change log (we do so for a couple other entries, which you can use as an example ("This change is not versioned, and affects all API versions if the daemon has this patch."))
    https://github.com/moby/moby/blob/master/docs/api/version-history.md

daemon/info_unix.go Outdated Show resolved Hide resolved
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, but left a comment about the function being exported 🙈

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah thaJeztah merged commit 20a1d23 into moby:master Apr 18, 2023
92 checks passed
@akerouanton akerouanton deleted the info-no-new-privileges branch April 18, 2023 12:57
@Frydzyslaw
Copy link

Super and working

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.

Security Options not getting update if no-new-privileges is enabled in /etc/docker/daemon.json
4 participants