Valgrind showed that the disconnected paths variables were leaking
during the merge. That happened because flagvals did not implement a
destructor freeing the variables, so they leaked. flagvals cannot
implement a destructor, because that would make it a non-trivial union
member and parser_yacc.y would not compile. This patch implements a
"clear" function that is supposed to act as the destructor.
$ /usr/bin/valgrind --leak-check=full --error-exitcode=151 ../apparmor_parser -Q -I simple_tests/ -M ./features_files/features.all flags_ok_disconnected_ipc15.sd
...
==3708747== 5 bytes in 1 blocks are definitely lost in loss record 1 of 11
==3708747== at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==3708747== by 0x492E35E: strdup (strdup.c:42)
==3708747== by 0x14C74E: set_disconnected_path (profile.h:188)
==3708747== by 0x14C74E: flagvals::init(char const*) (profile.h:223)
==3708747== by 0x14859B: yyparse() (parser_yacc.y:592)
==3708747== by 0x141A99: process_profile(int, aa_kernel_interface*, char const*, aa_policy_cache*) (parser_main.c:1187)
==3708747== by 0x135421: main (parser_main.c:1771)
...
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
While symtab for now has only static members, it will allow for a
change in the future for each profile to have their own symbols like
profile_name, etc.
Currently abi=<kernel> always grabs the kernels actual features but
it should respect --kernel-features=. This is causing the simple
tests to fail when abi=<kernel> is specified.
Fix it so abi=<kernel> respects the kernel abi specified in the configs
or on the command line.
Signed-off-by: John Johansen <john.johansen@canonical.com>
There is an integer overflow when comparing priorities when cmp is
used because it uses subtraction to find lessthan, equal, and greater
than in one operation.
But INT_MAX and INT_MIN are being used by priorities and this results
in INT_MAX - INT_MIN and INT_MIN - INT_MAX which are both overflows
causing an incorrect comparison result and selection of the wrong
rule permission.
Closes: https://gitlab.com/apparmor/apparmor/-/issues/452
Fixes: e3fca60d1 ("parser: add the ability to specify a priority prefix to rules")
Signed-off-by: John Johansen <john.johansen@canonical.com>
As for %s when the pointer is null: even if gcc prints (null) this is still undefined behavior, so we should do this explicitly
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
This enables adding a priority to a rules in policy, finishing out the
priority work done to plumb priority support through the internals in
the previous patch.
Rules have a default priority of 0. The priority prefix can be added
before the other currently support rule prefixes, ie.
[priority prefix][audit qualifier][rule mode][owner]
If present a numerical priority can be assigned to the rule, where the
greater the number the higher the priority. Eg.
priority=1 audit file r /etc/passwd,
priority=-1 deny file w /etc/**,
Rule priority allows the rule with the highest priority to completely
override lower priority rules where they overlap. Within a given
priority level rules will accumulate in standard apparmor fashion.
Eg. given
priority=1 w /*c,
priority=0 r /a*,
priority=-1 k /*b*,
/abc, /bc, /ac .. will have permissions of w
/ab, /abb, /aaa, .. will have permissions of r
/b, /bcb, /bab, .. will have permissions of k
User specified rule priorities are currently capped at the arbitrary
values of 1000, and -1000.
Notes:
* not all rule types support the priority prefix. Rukes like
- network
- capability
- rlimits need to be reworked
need to be reworked to properly preserve the policy rule structure.
* this patch does not support priority on rule blocks
* this patch does not support using a variable in the priority value.
Signed-off-by: John Johansen <john.johansen@canonical.com>
the parser front end boolean is used for both boolean and integer
values. This is confusing when integer values different than 1 or 0
are being assigned to and from boolean.
Split its uses into the correct semantic boolean and integer cases.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This adds support for prompt rules and the beginning of support for extended permissions. Currently extended permissions are only used if a prompt rule is used in policy.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1305
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
There are two distinct declarations of perms_t.
rule.h: typedef uint32_t perms_t
hfa.h: class perms_t
these definitions clash when the front end and backend share more info.
To avoid this rename rule.h to perm32_t, and move the definition into
perms.h and use it in struct aa_perms.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add the ability to parse the prompt qualifier but do not provide
functionality because the backend does not currently support prompt
permissions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Remove conditional logic from the parser and move it to its own class,
that way any improvements or conditional features will make cleaner
changes.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Options available are ip= and port= inside the peer group or outside,
representing local addresses and ports:
network peer=(ip=127.0.0.1 port=8080),
network ip=::1 port=8080 peer=(ip=::2 port=8081),
The 'ip' option supports both IPv4 and IPv6. Examples would be
ip=192.168.0.4, or ip=::578d
The 'port' option accepts a 16-bit unsigned integer. An example would
be port=1234
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Extend the policy syntax to have a rule that allows specifying all
permissions for all rule types.
allow all,
This is useful for making blacklist based policy, but can also be
useful when combined with other rule prefixes, eg. to add audit
to all rules.
audit access all,
Signed-off-by: John Johansen <john.johansen@canonical.com>
There is one significant difference in the encoding of the network
rules. Before this change, when the parser was encoding a "network,"
rule, it would generate an entry for every family and every
type/protocol. After this patch the parser should generate an entry
for every family, but the type/protocol is changed to .. in the pcre
syntax. There should be no difference in behavior.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
We have a basic flagvals class it makes sense to move the parser
code into it, to help make management easier going forward.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently you need to use attach_disconnected with
attach_disconnected.path=XXX to be able to attach to a different
location than / whic is ugly and redundant.
Make it so attach_disconnected.path implies attach_disconnected.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add support for specifying the path prefix used when attach disconnected
is specified. The kernel supports prepending a different value than
/ when a path is disconnected. Expose through a profile flag.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Allowing access to a debug flag can greatly improve policy debugging.
This is different than the debug mode of old, that was removed. It only
will trigger additional messages to the kernel ring buffer, not
the audit log, and it does not change mediation.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The code for add_local_entry is actually currently unused and will
have to change anyways by the time it is. Some drop it and the
associated variables.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Another step towards having a block rule and retaining parsed rule
structure. Setup the parse to use a common block pattern, that when
we are ready will become an actual rule.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The post_process() method is misnamed, it fires when the profile is
finished parsing but fires before variable expansion. Rename it
to better reflect what it does and move the trigger code into
profile as a start of cleaning this stage up.
Also document the order the hooks fire in
Signed-off-by: John Johansen <john.johansen@canonical.com>
The previous patch enable the prefix based rules all to use the
same code pattern. Group them together
Signed-off-by: John Johansen <john.johansen@canonical.com>
Cleanup the parse code by making shared prefix and perms classes for
rules and convert rules to use them.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This removes the struct wrapper used in the previous patch to ensure
that all uses are properly converted.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Audit control support is going to be extended to support allowing
policy to which rules should quiet auditing. Update the frontend
internals to prepare for this.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This removes the struct wrapper used in the previous patch to ensure
that all uses are properly converted.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This delays the convertion of the audit flag until passing to the
backend. This is a step towards fix the parser front end so that it
doesn't use encoded permission mappings.
Note: the patch embedds the bool conversion into a struct to ensure
the compiler will fail to build unless every use is fixed. The
struct is removed in the following patch.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Make mount permission set consistent with the other rule types. This
is a step towards refactoring.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Move from using and int for permissions bit mask to a perms_t type.
Also move any perms mask that uses the name mode to perms to avoid
confusing it with other uses of mode.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The kernel will allow for a couple of debug flags on a profile that
can be used to trigger debug messages for only profiles/labels that
have the flag set. Add basic support for these to the parser.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Unfortunately the parser was doing ifdef checks for capabilities
in two places. Move all the capability ifdefs into capability.h
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/768
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Georgia Garcia <georgia.garcia@canonical.com>
Profile includes can be setup to loop and expand in a pathalogical
manner that causes build failures. Fix this by caching which includes
have already been seen in a given profile context.
In addition this can speed up some profile compiles, that end up
re-including common abstractions. By not only deduping the files
being included but skipping the need to reprocess and dedup the
rules within the include.
Fixes: https://bugzilla.suse.com/show_bug.cgi?id=1184779
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/743
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
yyerror is outputting the file name twice when not in a profile or
the profilename global is not defined. Drop the second output of
the file name as it just clutters up the error message.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/610
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Mike Salvatore <mike.salvatore@canonical.com>
Make all warnings that go through pwarn() controllable by warning
flags. This adds several new warning control flags, documented in
--help=warn
Convert --debug-cache to be unified with warning flags. So it can be
set by either
--debug-cache
or
--warn=debug-cache
Also add an "all" option to be able to turn on all warnings.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add an option to allow setting/pinning the feature ABI and overriding
of ABI rules if they exist.
--override-policy-abi
This option is primarily for profile development and testing without
allowing adjusting feature abis temporarily without modifying the
profile.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/579
Signed-off-by: John Johansen <john.johansen@canonical.com>
