This is not a real change - since some commits, include_dir_filelist()
gets only called with absolute paths.
Add a check to ensure this, drop the now superfluous get_include_path()
call, and replace usage of include_name_abs with include_name (which are
the same now).
Also drop the superfluous profile_dir parameter, and adjust the only
caller accordingly.
With the check that incfile starts with a '/', incfile and incfile_abs
(as returned by get_include_path()) are always the same.
Drop the get_include_path() call and setting incfile_abs, and replace
usage of incfile_abs with incfile.
This is just a cleanup, no behaviour change.
... so that the include rules proposed by aa-logprof continue to be
relative to the profile directory.
This fixes the behaviour change introduced in the previous commit.
This removes the need to remove profile_dir from include paths at
various places.
A side effect is that aa-logprof / match_includes() now propose more
include rules, for example matching local/ files.
Another side effect is that proposals for include rules
(match_includes() again) now come with the full path.
Both side effects will be fixed in the next commits.
This is needed for running the tests, because test/logprof.conf contains
a relative path, and tests only "manually" set the profile_dir if they
need/have a modified copy of the profiles.
chfa.cc:348:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 16));
^~
chfa.cc:349:3: note: here
case 2:
^~~~
chfa.cc:350:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 8));
^~
chfa.cc:351:3: note: here
case 1:
^~~~
chfa.cc: In function ‘void write_flex_table(std::ostream&, int, Iter, Iter) [with Iter = __gnu_cxx::__normal_iterator<unsigned int*, std::vector<unsigned int> >]’:
chfa.cc:348:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 16));
^~
chfa.cc:349:3: note: here
case 2:
^~~~
chfa.cc:350:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 8));
^~
chfa.cc:351:3: note: here
case 1:
^~~~
chfa.cc: In function ‘void write_flex_table(std::ostream&, int, Iter, Iter) [with Iter = __gnu_cxx::__normal_iterator<short unsigned int*, std::vector<short unsigned int> >]’:
chfa.cc:348:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 16));
^~
chfa.cc:349:3: note: here
case 2:
^~~~
chfa.cc:350:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 8));
^~
chfa.cc:351:3: note: here
case 1:
^~~~
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
-Wimplicit-fallthrough only recognizes specic comment patterns
switch to a comment it recognizes.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
aa_status.c: In function ‘get_processes’:
aa_status.c:236:10: warning: unused variable ‘len’ [-Wunused-variable]
size_t len = 0;
^~~
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Cleanup unused var warning
aa_enabled.c: In function ‘exit_with_error’:
aa_enabled.c:34:6: warning: unused variable ‘err’ [-Wunused-variable]
int err;
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Tag unused parameters so the -Wunused-parameter won't complain about
them.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Tag unused parameters so the -Wunused-parameter won't complain about
them.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
PDEBUG is defined away when debug isn't configure in the build, this
hides the bad format string and argument.
Fix this and make sure we can still have the debug output that was
supposed to be printed.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
private.c: In function ‘_aa_is_blacklisted’:
private.c:140:35: warning: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
140 | found - name + suffix->len == name_len ) {
| ^~
private.c: In function ‘readdirfd’:
private.c:352:16: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘ssize_t’ {aka ‘long int’} [-Wsign-compare]
352 | for (i = 0; i < n; ) {
| ^
private.c:378:17: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘ssize_t’ {aka ‘long int’} [-Wsign-compare]
378 | for (i = 0; i < n; i++)
| ^
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The len parameter returns a value that correlates to a getsockopt
parameter which is typed to socklen_t which is an unsigned int.
This technically changes the fn() api but old code using this is
already broken if the getsockopt parameter is large enough to overflow
the value.
In reality what is returned shouldn't ever be negative and the value
should never be large enough to trip the overflow. This is just
cleaning up a corner case.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561 Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
kernel.c: In function ‘aa_getpeercon_raw’:
kernel.c:823:14: warning: comparison of integer expressions of different signedness: ‘socklen_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
823 | if (optlen < *len) {
| ^
kernel.c: In function ‘query_label’:
kernel.c:966:10: warning: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
966 | if (ret != size) {
| ^~
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561 Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
IncludeRule: sort files in included directory
... instead of relying on the filesystem(!) ordering, which will look
random to both users and unittests.
Also partially revert the test changes from
c5a7bcd50e /
https://gitlab.com/apparmor/apparmor/-/merge_requests/548 -
sorting the result only in the tests is a bad idea.
See merge request apparmor/apparmor!552
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
This fixes lots of warnings like
```
*** Checking abstractions from ./apparmor.d/abstractions against apparmor_parser
Warning from stdin (stdin line 1): ../parser/apparmor_parser: File 'stdin' missing feature abi, falling back to default policy feature abi
Warning from stdin ([...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile line 9): ../parser/apparmor_parser: [...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile features abi 'abi/3.0' differes from policy declared feature abi, using the features abi declared in policy
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/554
Acked-by: John Johansen <john.johansen@canonical.com>
This matches what we use in the profiles for local abstractions.
Also adjust the check in the Makefile to expect the variant without '#'.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/553
Acked-by: John Johansen <john.johansen@canonical.com>
```
libaalogparse.c: In function 'hex_to_string':
libaalogparse.c:144:16: warning: comparison of integer expressions of different signedness: 'int' and 'size_t' {aka 'long unsigned int'} [-Wsign-compare]
144 | for (i = 0; i < len; i++) {
| ^
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/558
Acked-by: John Johansen <john.johansen@canonical.com>
Following up on !549, this patchset unifies most of the compiler warnings settings to use EXTRA_WARNINGS as newly defined in `common/Make.rules` and then adds the `-Wimplicit-fallthrough` compiler warning to the default set.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/551
Acked-by: John Johansen <john.johansen@canonical.com>
```
libaalogparse.c: In function 'hex_to_string':
libaalogparse.c:144:16: warning: comparison of integer expressions of different signedness: 'int' and 'size_t' {aka 'long unsigned int'} [-Wsign-compare]
144 | for (i = 0; i < len; i++) {
| ^
```
`i` gets used/changed as counter variable in the for loop and only gets
increased (starting at 0), so making it an (unsigned) size_t should be
safe.
This fixes lots of warnings like
```
*** Checking abstractions from ./apparmor.d/abstractions against apparmor_parser
Warning from stdin (stdin line 1): ../parser/apparmor_parser: File 'stdin' missing feature abi, falling back to default policy feature abi
Warning from stdin ([...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile line 9): ../parser/apparmor_parser: [...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile features abi 'abi/3.0' differes from policy declared feature abi, using the features abi declared in policy
```
AppArmor 3.0 tags policy with the feature abi it was developed under. This fixes issues with kernel upgrades that add new mediation features and reduces the need to pin policy.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Tag profiles and abstractions with abi information.
Tagging abstractions is not strictly necessary but allows the parser
to detect when their is a mismatch and that policy will need an
update for abi.
We do not currently tag the tunables because variable declarations
are not currently affected by abi.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Add basic support for policy to specify a feature abi. Under the
current implementation the first feature abi specified will be
used as the policy abi for the entire profile.
If no feature abi is defined before rules are processed then the
default policy abi will be used.
If multiple feature abi rules are encountered and the specified
abi is different then a warning will be issued, and the initial abi
will continue to be used. The ability to support multiple policy
feature abis during a compile will be added in a future patch.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The features abi adds the ability to track the policy abi separate
from the kernel. This allow the compiler to determine whether policy
was developed with a certain feature in mind, eg. unix rules.
This allows the compiler to know whether it should tell the kernel to
enforce the feature if the kernel supports the rule but the policy
doesn't use it.
To find if a feature is supported we take the intersection of what is
supported by the policy and what is supported by the kernel.
Policy encoding features like whether to diff_encode policy are not
influenced by policy so these remain kernel only features.
In addition to adding the above intersection of policy rename
--compile-features to --policy-features as better represents what it
represents. --compile-features is left as a hidden item for backwards
compatibility.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
As usual, the classes come with some tests.
A side effect of the change is a fix for the "last one wins" bug if a profile has two alias rules with the same path on the left side.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/550
Acked-by: John Johansen <john.johansen@canonical.com>
Define EXTRA_WARNINGS in the common/Make.rules helper so that adding
additional warnings can be done in one(-ish) location, and replace
locally defined C compiler warning flags with EXTRA_WARNINGS in most
locations in the build tree.
v2: issue a warning for any compiler option that the compiler does not
support
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
... instead of relying on the filesystem(!) ordering, which will look
random to both users and unittests.
Also partially revert the test changes from
c5a7bcd50e /
https://gitlab.com/apparmor/apparmor/-/merge_requests/548 -
sorting the result only in the tests is a bad idea.
This fixes cases when two aliases with the same left side were
configured - instead of "last one wins" in the dict, AliasRuleset now
keeps both.
ProfileList add_alias() changes its parameters and now expects an
AliasRule object. Adjust all callers to that.
Drop the no longer needed write_alias().
Also adjust the tests to use AliasRule and add a dedup test promised in
an earlier patch series.
asprintf(3) returns a signed int, so storing the result in a size_t is
and then comparing that stored value against -1 is not such a good idea.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/549
Add VariableRule and VariableRuleset and use it for variable handling
Besides the usual advantages of switching to classes, we finally get rid of the `filelist` hasher.
While on it, also fix some bugs around variable handling, including https://bugs.launchpad.net/apparmor/+bug/1331856 and some that maybe nobody noticed before.
As usual, see the individual commits for details.
See merge request apparmor/apparmor!544
Acked-by: Steve Beattie <steve.beattie@canonical.com>
