Bug 236857
| Summary: | Fix sysctl check for some sys/audit/process-control tests | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Olivier Cochard <olivier> |
| Component: | tests | Assignee: | Olivier Cochard <olivier> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | CC: | asomers, emaste, ngie |
| Priority: | --- | ||
| Version: | 12.0-RELEASE | ||
| Hardware: | Any | ||
| OS: | Any | ||
| Attachments: | |||
This needs to test for errno in {ENOTDIR,ENOENT}.
Created attachment 203222 [details]
patch (with errno) to skip tests if kernel build without CAPABILITIES
Ok, here is a patch with errno support.
But I don't understand what is the purpose of errno here, because it didn't give very useful information ?
Output with this -errno version patch applied:
# kyua test sys/audit/process-control:cap_enter_success
sys/audit/process-control:cap_enter_success -> skipped: sysctl for kern.features.security_capability_mode failed: No such file or directory [0.003s]
Results file id is usr_tests.20190328-201847-910402
Results saved to /root/.kyua/store/results.usr_tests.20190328-201847-910402.db
1/1 passed (0 failed)
=> The 'No such file or directory' seems misleading for me, because the message is about a sysctl.
(In reply to Olivier Cochard from comment #2) Let me rephrase for clarity; the test should be something of the form: if (sysctlbyname(...) == -1) { if (errno == ENOTDIR || errno == ENOENT) { atf_tc_skip("not testing; : %s", capname, strerror(errno)); } else { /* something happened with the sysctlbyname call; report the error */ } } Also, I realize that there's already a macro for this, called `ATF_REQUIRE_FEATURE(..)`. If you change the call to `ATF_REQUIRE_FEATURE("security_capability_mode");`, it will "do the right thing". You'll need to add... `#include <freebsd_test_suite/macros.h>` ... to the source file and also add `CFLAGS+= -I${SRCTOP}/tests` to the Makefile. Created attachment 203223 [details]
patch to skip tests if kernel build without CAPABILITIES
Thanks for the tips!
The code a lot's cleaner now and the error message too:
# kyua test sys/audit/process-control:cap_enter_success
sys/audit/process-control:cap_enter_success -> skipped: kernel feature (security_capability_mode) not present [0.003s]
Results file id is usr_tests.20190328-204336-320096
Results saved to /root/.kyua/store/results.usr_tests.20190328-204336-320096.db
1/1 passed (0 failed)
LGTM! Actually, I take that back. You should remove the old sysctlbyname(3) call and only use ATF_REQUIRE_FEATURE. Created attachment 203226 [details]
patch using ATF_REQUIRE_SYSCTL_INT
Good remark.
The original code was:
```
const char *capname = "kern.features.security_capability_mode";
ATF_REQUIRE_EQ(0, sysctlbyname(capname, &capinfo, &len, NULL, 0))
/* Without CAPABILITY_MODE enabled, cap_enter() returns ENOSYS */
if (!capinfo)
atf_tc_skip("Capsicum is not enabled in the system");
```
So, the purpose was to check if kern.features.security_capability_mode==1.
Which mean "Check if capability is enabled".
My patch was adding a "Check if this capability exist", before to check its status.
If I remove the old sysctlbyname(3) call, I will not catch this condition:
- Capability exist
- But capability is disabled
So I've read freebsd_test_suite/macros.h, and I've found these functions:
- ATF_REQUIRE_FEATURE(_feature_name)
- ATF_REQUIRE_KERNEL_MODULE(_mod_name)
- ATF_REQUIRE_SYSCTL_INT(_mib_name, _required_value)
And I think the best way should using ATF_REQUIRE_SYSCTL_INT(capname,1) that manage both condition.
With this new patch:
# kyua test sys/audit/process-control:cap_enter_success
sys/audit/process-control:cap_enter_success -> skipped: sysctl for kern.features.security_capability_mode failed: No such file or director
y [0.003s]
Results file id is usr_tests.20190328-230635-632822
Results saved to /root/.kyua/store/results.usr_tests.20190328-230635-632822.db
1/1 passed (0 failed)
Actually, ATF_REQUIRE_FEATURE is correct, it just isn't obvious. You have to look at the source for feature_present(3) in lib/libc/gen/feature_present.c to see that it does just what you want. It returns true if the sysctl exists and its value is unequal to 0. (In reply to Alan Somers from comment #8) Bingo :)... $ man feature_present FEATURE_PRESENT(3) FreeBSD Library Functions Manual FEATURE_PRESENT(3) ... DESCRIPTION The feature_present() function provides a method for an application to determine if a specific kernel feature is present in the currently running kernel. The feature argument specifies the name of the feature to check. The feature_present() function will return 1 if the specified feature is present, otherwise it will return 0. If the feature_present() function is not able to determine the presence of feature due to an internal error it will return 0. RETURN VALUES If feature is present then 1 is returned; otherwise 0 is returned. Created attachment 203241 [details]
cleanup the kernel feature check code
New version according to your explanations (thanks you for your patience).
LGTM! If you need a src committer approval, Approved by: emaste A commit references this bug: Author: olivier Date: Mon Apr 1 14:21:32 UTC 2019 New revision: 345765 URL: https://svnweb.freebsd.org/changeset/base/345765 Log: Fix and simplify code by using ATF_REQUIRE_FEATURE macro PR: 236857 Reviewed by: asomers, ngie Approved by: emaste MFC after: 1 month Sponsored by: Netflix Changes: head/tests/sys/audit/Makefile head/tests/sys/audit/process-control.c Waiting for MFC ^Triage: committed and MFCed back in 2019. |
Created attachment 203217 [details] patch to skip tests if kernel build without CAPABILITIES There are two tests that check if kern.features.security_capability_mode is correctly set (=1) , but if a kernel without CAPABILITIES these tests failed because non existing sysctl. Before patch: NANOBSD# kyua test sys/audit/process-control:cap_enter_success sys/audit/process-control:cap_enter_success -> failed: /var/jenkins/workspace/ocafirmware_build-master/FreeBSD/tests/sys/audit/process-con trol.c:1518: 0 != sysctlbyname(capname, &capinfo, &len, NULL, 0) [0.003s] Results file id is usr_tests.20190328-140620-191316 Results saved to /root/.kyua/store/results.usr_tests.20190328-140620-191316.db 0/1 passed (1 failed) ============== After patch: NANOBSD# kyua test sys/audit/process-control:cap_enter_success sys/audit/process-control:cap_enter_success -> skipped: sysctl kern.features.security_capability_mode doesn't exist [0.003s] Results file id is usr_tests.20190328-181625-015778 Results saved to /root/.kyua/store/results.usr_tests.20190328-181625-015778.db 1/1 passed (0 failed) NANOBSDx# kyua test sys/audit/process-control:cap_getmode_success sys/audit/process-control:cap_getmode_success -> skipped: sysctl kern.features.security_capability_mode doesn't exist [0.003s] Results file id is usr_tests.20190328-181746-138037 Results saved to /root/.kyua/store/results.usr_tests.20190328-181746-138037.db 1/1 passed (0 failed)