checkAPI: also check version

While testing this package against a recent kernel and older libseccomp
library (2.2.x to 2.4.x) it appears that every time we perform an API
level check, libseccomp version check is also needed.

Modify checkAPI to also call checkVersion. Unify both functions, as well
as VersionError, to use minor, major, micro triad everywhere.

While at it:
 - fix checkAPI and checkVersion docs;
 - simplify their tests, add more test cases.

Signed-off-by: Kir Kolyshkin <[email protected]>

Author: kolyshkin

seccomp.go

@@ -35,9 +35,9 @@ import "C"
 // VersionError represents an error when either the system libseccomp version
 // or the kernel version is too old to perform the operation requested.
 type VersionError struct {
-	op             string // operation that failed or would fail
-	minVer         string // minimally required libseccomp version
-	curAPI, minAPI uint   // current and minimally required API versions
+	op                  string // operation that failed or would fail
+	major, minor, micro uint   // minimally required libseccomp version
+	curAPI, minAPI      uint   // current and minimally required API versions
 }
 
 func init() {
@@ -49,13 +49,13 @@ func init() {
 
 func (e VersionError) Error() string {
 	if e.minAPI != 0 {
-		return fmt.Sprintf("%s requires libseccomp >= %s and API level >= %d "+
+		return fmt.Sprintf("%s requires libseccomp >= %d.%d.%d and API level >= %d "+
 			"(current version: %d.%d.%d, API level: %d)",
-			e.op, e.minVer, e.minAPI,
+			e.op, e.major, e.minor, e.micro, e.minAPI,
 			verMajor, verMinor, verMicro, e.curAPI)
 	}
-	return fmt.Sprintf("%s requires libseccomp >= %s (current version: %d.%d.%d)",
-		e.op, e.minVer, verMajor, verMinor, verMicro)
+	return fmt.Sprintf("%s requires libseccomp >= %d.%d.%d (current version: %d.%d.%d)",
+		e.op, e.major, e.minor, e.micro, verMajor, verMinor, verMicro)
 }
 
 // ScmpArch represents a CPU architecture. Seccomp can restrict syscalls on a
@@ -874,7 +874,7 @@ func (f *ScmpFilter) GetNoNewPrivsBit() (bool, error) {
 func (f *ScmpFilter) GetLogBit() (bool, error) {
 	log, err := f.getFilterAttr(filterAttrLog)
 	if err != nil {
-		if e := checkAPI("GetLogBit", 3, "2.4.0"); e != nil {
+		if e := checkAPI("GetLogBit", 3, 2, 4, 0); e != nil {
 			err = e
 		}
 
@@ -897,7 +897,7 @@ func (f *ScmpFilter) GetLogBit() (bool, error) {
 func (f *ScmpFilter) GetSSB() (bool, error) {
 	ssb, err := f.getFilterAttr(filterAttrSSB)
 	if err != nil {
-		if e := checkAPI("GetSSB", 4, "2.5.0"); e != nil {
+		if e := checkAPI("GetSSB", 4, 2, 5, 0); e != nil {
 			err = e
 		}
 
@@ -950,7 +950,7 @@ func (f *ScmpFilter) SetLogBit(state bool) error {
 
 	err := f.setFilterAttr(filterAttrLog, toSet)
 	if err != nil {
-		if e := checkAPI("SetLogBit", 3, "2.4.0"); e != nil {
+		if e := checkAPI("SetLogBit", 3, 2, 4, 0); e != nil {
 			err = e
 		}
 	}
@@ -971,7 +971,7 @@ func (f *ScmpFilter) SetSSB(state bool) error {
 
 	err := f.setFilterAttr(filterAttrSSB, toSet)
 	if err != nil {
-		if e := checkAPI("SetSSB", 4, "2.5.0"); e != nil {
+		if e := checkAPI("SetSSB", 4, 2, 5, 0); e != nil {
 			err = e
 		}
 	}

seccomp_internal.go

@@ -302,19 +302,23 @@ var (
 
 // Nonexported functions
 
-// checkVersion returns an error if libseccomp version used during runtime
-// is less than the one required by major, minor, and micro arguments.
-// Argument op is arbitrary non-empty operation description, which
-// may is used as a part of the  error message returned.
+// checkVersion returns an error if the libseccomp version being used
+// is less than the one specified by major, minor, and micro arguments.
+// Argument op is an arbitrary non-empty operation description, which
+// is used as a part of the error message returned.
+//
+// Most users should use checkAPI instead.
 func checkVersion(op string, major, minor, micro uint) error {
 	if (verMajor > major) ||
 		(verMajor == major && verMinor > minor) ||
 		(verMajor == major && verMinor == minor && verMicro >= micro) {
 		return nil
 	}
 	return &VersionError{
-		op:     op,
-		minVer: fmt.Sprintf("%d.%d.%d", major, minor, micro),
+		op:    op,
+		major: major,
+		minor: minor,
+		micro: micro,
 	}
 }
 
@@ -721,29 +725,32 @@ func (scmpResp *ScmpNotifResp) toNative(resp *C.struct_seccomp_notif_resp) {
 	resp.flags = C.__u32(scmpResp.Flags)
 }
 
-// checkAPI checks if API level is at least minLevel, and returns an error
-// otherwise. Argument op is an arbitrary string description the operation,
-// and minVersion is the minimally required libseccomp version.
-// Both op and minVersion are only used in an error message.
-func checkAPI(op string, minLevel uint, minVersion string) error {
-	// Ignore error from getAPI -- it returns level == 0 in case of error.
+// checkAPI checks that both the API level and the seccomp version is equal to
+// or greater than the specified minLevel and major, minor, micro,
+// respectively, and returns an error otherwise. Argument op is an arbitrary
+// non-empty operation description, used as a part of the error message
+// returned.
+func checkAPI(op string, minLevel uint, major, minor, micro uint) error {
+	// Ignore error from getAPI, as it returns level == 0 in case of error.
 	level, _ := getAPI()
 	if level >= minLevel {
-		return nil
+		return checkVersion(op, major, minor, micro)
 	}
 	return &VersionError{
 		op:     op,
 		curAPI: level,
 		minAPI: minLevel,
-		minVer: minVersion,
+		major:  major,
+		minor:  minor,
+		micro:  micro,
 	}
 }
 
 // Userspace Notification API
 // Calls to C.seccomp_notify* hidden from seccomp.go
 
 func notifSupported() error {
-	return checkAPI("seccomp notification", 6, "2.5.0")
+	return checkAPI("seccomp notification", 6, 2, 5, 0)
 }
 
 func (f *ScmpFilter) getNotifFd() (ScmpFd, error) {

seccomp_ver_test.go

@@ -6,25 +6,26 @@ import (
 
 func TestCheckVersion(t *testing.T) {
 	for _, tc := range []struct {
-		// test input
+		// input
 		op      string
 		x, y, z uint
-		// test output
-		res string // empty string if no error is expected
+		// expectations
+		isErr bool
 	}{
-		{
-			op: "frobnicate", x: 100, y: 99, z: 7,
-			res: "frobnicate requires libseccomp >= 100.99.7 (current version: ",
-		},
-		{
-			op: "old-ver", x: 2, y: 2, z: 0, // 2.2.0 is guaranteed to succeed
-		},
+		{op: "verNew", x: 100, y: 99, z: 7, isErr: true},
+		{op: "verMajor+1", x: verMajor + 1, isErr: true},
+		{op: "verMinor+1", x: verMajor, y: verMinor + 1, isErr: true},
+		{op: "verMicro+1", x: verMajor, y: verMinor, z: verMicro + 1, isErr: true},
+		// Current version is guaranteed to succeed.
+		{op: "verCur", x: verMajor, y: verMinor, z: verMicro},
+		// 2.2.0 is guaranteed to succeed as it's minimally supported version.
+		{op: "verOld", x: 2, y: 2, z: 0},
 	} {
 		err := checkVersion(tc.op, tc.x, tc.y, tc.z)
 		t.Log(err)
-		if tc.res != "" { // error expected
+		if tc.isErr {
 			if err == nil {
-				t.Errorf("case %s: expected %q-like error, got nil", tc.op, tc.res)
+				t.Errorf("case %s: expected error, got nil", tc.op)
 			}
 			continue
 		}
@@ -35,27 +36,30 @@ func TestCheckVersion(t *testing.T) {
 }
 
 func TestCheckAPI(t *testing.T) {
+	curAPI, _ := getAPI()
 	for _, tc := range []struct {
-		// test input
-		op    string
-		level uint
-		ver   string
-		// test output
-		res string // empty string if no error is expected
+		// input
+		op      string
+		level   uint
+		x, y, z uint
+		// expectations
+		isErr bool
 	}{
-		{
-			op: "deviate", level: 99, ver: "100.99.88",
-			res: "frobnicate requires libseccomp >= 100.99.7 (current version: ",
-		},
-		{
-			op: "api-0", level: 0, // API 0 will succeed
-		},
+		{op: "apiHigh", level: 99, isErr: true},
+		{op: "api+1", level: curAPI + 1, isErr: true},
+		// Cases that should succeed.
+		{op: "apiCur", level: curAPI},
+		{op: "api0", level: 0},
+		{op: "apiCur_verCur", level: curAPI, x: verMajor, y: verMinor, z: verMicro},
+		// Adequate API level but version is too high.
+		{op: "verHigh", level: 0, x: 99, isErr: true},
+		// Other cases with version are checked by testCheckVersion.
 	} {
-		err := checkAPI(tc.op, tc.level, tc.ver)
+		err := checkAPI(tc.op, tc.level, tc.x, tc.y, tc.z)
 		t.Log(err)
-		if tc.res != "" { // error expected
+		if tc.isErr {
 			if err == nil {
-				t.Errorf("case %s: expected %q-like error, got nil", tc.op, tc.res)
+				t.Errorf("case %s: expected error, got nil", tc.op)
 			}
 			continue
 		}