Commit graph

6191 commits

Author SHA1 Message Date
Christian Boltz
c0734411ee
aa-genprof: get rid of subprocess with shell=True 2022-02-21 22:43:29 +01:00
Christian Boltz
c6dabdf1e8
add a common reload_profile() function to aa.py
This function is based on reload_profile() in tools.py, but also
replaces most of reload_base() in aa.py.

For bonus points, we get rid of shell=True when calling apparmor_parser.

Note: This slightly changes the behaviour of aa-logprof and aa-genprof -
if the parser errors out ($? > 0), the output no longer gets hidden.
However, this will not raise an exception, and aa-logprof and aa-genprof
won't abort on parser errors.
2022-02-21 22:05:59 +01:00
John Johansen
6e5fdb5c05 Merge smbd: allow reading under /usr/share/samba
Today, a normal user connected and did something (dunno what) that caused smbd to try to `/usr/share/samba/mdssvc/elasticsearch_mappings.json`:

Samba logs:

```
root@smb:~# journalctl -b0 -u smbd
-- Logs begin at Fri 2022-01-21 14:17:01 UTC, end at Thu 2022-02-17 23:56:02 UTC. --
Feb 17 14:01:20 smb systemd[1]: Starting Samba SMB Daemon...
Feb 17 14:01:26 smb smbd[113]: [2022/02/17 14:01:26.904865,  0] ../../lib/util/become_daemon.c:135(daemon_ready)
Feb 17 14:01:26 smb systemd[1]: Started Samba SMB Daemon.
Feb 17 14:01:26 smb smbd[113]:   daemon_ready: daemon 'smbd' finished starting up and ready to serve connections
Feb 17 21:05:35 smb smbd[3084]: pam_unix(samba:session): session opened for user jdoe by (uid=0)
Feb 17 21:05:37 smb smbd[3084]: [2022/02/17 21:05:37.735182,  0] ../../source3/rpc_server/mdssvc/mdssvc_es.c:92(mdssvc_es_init)
Feb 17 21:05:37 smb smbd[3084]:   mdssvc_es_init: Opening mapping file [/usr/share/samba/mdssvc/elasticsearch_mappings.json] failed: unable to open /usr/share/samba/mdssvc/elasticsearch_mappings.json: Permission denied
Feb 17 21:05:37 smb smbd[3084]: [2022/02/17 21:05:37.735436,  0] ../../source3/rpc_server/mdssvc/mdssvc.c:1490(mdssvc_init)
Feb 17 21:05:37 smb smbd[3084]:   mdssvc_init: backend init failed
Feb 17 21:05:37 smb smbd[3084]: [2022/02/17 21:05:37.735562,  0] ../../source3/rpc_server/mdssvc/srv_mdssvc_nt.c:152(_mdssvc_open)
Feb 17 21:05:37 smb smbd[3084]:   _mdssvc_open: Couldn't create policy handle for partage
Feb 17 23:56:02 smb smbd[3084]: pam_unix(samba:session): session closed for user jdoe
```

Since the 'smb' machine is a container, the Apparmor denial ended up in the host's log:

```
$ journalctl -o cat --grep samba -k --since today | cat
audit: type=1400 audit(1645131937.730:98): apparmor="DENIED" operation="open" namespace="root//lxd-smb_<var-snap-lxd-common-lxd>" profile="smbd" name="/usr/share/samba/mdssvc/elasticsearch_mappings.json" pid=35359 comm="smbd" requested_mask="r" denied_mask="r" fsuid=166549 ouid=165536

```

It is the first time it occurs in years of use but it seems legitimate as:

1) this file is installed by the package
2) `git grep -F elasticsearch_mappings` in Debian samba's source shows many hits:
```
$ git grep -F elasticsearch_mappings
debian/samba.install:usr/share/samba/mdssvc/elasticsearch_mappings.json
docs-xml/manpages/mdsearch.1.xml:         <filename>/usr/share/samba/mdssvc/elasticsearch_mappings.json</filename>
docs-xml/smbdotconf/misc/elasticsearchmappings.xml:  <value type="default">&pathconfig.SAMBA_DATADIR;/elasticsearch_mappings.json</value>
docs/manpages/mdfind.1:/usr/share/samba/mdssvc/elasticsearch_mappings\&.json
docs/manpages/smb.conf.5:\fI\fIelasticsearch:mappings\fR\fR\fI = \fR\fI${prefix}/var/samba/elasticsearch_mappings\&.json\fR\fI \fR
selftest/selftest.pl:   elasticsearch:mappings = $srcdir_abs/source3/rpc_server/mdssvc/elasticsearch_mappings.json
selftest/target/Samba3.pm:      elasticsearch:mappings = $srcdir_abs/source3/rpc_server/mdssvc/elasticsearch_mappings.json
source3/rpc_server/mdssvc/es_parser_test.c:             "%s/mdssvc/elasticsearch_mappings.json",
source3/rpc_server/mdssvc/mdssvc_es.c:          "%s/mdssvc/elasticsearch_mappings.json",
source3/rpc_server/wscript_build:                          'mdssvc/elasticsearch_mappings.json')
```

While only the `mdssvc` sub-dir could be authorized, the whole dir content seemed OK for read access anyway:

```
root@smb:~# ll /usr/share/samba/
total 53
drwxr-xr-x  5 root root   10 Feb  1 14:08 ./
drwxr-xr-x 67 root root   67 Jun 22  2021 ../
-rwxr-xr-x  1 root root 1163 Jan 31 13:11 addshare.py*
drwxr-xr-x  3 root root    4 Feb  1 14:08 admx/
drwxr-xr-x  2 root root    3 Feb  1 14:08 mdssvc/
-rwxr-xr-x  1 root root 2059 Jan 31 13:11 panic-action*
-rwxr-xr-x  1 root root 1333 Jan 31 13:11 setoption.py*
drwxr-xr-x  5 root root   57 Feb  1 14:08 setup/
-rw-r--r--  1 root root 8942 Jan 31 13:11 smb.conf
-rwxr-xr-x  1 root root 2682 Jan 31 13:11 update-apparmor-samba-profile*
```

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/853
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-21 20:04:49 +00:00
John Johansen
40402e2436 Merge Revert "gitlab: testing: temporarily disable secret-detect"
It appears secret detection is failing if the master branch in the tree a merge request is being made from is too (some unknown value) far behind the branch of the tree it is being merged into.

This is problematic as it is not started practice to refresh the upstream branches of forked trees, but to keep multiple remotes in a single local tree, branch from mainline master, work on the branch and push to the fork for the merge request. This will require contributors to refresh their forked trees in secret-detection fails. Which may be problematic for some contributors, but since we don't know how bad this is going to be, for now re-enable secret detection.

This reverts commit 8b4344c17b.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/854
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
2022-02-21 19:58:36 +00:00
John Johansen
51615755f8 Revert "gitlab: testing: temporarily disable secret-detect"
This reverts commit 8b4344c17b.
2022-02-21 11:31:44 -08:00
John Johansen
8b4344c17b gitlab: testing: temporarily disable secret-detect
Unfortunately secret detection is failing with
   fatal: error in object: unshallow sha1

and blocking merge requests. Unfortuntely all suggested work arounds
from https://gitlab.com/gitlab-org/gitlab/-/issues/351976 failed to
work.

Signed-off-by: John Johansen <john.johansen@canonical.com>
2022-02-18 00:19:31 -08:00
Simon Deziel
9e0adcfd07 smbd: allow reading under /usr/share/samba
Signed-off-by: Simon Deziel <simon@sdeziel.info>
2022-02-17 18:54:54 -05:00
Georgia Garcia
5001431cdf Merge parser: fix building with link time optimization (lto)
Libapparmor was fixed for lto builds on commit 7cde91f5 but
the parser was also failing due to the same reasons when lto
was enabled.

Fixes: https://gitlab.com/apparmor/apparmor/-/issues/214
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>

Closes #214
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/851
Acked-by: Approved-by: John Johansen <john@jjmx.net>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
2022-02-17 17:09:13 +00:00
John Johansen
6fa2d528e9 Merge Update apache2-common so that other processes can trace the hats that include...
Update apache2-common so that other processes can trace the hats that include this file. The main `usr.sbin.apache2` profile includes `abstractions/base` which has these lines in it, which is why `ss -tnlp` sometimes fails and sometimes works.

See also: [Debian Bug #1003153](https://bugs.debian.org/1003153) for more details about how this occurs.

Fixes: https://bugs.debian.org/1003153
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/852
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-17 09:16:20 +00:00
Craig Small
071935b914 Update apache2-common so that other processes can trace the hats that include this file. The main includes abstractions/base which has these lines in it, which is why ss -tnlp sometimes fails.
See also: https://bugs.debian.org/1003153 for more details about how this occurs.
2022-02-17 07:57:55 +00:00
Georgia Garcia
b6d3daa715 parser: fix building with link time optimization (lto)
Libapparmor was fixed for lto builds on commit 7cde91f5 but
the parser was also failing due to the same reasons when lto
was enabled.

Fixes: https://gitlab.com/apparmor/apparmor/-/issues/214
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
2022-02-16 20:49:42 -03:00
John Johansen
e71e27c574 Merge smbd: include snippet generated at runtime on Debian and openSUSE
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/838
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-15 21:29:02 +00:00
Christian Boltz
19abc66425
smbd: include snippet generated at runtime on openSUSE 2022-02-15 21:52:15 +01:00
John Johansen
ba14227bb5 Merge make test-aa-notify test_help_contents () less strict
Python 3.10 generates a slightly different --help output.

Fixes https://gitlab.com/apparmor/apparmor/-/issues/220

Closes #220
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/848
Acked-by: Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-15 19:17:30 +00:00
Christian Boltz
39f4132ab9
make test-aa-notify test_help_contents () less strict
Python 3.10 generates a slightly different --help output.

Fixes https://gitlab.com/apparmor/apparmor/-/issues/220
2022-02-15 19:39:32 +01:00
John Johansen
583e1905e9 Merge profile-load: use safer and less ambiguous shell constructs
Thanks to @cboltz for noticing this on https://gitlab.com/apparmor/apparmor/-/merge_requests/841#note_842436025.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/849
Acked-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-15 08:16:38 +00:00
intrigeri
322b3f4d3e profile-load: use less ambiguous if/then construct
As shellcheck taught me
today (https://github.com/koalaman/shellcheck/wiki/SC2015),
"A && B || C is not if-then-else. C may run when A is true".

It does not matter here in practice, because worst case we would run "true" once
too many, but still.
2022-02-15 07:34:17 +00:00
intrigeri
35f23a6da1 profile-load: use safer "read" construct
In this case it does not matter, we're merely testing if we can actually
read from that file, but let's make this robust (and shellcheck happy)
for future's sake.

Reference: https://www.shellcheck.net/wiki/SC2162
2022-02-15 07:28:27 +00:00
John Johansen
5a41024bbe Merge Make the systemd unit a no-op in containers with no internal policy
In 73e124d4fb I've upstreamed the `is_container_with_internal_policy()` function, but so far it was not used anywhere upstream. This is the missing bit.

I could trace the history of that patch back to 2012 (2.7.102-0ubuntu3):

    * debian/apparmor.init: do nothing in a container.  This can be
      removed once stacked profiles are supported and used by lxc.
      (LP: #978297)

Context: I lack both knowledge and motivation to keep maintaining this as part of the Debian delta. I'd rather see upstream, and in particular folks more knowledgeable than me about LXC/LXD, or with external motivation factors to work on this part of the stack, take care of it.

Note: Debian has similar code in its [sysvinit script](https://salsa.debian.org/apparmor-team/apparmor/-/blob/debian/master/debian/apparmor.init). I'm not touching that one.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/840
Acked-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-14 22:11:28 +00:00
John Johansen
26b7ddee36 Merge Allow access to socket directory used by recent ibus-daemon
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/837
Acked-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-14 22:07:19 +00:00
John Johansen
4300953dc8 Merge CI: enable Secret-Detection and a few SAST analyzers
This MR depends on !843, mostly for convenience and to avoid having to rework it once !843 is merged. If this turns out to be a blocker, I can rebase it `--onto` master.

It's based on the draft from !584 and !716, but on top of copying'n'pasting the examples from the GitLab documentation, which was necessary but not sufficient, in this MR I tried my best to make these features work in our context: it actually passes CI, it does not clutter the CI UI with jobs that are not applicable here, and it yields a manageable amount of output (as opposed to hundreds of "OMG you're using format strings", that I don't think any of us is going to triage one by one any time soon).

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/844
Acked-by: Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-14 08:38:33 +00:00
John Johansen
6f0e361c8e Merge CI: parallelize across multiple jobs, only install necessary dependencies
This makes the pipeline run time about 30% shorter and, perhaps more importantly, this gives us more direct access to test failures: they are not hidden in the middle of the huge `test-all` log anymore.

As a bonus, this gives us much faster feedback for tests with a short duration.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/843
Acked-by: Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-14 08:17:47 +00:00
John Johansen
6a54d59172 Merge Import profile-load script from Debian
This script is used at least by LXC upstream and MySQL in Debian:
https://codesearch.debian.net/search?q=%2Flib%2Fapparmor%2Fprofile-load

Presumably it could be useful elsewhere if it was more readily available.

Similarly to !840, this is another user of the `is_container_with_internal_policy()` function. I'd like all the callers of this function to live in harmony under the same roof, upstream.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/841
Acked-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-14 06:23:51 +00:00
John Johansen
046442741f Merge Add .desktop file for aa-notify
This allows distributions to start aa-notify automatically, should they wish so, by installing that file in a suitable location, such as `/etc/xdg/autostart`.

This file was introduced in Ubuntu 2.8.95~2430-0ubuntu3 package in 2014, replacing the `/etc/X11/Xsession.d` snippet that Ubuntu had added in 2010.

I'd like to stop having to care about this file as part of the Debian delta and to enable greater collaboration.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/839
Acked-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
2022-02-14 06:21:22 +00:00
Christian Boltz
1bb684b474 Merge shellcheck: skip files generated during libapparmor build
libtool generates horrible shell code, you don't want to see the
shellcheck results for it ;-)

This is only relevant for local testing (not in CI) because these files
don't exist in git and therefore don't exist when the shellcheck CI job
runs.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/847
Acked-by: Approved-by: intrigeri <intrigeri@boum.org>
Merged-by: Christian Boltz <apparmor@cboltz.de>
2022-02-13 21:21:37 +00:00
intrigeri
bf4134e705 Provide examples of how to disable a Bandit SAST analyzer warning 2022-02-13 21:05:54 +00:00
intrigeri
8c8965a720 CI: don't run the Bandit SAST analyzer on our test suites
Let's focus for now on code that runs on our users' systems.
2022-02-13 21:05:54 +00:00
intrigeri
086fb04f21 CI: disable SemGrep SAST analyzer
It runs the flawfinder checks, so let's disable this one for the same reason
we disabled flawfinder.
2022-02-13 21:05:54 +00:00
intrigeri
9bbbcd8447 CI: disable ESLint SAST analyzer
We have no JavaScript code, let's make the GitLab CI user interface leaner.
2022-02-13 21:05:54 +00:00
intrigeri
8655cf162a CI: disable flawfinder SAST analyzer
It reports hundreds of issues, lots of them with critical severity.
The GitLab UI allows dismissing them one-by-one very quickly,
but I'm not a good person to do that.
Let's try to have a better signal/noise ratio for this first iteration.
2022-02-13 21:05:54 +00:00
intrigeri
9240e12e73 CI: disable Dependency Scanning
According to
https://docs.gitlab.com/ee/user/application_security/dependency_scanning/,
"dependency scanning lets you know if your application uses an external (open
source) library that is known to be vulnerable".

AppArmor is not the kind of project that benefits from it: we don't link
statically against our dependencies, nor bundle them into released
artifacts.
2022-02-13 21:05:54 +00:00
intrigeri
7d69e55074 CI: disable spotbugs SAST analyzer
It requires building our Ant projects, which have not been touched in years.
2022-02-13 21:05:54 +00:00
intrigeri
9f8c0d25e3 CI: only run Debian'ish commands on jobs run on Debian'ish systems 2022-02-13 21:05:52 +00:00
Eric Rosenberg
2a7bd3aa9e CI: enable SAST, Secret-Detection, and Dependency Scanning 2022-02-13 21:04:18 +00:00
intrigeri
099f99a395 CI: ensure test-utils runs all intended tests 2022-02-13 21:02:58 +00:00
intrigeri
368625a9d3 utils/test/README.md: document not-totally-obvious cross-tree semi-dependency
Thanks to @cboltz for the explanation.
2022-02-13 21:02:58 +00:00
intrigeri
f0ff344e2a CI: normalize indentation 2022-02-13 21:02:58 +00:00
intrigeri
694b3348da CI: parallelize across multiple jobs, only install necessary dependencies 2022-02-13 21:02:56 +00:00
intrigeri
3c1163825b CI: don't install unneeded python-all-dev (Python 2) 2022-02-13 21:01:24 +00:00
Christian Boltz
6ae7b1566c
shellcheck: skip files generated during libapparmor build
libtool generates horrible shell code, you don't want to see the
shellcheck results for it ;-)

This is only relevant for local testing (not in CI) because these files
don't exist in git and therefore don't exist when the shellcheck CI job
runs.
2022-02-13 21:41:54 +01:00
Christian Boltz
73c24a8b12 Merge Lint shell code and add shellcheck CI job
This should avoid unconsciously introducing regressions wrt. best practices for shell code.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/842
Acked-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
2022-02-13 20:06:55 +00:00
intrigeri
56dd267a24 Mark variables as dummy in a way that shellcheck 0.7.0 supports
The "_" prefix is only supported in shellcheck > 0.7.2.
2022-02-13 19:43:48 +00:00
intrigeri
529d386775 CI: enable all shellcheck severity levels
The few previous commits make this pass, let's profit.
2022-02-13 19:43:48 +00:00
intrigeri
6db9ebfd15 aa-decode: "fix" all remaining shellcheck style violations 2022-02-13 19:43:48 +00:00
intrigeri
985f9ca788 CI: set shellcheck minimum severity to info
The few previous commits make this pass, let's profit.
2022-02-13 19:43:48 +00:00
intrigeri
825f761c77 aa-decode, aa-remove-unknown: fix remaining shellcheck info-level violations
For details, see:

- https://www.shellcheck.net/wiki/SC2086
- https://www.shellcheck.net/wiki/SC2162
2022-02-13 19:43:48 +00:00
intrigeri
e55a9b3735 CI: set shellcheck minimum severity to warning
The few previous commits make this pass, let's profit.
2022-02-13 19:43:48 +00:00
intrigeri
af76d98fce utils/test/: drop support for running with Python 2 2022-02-13 19:43:48 +00:00
intrigeri
8d219e1f31 aa-remove-unknown: mark dummy variable as such
For details, see https://www.shellcheck.net/wiki/SC2034.
2022-02-13 19:43:48 +00:00
intrigeri
cfcc271b3c aa-remove-unknown: prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
For details, see https://www.shellcheck.net/wiki/SC2166.
2022-02-13 19:43:48 +00:00