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

seccomp: should valueTwo be required with SCMP_CMP_MASKED_EQ? #971

Open
flx42 opened this issue May 31, 2018 · 1 comment
Open

seccomp: should valueTwo be required with SCMP_CMP_MASKED_EQ? #971

flx42 opened this issue May 31, 2018 · 1 comment

Comments

@flx42
Copy link

flx42 commented May 31, 2018

valueTwo is listed as OPTIONAL. And in the Go it's listed as omitempty.

As a result, docker's seccomp policy will be encoded as this:

        {
          "names": [
            "clone"
          ],
          "action": "SCMP_ACT_ALLOW",
          "args": [
            {
              "index": 0,
              "value": 2080505856,
              "op": "SCMP_CMP_MASKED_EQ"
            }
          ]
        }

If the spec was generated by a Go program, you can assume that valueTwo is 0.
But if it was generated in any other way, it could be a malformed configuration.
Since it's potentially a critical security piece, I don't want to have to guess.

I think the spec should be more explicit in what to do if op == SCMP_CMP_MASKED_EQ, some ideas:

  • valueTwo could be REQUIRED for this op, it would require using a pointer in the Golang struct.
  • Or, the spec could mention that valueTwo defaults to 0 for this op.
  • valueTwo could be required to be unset for all other ops.
@wking
Copy link
Contributor

wking commented Jun 2, 2018

  • valueTwo could be REQUIRED for this op, it would require using a pointer in the Golang struct.
  • Or, the spec could mention that valueTwo defaults to 0 for this op.

Either of these would work, although I personally prefer the first.

  • valueTwo could be required to be unset for all other ops.

I think this is a good idea, as long as we make it clear that the requirement only applies to ops listed in the spec. libseccomp could add new comparison ops taking two (or more) arguments, and runtimes supporting those extensions would still be compliant with the spec.

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

No branches or pull requests

2 participants