aa_getprocattr is returning the size of the buffer not the size of the
data read that it is supposed to return. Also update the man page to
reflect the return value as documented in the functions, and update
the test cases to check the return value.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Steve Beattie <sbeattie@ubuntu.com>
While aa_getprocattr does return the documented error code on failure
the **buf and **mode parameters can point into the buffer that was
allocated and then discarded on failure.
Set them to null on failure so that even if the error code is ignored
they do not point to heap data.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Remount should not be screening off the set of flags it is. They are
the set of flags that the kernel is masking out for make_type and
should not be used on remount. Instead just screen off the other cmds
that can have their own rules generated.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
The deny information is not used as valid accept state information,
so remove it from the is_null test. This does not change the dfa
generated but does result in the dumped information changing,
as states that don't have any accept information are no longer
reported as accepting. This is what changes the number of states
reported in the minimize tests.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
The same mappings routine had two bugs in it, that in practice haven't
manifested because of partition ordering during minimization. The
result is that some states may fail comparison and split, resulting
in them not being eliminated when they could be.
The first is that direct comparison to the nonmatching state should
not be done as it is a candiate for elimination, instead its partion
should be compared against. This simplifies the first test
The other error is the comparison
if (rep->otherwise != nonmatching)
again this is wrong because nomatching should not be directly
compared against. And again can result in the current rep->otherwise
not being eliminated/replaced by the partion. Again resulting in
extra trap states.
These tests where original done the way they were because
->otherwise could be null, which was used to represent nonmatching.
The code was cleaned up a while ago to remove this, ->otherwise is
always a valid pointer now.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Also make sure the perms method properly switches to hex and back to dec
as some of the previous perm dump code did not.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
The aa-exec command can be used to launch an application under a specified
confinement, which may be different for what regular profile attachment
would apply.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The changes are around how user data is handled.
1. permissions are mapped before data is matched
2. If data is to be mapped a AA_CONT_MATCH flag is set in the permissions
which allows data matching to continue.
3. If data auditing is to occur the AA_AUDIT_MNT_DATA flag is set
This allows better control over matching and auditing of data which can
be binary and should not be matched or audited
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
file, should grant access to all files paths on the system but it does
not currently allow access to /
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Rename the pivotroot rule to pivot_root to match the command and the fn
and fix it to support named transition correctly leveraging the parsing
action used for exec transitions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Currently the backend doesn't like it (blows up) when the a vector entry is
empty. For the case where no flags match build_mnt_flags generates an
alternation of an impossible entry and nothing
(impossible|)
This provides the effect of a null entry without having an empty vector
entry. Unfortunately the impossible entry is not correct.
Note: how this is done needs to be changed and fixed in the next release
this is just a minimal patch to get it working for 2.8
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
When generating the flag set the parser was not generating the complete
set when flags where not consecutive. This is because the len value
was not being reset for each flag considered, so once it was set for
a flag, then the next flag would have to be set to reset it else the
output string was still incremented by the old len value.
Eg.
echo "/t { mount options=rbind, }" | apparmor_parser -QT -D rule-exprs
results in
rule: \x07[^\000]*\x00[^\000]*\x00[^\000]*\x00\x0d ->
however \x0d only covers the bind and not the recursive flag
This is fixed by adding a continue to the flags generation loop for the
else case.
resulting the dump from above generating
rule: \x07[^\000]*\x00[^\000]*\x00[^\000]*\x00\x0d\x0f ->
\x0d\x0f covers both of the required flags
Also fix the flags output to allow for the allow any flags case. This
was being screened out. By masking the flags even when no flags where
specified.
this results in a difference of
echo "/t { mount, }" | apparmor_parser -QT -D rule-exprs
rule: \x07[^\000]*\x00[^\000]*\x00[^\000]*\x00(\x01|)(\x02|)(\x03|)(\x04|)(\x05|)\x00[^\000]*
becoming
\x07[^\000]*\x00[^\000]*\x00[^\000]*\x00[^\000]*\x00[^\000]*
which is simplified and covers all permissions vs. the first rule output
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
This commit causes policy problems because we do not have chroot rules
and policy extension to support it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
the cache test is failing because it assumes that kernel features are
stored in a file instead of a directory
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
On newer kernels the features directory causes the creation of a
cache/.feature file that contains newline characters. This causes the
feature comparison to fail, because get_flags_string() uses fgets
which stop reading in the feature file after the first newline.
This caches the features comparision to compare a single line of the
file against the full kernel feature directory resulting in caching
failure.
Worse this also means the cache won't get updated as the parser doesn't
change what set gets caches after the .feature file gets created.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
file,
was not given the correct permissions. It was only being given the owner
set of permissions. This would result in rejects when trying look at
files owned by other users
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
There are some rare occassions, when lots of alternations are used that
tree simplification can result in an expression of
(E | (E | E)) or (E . (E . E)) where E is the epsnode
both of these expressions will lead to an inifinite loop in normalize_tree
as the epsnode test
if ((&epsnode == t->child[dir]) &&
(&epsnode != t->child[!dir]) &&
dynamic_cast<TwoChildNode *>(t)) {
and the tree node rotation test
} else if ((dynamic_cast<AltNode *>(t) &&
dynamic_cast<AltNode *>(t->child[dir])) ||
(dynamic_cast<CatNode *>(t) &&
dynamic_cast<CatNode *>(t->child[dir]))) {
end up undoing each others work, ie.
eps flip rotate
(E | (E | E)) --------> ((E | E) | E) -------> (E | (E | E))
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Minimization was failing because it was too agressive. It was minimizing
as if there was only 1 accept condition. This allowed it to remove more
states but at the cost of loosing unique permission sets, they where
being combined into single commulative perms. This means that audit,
deny, xtrans, ... info on one path would be applied to all other paths
that it was combined with during minimization.
This means that we need to retain the unique accept states, not allowing
them to be combined into a single state. To do this we put each unique
permission set into its own partition at the start of minimization.
The states within a partition have the same permissions and can be combined
within the other states in the partition as the loss of unique path
information is will not result in a conflict.
This is similar to what perm hashing used to do but deny information is
still being correctly applied and carried.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
The in x intersection consistency test for minimization was failing because
it was screening off the AA_MAY_EXEC permission before passing the exec
information to the consistency test fn. This resulted in the consistency
test fn not testing the consistency because it treated the permission set
as not having x permissions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Make them report a hex value strings instead of the default C++
\vvvvv
Make them consistent,
- Dump to report the default transition and what isn't transitioned
on it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The permission reporting was not reporting the full set of permission
flags and was inconsistent between the dump routines.
Report permissions as the quad (allow/deny/audit/quiet) in hex.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Fix the transitions states output so that they output the state label
instead of the state address. That is
{1} -> 0x10831a0: /
now becomes
{1} -> {2}: /
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
The pcre parser in the dfa backend is not correctly converting escaped
hex string like
\0x0d
This is the minimal patch to fix, and we should investigate just using
the C/C++ conversion routines here.
I also I nominated for the 2.7 series.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@gmail.com>
Add the ability to control mounting and unmounting
The basic form of the rules are.
[audit] [deny] mount [conds]* [device] [ -> [conds] path],
[audit] [deny] remount [conds]* [path],
[audit] [deny] umount [conds]* [path],
[audit] [deny] pivotroot [oldroot=<value>] <path> -> <profile>
remount is just a short cut for mount options=remount
where [conds] can be
fstype=<expr>
options=<expr>
conds follow the extended conditional syntax of allowing either:
* a single value after the equals, which has the same character range as
regular IDS (ie most anything but it can't be terminated with a , (comma)
and if spaces or other characters are needed it can be quoted
eg.
options=foo
options = foo
options="foo bar"
* a list of values after the equals, the list of values is enclosed within
parenthesis () and its has a slightly reduced character set but again
elements can be quoted.
the separation between elements is whitespace and commas.
eg.
options=(foo bar)
options=(foo, bar)
options=(foo , bar)
options=(foo,bar)
The rules are flexible and follow a similar pattern as network, capability,
etc.
mount, # allow all mounts, but not umount or pivotroot
mount fstype=procfs, # allow mounting procfs anywhere
mount options=(bind, ro) /foo -> /bar, # readonly bind mount
mount /dev/sda -> /mnt,
mount /dev/sd** -> /mnt/**,
mount fstype=overlayfs options=(rw,upperdir=/tmp/upper/,lowerdir=/) overlay -> /mnt/
umount,
umount /m*,
Currently variables and regexs are are supported on the device and mount
point. ie.
mount <devince> -> <mount point>,
Regexes are supported in fstype and options. The options have a further
caveat that regexs only work if the option is fs specific option.
eg. options=(upperdir=/tmp/*,lowerdir=/)
regex's will not currently work against the standard options like ro, rw
nosuid
Conditionals (fstype) can only be applied to the device (source) at this
time and will be disregarded in situations where the mount is manipulating
an existing mount (bind, remount).
Options can be specified multiple times
mount option=rw option=(nosuid,upperdir=/foo),
and will be combined together into a single set of values
The ordering of the standard mount options (rw,ro, ...) does not matter
but the ordering of fs specific options does.
Specifying that the value of a particular option does not matter can be
acheived by providing both the positive and negative forms of and option
option=(rw,ro) options=(suid,nosuid)
For the fs specific options specifying that a particular value does not
matter is achieve using a regex with alternations.
Improvements to the syntax and order restrictions are planned for the
future.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Newer versions of AppArmor use a features directory instead of a file
update the parser to use this to determine features and match string
This is just a first pass at this to get things up quickly. A much
more comprehensive rework that can parse and use the full information
set is needed.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Due to changes in path looks and the work going forward default profiles
to resolve relative to the chroot instead of the namespace.
This will only affect profiles that are used on tasks within a chroot.
For now it will be possible to get the old default namespace relative
behavior by passing the namespace_relative flag to the profile
eg.
profile /example (namespace_relative) { .. }
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
policydb is the new matching format, that combines the matching portions
of different rules into a single dfa/hfa. This patch only lays some ground
work it does not add encoding of any rules into the policydb
Signed-off-by: John Johansen <john.johansen@canonical.com>
value_list can be reused by conditionals and list values, so pull it out
and abstract it some more.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
Add the optional 'file' keyword to the language/grammer. The main reason
for doing this is to support false token injection. Which is needed
to move towards the parser being broken out into an api that can be
used to parse individual rule types, separate from parsing the whole file.
Since we are adding the token to the grammar expose it to userspace with
the 'file' keyword. While not needed it helps bring consistency, as all
the other rule types start with a keyword (capability, network, rlimit, ...).
Also allow the bare keyword to be used to represent allowing all file
operations, just as with network and capability. Domain transitions are
defaulted to ix. Thus
file,
is equivalent to
/** rwlkmix,
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
The ability to set capabilities from a profile has been removed from the
kernel for several releases. Remove it from the parser as well.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
