Skip to content

chroot: handle slightly broken seccomp defaults #2105

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.

Sign up for GitHub

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

Closed
nalind wants to merge 1 commit into from
Closed

chroot: handle slightly broken seccomp defaults #2105

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions chroot/seccomp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
package chroot

import (
"github.com/opencontainers/runtime-spec/specs-go"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
libseccomp "github.com/seccomp/libseccomp-golang"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -118,15 +118,32 @@ func setSeccomp(spec *specs.Spec) error {
continue
}
var conditions []libseccomp.ScmpCondition
opsAreAllEquality := true
for _, arg := range rule.Args {
condition, err := libseccomp.MakeCondition(arg.Index, mapOp(arg.Op), arg.Value, arg.ValueTwo)
if err != nil {
return errors.Wrapf(err, "error building a seccomp condition %d:%v:%d:%d", arg.Index, arg.Op, arg.Value, arg.ValueTwo)
}
if arg.Op != specs.OpEqualTo {
opsAreAllEquality = false
}
conditions = append(conditions, condition)
}
if err = filter.AddRuleConditional(scnum, mapAction(rule.Action), conditions); err != nil {
return errors.Wrapf(err, "error adding a conditional rule (%q:%q) to seccomp filter", scnames[scnum], rule.Action)
// Okay, if the rules specify multiple equality
// checks, assume someone thought that they
// were OR'd, when in fact they're ordinarily
// supposed to be AND'd. Break them up into
// different rules to get that OR effect.
if len(rule.Args) > 1 && opsAreAllEquality && err.Error() == "two checks on same syscall argument" {
Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat Jan 22, 2020

Choose a reason for hiding this comment

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

Where's the string from? It always worries me to check for an error by looking at the string output rather than a particular error type.

Copy link
Member Author

@nalind nalind Jan 23, 2020

Choose a reason for hiding this comment

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

I share your concern. This is an error object that github.com/seccomp/libseccomp-golang creates every time it returns the error, so we can't just compare the error itself for equality with a known value. If there's a better method, I'm happy to switch to it.

for i := range conditions {
if err = filter.AddRuleConditional(scnum, mapAction(rule.Action), conditions[i:i+1]); err != nil {
return errors.Wrapf(err, "error adding a conditional rule (%q:%q[%d]) to seccomp filter", scnames[scnum], rule.Action, i)
}
}
} else {
return errors.Wrapf(err, "error adding a conditional rule (%q:%q) to seccomp filter", scnames[scnum], rule.Action)
}
}
}
}
Expand Down