Apparmor support #286

sre · 2022-03-18T18:46:23Z

Hi,

This adds support for AppArmor using libapparmor and the Ubuntu kernel patch
requested in #169. For the implementation dbus-daemon has been used as reference.
The patchset handles restriction of all common DBus operations (sending, receiving,
bus owning and monitoring) - just like dbus-daemon.

Note, that the downstream kernel patch is no longer limited to Ubuntu. It has been
applied to some kernels used in the embedded sector. Missing support for AppArmor
is the limiting factor to switch from dbus-daemon to dbus-broker on these embedded
systems and the reason I wrote this code.

Regarding the status of the kernel patch required for this support: The main reason,
that the kernel is still missing support for kernel based af_unix/dbus meditation is a
pending code restructuring that got postponed for multiple years. Current expectation
is, that this restructuring finally happens in the 5.19 cycle (fingers crossed).

-- Sebastian

dvdhrm

Thanks a lot for doing this! Some basic questions on the code inline, but it generally it looks good. I intend to merge this when I find the time to properly review this. But it will not make the next release, as I really need to tag that as soon as possible.

The second patch is quite big and could easily be split in lots of smaller ones. This would make reviewing a lot easier and make me more confident I don't miss anything. I can pick the patch apart myself and apply the changes individually, if you prefer. But if you want to do that yourself, please go ahead. I don't mind either way.

Again, thanks for looking into this!

dvdhrm · 2022-05-06T07:51:36Z

src/util/apparmor.c

+        const char *mode; /* AppArmor confinement mode (freed by freeing *label) */
+};
+
+static struct BusAppArmorConfinement *bus_con = NULL;


Any particular reason this is a global variable rather than part of struct Bus? For SELinux we need the global context, because they did not provide any context object in libselinux. But if possible, I would avoid any *_init_global() functions.

dvdhrm · 2022-05-06T08:02:34Z

src/bus/policy.h

@@ -162,16 +166,21 @@ int policy_snapshot_dup(PolicySnapshot *snapshot, PolicySnapshot **newp);
 int policy_snapshot_check_connect(PolicySnapshot *snapshot);
 int policy_snapshot_check_own(PolicySnapshot *snapshot, const char *name);
 int policy_snapshot_check_send(PolicySnapshot *snapshot,
-                               const char *subject_context,
+                               const char *sender_context,


The sender context should be taken from snapshot (see snapshot->seclabel). Otherwise, you will query the context too late. Note that the point of the policy-snapshot is to snapshot all sender information at a particular time, and then later check whether it matches. This is used for activation messages where sender and receiver information is not available simultaneously. If this does not work for apparmor, this needs to be somehow documented.

dvdhrm · 2022-05-06T08:08:48Z

src/util/apparmor.c

+
+        /* XXX: we don't have access to any context, so can't find
+         * the right UID to use, follow dbus-daemon(1) and use our
+         * own. */


Why can't we pass the context in? I don't understand this comment. Which context would be needed?

I took over that function incl. the comment from the dbus-broker SELinux implementation without investigating further (and adapted it for apparmor).

sre · 2022-05-06T15:34:10Z

Thanks for having a look at it. I will check the things you pointed out next week and also try to figure out a way to split the second patch into smaller chunks.

Conan-Kudo · 2022-05-10T12:58:10Z

@sre Is there a link to the patch review on lore for the revised patch slated for 5.19 yet?

sre · 2022-05-11T17:43:26Z

@sre Is there a link to the patch review on lore for the revised patch slated for 5.19 yet?

No, not that I'm aware of.

dvdhrm

So I pulled in most of the preparatory patches, and I adjusted some bits. The major apparmor.c code is still missing. I am working on it.

I commented on all things I changed. Let me know if something is unclear, or if I unintentionally broke something.

dvdhrm · 2022-05-17T11:30:00Z

meson.build

+if use_apparmor
+        dep_libapparmor = dependency('libapparmor', version: '>=3.0')
+endif
+


I moved this above the audit-check, since apparmor is before audit in alphabetical order ;)

dvdhrm · 2022-05-17T11:30:13Z

meson_options.txt

@@ -1,3 +1,4 @@
+option('apparmor', type: 'boolean', value: false, description: 'AppArmor support')
 option('audit', type: 'boolean', value: false, description: 'Audit support')


Here it was in the correct order :)

dvdhrm · 2022-05-17T11:31:21Z

src/bus/driver.c

+        case POLICY_E_SELINUX_ACCESS_DENIED:
+                return BUS_LOG_POLICY_TYPE_SELINUX;
+        default:
+                return BUS_LOG_POLICY_TYPE_INTERNAL;


I did some re-ordering all over the place to stay consistent with the order the enum is defined. So first check INTERNAL (if it applies), then SELINUX, and APPARMOR last. Not a big deal, just for consistency.

dvdhrm · 2022-05-17T11:33:05Z

src/bus/driver.c

+                case POLICY_E_ACCESS_DENIED:
+                case POLICY_E_APPARMOR_ACCESS_DENIED:
+                case POLICY_E_SELINUX_ACCESS_DENIED:
+			{


I know we have a mixed style of switch and if clauses, but I think I prefer the if-clause here. I fixed it up everywhere. I also added the same errno-check to all instances of policy_snapshot_check_{send,receive}, you missed one in driver.c. I also changed it to always check for SELINUX as well. Technically, SELINUX only checks for SEND, not RECEIVE, but the callsite should be consistent, imho. No reason to be overly pedantic there, it just makes it a lot harder to argue about it, and nothing really technically prevents us from adding selinux checks to recv as well.

src/bus/policy.c

dvdhrm · 2022-05-17T11:36:39Z

src/launch/policy.h

@@ -114,6 +115,7 @@ struct Policy {
                .gid_tree = C_RBTREE_INIT,                                              \
                .selinux_list = C_LIST_INIT((_x).selinux_list),                         \
                .apparmor_mode = CONFIG_APPARMOR_ENABLED,                               \
+                .bus_type = CONFIG_BUS_TYPE_SYSTEM,                                     \


Is system actually the default? The man-page doesn't mention it, and I didn't check the dbus sources. Did you?

No, I did not check when writing this and totally forgot about it. I just did check dbus-daemon. Apparently it supports arbitrary bus types incl. the bus type being empty and then has special handling for "session" and "system". So parsing needs to go and instead the string should be used directly.

Yeah, I think we need to avoid parsing it and just send it through, indeed.

dvdhrm · 2022-05-17T11:37:38Z

src/util/apparmor.c

+        if (f) {
+                errno = 0;
+                if (!fgets(buffer, sizeof(buffer), f)) {
+                        if (ferror(f) && errno != EINVAL)


I assume this EINVAL check was copy-pasted. Looks like an error. I dropped it. If it was intentional, let me know! I also dropped it from the old is_enabled() code.

sre · 2022-05-17T17:04:14Z

So I pulled in most of the preparatory patches, and I adjusted some bits. The major apparmor.c code is still missing. I am working on it.

Thanks, the changes look fine to me.

Let me know if something is unclear, or if I unintentionally broke something.

I just rebased the pull-request to current HEAD dropping the merged patches (no other changes) and the branch is still working. I will look into the CI issues and the bus_type problem.

dvdhrm · 2022-05-18T09:51:00Z

src/bus/policy.h

@@ -173,6 +173,8 @@ int policy_snapshot_check_send(PolicySnapshot *snapshot,
                               bool broadcast,
                               size_t n_fds);
 int policy_snapshot_check_receive(PolicySnapshot *snapshot,
+                                  const char *sender_seclabel,
+                                  const char *receiver_seclabel,


snapshot->seclabel is equivalent to receiver_seclabel. You only need to pass in sender_seclabel. See *_check_send() and the SELinux path for an example.

I dropped it myself and pushed the patch!

dvdhrm

A bunch more comments inline. It looks good! I think we can mostly take it that way, just some minor corrections needed.

Thanks a lot! I think I now got a full overview of the thing, and I am generally not opposed at all to merging this.

src/bus/driver.c

src/bus/policy.c

dvdhrm · 2022-05-18T11:11:06Z

src/bus/policy.c

@@ -1082,6 +1114,16 @@ int policy_snapshot_check_receive(PolicySnapshot *snapshot,
                                  size_t n_fds) {
        PolicyVerdict verdict = POLICY_VERDICT_INIT;
        size_t i;
+        int r;
+
+        r = bus_apparmor_check_xmit(snapshot->apparmor, false, sender_seclabel, receiver_seclabel,


This is now: subject_seclabel, snapshot->seclabel, rather than sender_seclabel, receiver_seclabel

dvdhrm · 2022-05-18T11:19:44Z

src/util/apparmor.c

+int bus_apparmor_set_bus_type(BusAppArmorRegistry *registry, const char* bustype) {
+        registry->bustype = strdup(bustype);
+        if (!registry->bustype)
+                return -ENOMEM;


return error_origin(-ENOMEM)

dvdhrm · 2022-05-18T11:22:52Z

src/util/apparmor.c

+                r = -EINVAL;
+        }
+
+        return r;


Uff. This is slow. I mean, we optimized everything so we have a single allocation per message transaction. Now the apparmor code adds one allocation per name the receiver and sender own. I am fine keeping the code, but man those LSMs really drag things down. Makes me a bit sad :-/

dvdhrm · 2022-05-18T11:23:40Z

src/util/apparmor.c

+                return 0;
+
+        if (!condup)
+                return -ENOMEM;


error_origin(-ENOMEM), and I would suggest moving the strdup() out of the variable declaration.

dvdhrm · 2022-05-18T11:24:54Z

src/util/apparmor.c

+                return error_origin(-errno);
+
+        if (audit)
+                bus_apparmor_log("apparmor=\"%s\" operation=\"dbus_bind\" bus=\"%s\" name=\"%s\"", allowstr(allow), registry->bustype, name);


bustype can contain quotes ", right? At least if we take it unmodified from the configuration. Probably doesn't matter much, though.

dvdhrm · 2022-05-18T11:26:24Z

src/util/apparmor.h

+enum {
+        _APPARMOR_E_SUCCESS,
+
+        APPARMOR_E_DENIED,


Can you prefix these with BUS_APPARMOR_*? The reason for the BUS_* prefix in selinux was that we do not want to conflict with selinux names, which sometimes start with selinux_*. I think the same is warranted for apparmor for consistency, even if the library uses aa_* as prefix (otherwise we should really drop it from the functions as well).

dvdhrm

I now pushed this with a bunch of fixups on top. This still lacks the last commit (handling apparmor in the launcher, though). Note that I am not really sure that this currently matches the behavior of dbus-daemon. I will try to align it with dbus-daemon later. In the meantime, if you have any comments on the code I pushed, let me know. I tried to implement everything that I mentioned inline in this PR.

dvdhrm · 2022-07-08T11:16:26Z

src/bus/driver.c

+        if (r)
+                return r;


Suggested change

if (r)

return r;

else if (r)

return error_fold(r);

Crossing error-namespaces, so this needs to be folded.

dvdhrm · 2022-07-08T11:17:09Z

src/bus/policy.c

+                r = bus_apparmor_set_bus_type(registry->apparmor, bustype ? bustype : "");
+                if (r)
+                        return error_trace(r);


Suggested change

r = bus_apparmor_set_bus_type(registry->apparmor, bustype ? bustype : "");

if (r)

return error_trace(r);

r = bus_apparmor_set_bus_type(registry->apparmor, bustype);

if (r)

return error_fold(r);

bustype cannot be NULL, and the error needs to be folded, as this is a different error-namespace.

I think this entire construct needs to drop the if (apparmor) and then use apparmor ? bustype : NULL as argument to set_bus_type(), right?

dvdhrm · 2022-07-08T11:26:37Z

src/launch/config.c

-                        state->current->bus_type.type = CONFIG_BUS_TYPE_SESSION;
-                else
-                        CONFIG_ERR(state, "Unknown bus type", ": %s", state->current->cdata);
+                state->current->bus_type.type = strdup(state->current->cdata);


This needs to be freed in config_node_free().

dvdhrm · 2022-07-08T11:27:05Z

src/util/apparmor-fallback.c

@@ -11,6 +11,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include "util/apparmor.h"
+#include "util/error.h"


Spurious include that is not needed, right?

dvdhrm · 2022-07-08T11:27:44Z

src/util/apparmor.c

@@ -93,3 +105,446 @@ int bus_apparmor_dbus_supported(bool *supportedp) {
        *supportedp = supported;
        return 0;
 }
+
+static inline bool is_apparmor_enabled(BusAppArmorRegistry *registry) {
+        return registry->bustype ? true : false;


Suggested change

return registry->bustype ? true : false;

return registry->bustype;

dvdhrm · 2022-07-12T10:09:51Z

src/util/apparmor.c

+}
+
+static bool is_unconfined(const char *context) {
+        return !strcmp(context, "unconfined");


dbus-daemon checks for the mode to be equal to unconfined, or unset and the label equal to unconfined. Any particular reason this is different?

dvdhrm · 2022-07-12T10:10:36Z

src/util/apparmor.c

+                                                                interface, method, allow, &audit_tmp);
+                                if (r < 0)
+                                        return r;
+                                if (!allow)


Shouldn't this be if (!*allow)?

dvdhrm · 2022-07-12T10:10:50Z

src/util/apparmor.c

+                                if (!allow)
+                                        return r;
+                                if (audit_tmp)
+                                        *audit = 1;


Shouldn't this conditional be before checking allow?

dvdhrm · 2022-07-12T10:11:35Z

src/util/apparmor.c

+
+        if (!is_apparmor_enabled(registry))
+                return 0;
+


Why does this function not check against unconfined like the others and dbus-daemon do?

dvdhrm · 2022-07-12T10:13:06Z

src/util/apparmor.c

+ *         or a negative error code on failure.
+ */
+int bus_apparmor_check_xmit(BusAppArmorRegistry *registry,
+                            bool check_send,


dbus-daemon only ever checks on send, why do we have two calls one for send and one for receive?

Maybe this is a confusion where dbus-daemon checks both for sending and receival in a single call, but that is not the case. Checking at send and receive is different to checking the receiver during send. I think this should be bus_apparmor_check_send() just like selinux does.

dvdhrm · 2022-07-13T07:10:01Z

I am quite confused now where you got the transmission checks from. dbus-daemon simply checks with the destination-string and source-unique-id, but your code iterates the set of names on the destination. This will very easily break existing AppArmor rules. I also don't understand why that change was done. Is there any reason not to imitate what dbus-daemon does?

That is, I would make apparmor check whether the sender is allowed to send to the destination used in the message. Then check whether the receiver is allowed to receive from the unique-id of the sender. If either is false, refuse the transmission.

I can implement this, but I want to make sure you agree on that, and that this is indeed the intended behavior?

setharnold

Hello, a few small thoughts; it's more of a quick skim than in-depth review, and while I'm alright with AppArmor, I know next to nothing about dbus-broker.

Thanks

setharnold · 2022-08-10T00:08:08Z

src/launch/launcher.c

+        } else if (*apparmor_mode == CONFIG_APPARMOR_REQUIRED) {
+                if (!enabled || !supported) {
+                        fprintf(stderr, "AppArmor required, but not supported. Exiting.\n");
+                        *apparmor_mode = CONFIG_APPARMOR_REQUIRED;


I expected to see a negative return value here, or perhaps an exit(3) or _exit(2) call.

exit is handled outside of this function on line 1250

perhaps the fprintf on line 1196 should move between lines 1250 and 1251 ?

setharnold · 2022-08-10T00:10:11Z

src/bus/policy.c

+        if (r) {
+                if (r == BUS_APPARMOR_E_DENIED)
+                        return POLICY_E_APPARMOR_ACCESS_DENIED;
+


Are there other return values that mean "don't grant access"? eg. "internal error" or "silently deny" or similar? This currently feels like a 'fail open' way to write this check. (And two more later.)

This is a translation of error codes from one domain to another. If we happen to forget converting one, the result is the following error_fold(), which takes any unhandled error and turns it into a fatal error code. Hence, the worst that can happen is that we end up exiting, rather than just refusing the message-transaction. Not good, but also we do not accidentally fail the security check.

setharnold · 2022-08-10T00:16:51Z

src/util/apparmor.c

+
+        if (is_complain(security_mode))
+                allow = 1;
+


Should this allow = 1 block be moved above the bus_apparmor_log() call, so that the allowstr(allow) generates an output that matches the decision?

Sounds good!

setharnold · 2022-08-10T00:30:38Z

src/util/apparmor.c

+                bus_apparmor_log("apparmor=\"%s\" operation=\"dbus_eavesdrop\" bus=\"%s\" label=\"%s\"", allowstr(allow), registry->bustype, context);
+
+        if (is_complain(security_mode))
+                allow = 1;


A similar question here, should this be moved above the logging line, so the string matches the result?

gegarcia · 2022-12-12T14:29:03Z

src/util/apparmor.c

+
+        if (src_audit) {
+                bus_apparmor_log(registry,
+                        "apparmor=\"%s\" bus=\"%s\" path=\"%s\" interface=\"%s\" method=\"%s\" "


I realize the previous code already did this but the AppArmor log parsing tools expects "method" to be "member" instead

I created a PR in AppArmor so we can parse method the same way as member so no change is needed here :)

gegarcia · 2022-12-12T14:34:32Z

src/util/apparmor.c

+        if (src_audit) {
+                bus_apparmor_log(registry,
+                        "apparmor=\"%s\" bus=\"%s\" path=\"%s\" interface=\"%s\" method=\"%s\" "
+                        "label=\"%s\" peer_label=\"%s\"",


I believe the denied mask is missing. It should look like
mask="send" for when send is denied and
mask="receive" for when receive is denied.

name="" is also missing

Ah, I also noticed that operation="dbus_method_call" is missing here

Thanks for the review, I added mask and operation. Logging name is non-trivial, since dbus-broker has a NameSet possibly containing multiple names.

Checking if the destination is allowed to receive during send time is not the same as checking if a message is allowed to be received at receive time. Fix this by following the dbus-daemon implementation more closely and handle everything in bus_apparmor_check_send(). Reported-by: David Rheinsberg <[email protected]> Fixes: 8b1e969 ("util/apparmor: add apparmor support") Signed-off-by: Sebastian Reichel <[email protected]>

Log failing apparmor checks as accepted when in complain mode for consistency. Reported-by: Seth Arnold <[email protected]> Fixes: 8b1e969 ("util/apparmor: add apparmor support") Signed-off-by: Sebastian Reichel <[email protected]>

All required AppArmor support code has landed, so enable it in the launcher. Signed-off-by: Sebastian Reichel <[email protected]>

Improve logging for compatibility with AppArmor log parsing tools: * Add denied mask to the send/receive/bind log messages * Add operation to the send/receive log messages Reported-by: Georgia Garcia <[email protected]> Signed-off-by: Sebastian Reichel <[email protected]>

gegarcia · 2023-03-09T13:34:49Z

From an AppArmor perspective, this LGTM. We updated our regression tests upstream and they are passing with these patches.

In the [merge request that adds AppArmor support on D-Bus Broker](bus1/dbus-broker#286), the word "method" is used instead of "member" on the auditing logs. So we are adding support to parse "method" the same way as "member" on D-Bus audit logs. MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/958 Approved-by: John Johansen <[email protected]> Merged-by: John Johansen <[email protected]>

In the [merge request that adds AppArmor support on D-Bus Broker](bus1/dbus-broker#286), the word "method" is used instead of "member" on the auditing logs. So we are adding support to parse "method" the same way as "member" on D-Bus audit logs. MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/958 Approved-by: John Johansen <[email protected]> Merged-by: John Johansen <[email protected]> (cherry picked from commit a96fa35) Signed-off-by: John Johansen <[email protected]>

eslerm

Hi @dvdhrm o/

@gegarcia and @setharnold from AppArmor have ACKd this PR

there is one outstanding review request that might be resolved by moving a line

eslerm · 2023-06-12T18:22:35Z

src/launch/launcher.c

+        } else if (*apparmor_mode == CONFIG_APPARMOR_REQUIRED) {
+                if (!enabled || !supported) {
+                        fprintf(stderr, "AppArmor required, but not supported. Exiting.\n");
+                        *apparmor_mode = CONFIG_APPARMOR_REQUIRED;


exit is handled outside of this function on line 1250

perhaps the fprintf on line 1196 should move between lines 1250 and 1251 ?

evverx · 2023-06-16T15:04:59Z

As far as I can tell the CI is currently unaware of AppArmor in the sense that -Dapparmor=true isn't passed to meson anywhere. At least CodeQL analyzes the apparmor fallback only as far as I can see because libapparmor-dev isn't installed there and the "autobuild" job can't figure out what to do. The other CI workflow passes -Daudit=true only. There it would probably make sense to build dbus-broker with both -Dapparmor=true and -Dapparmor=false to make sure it compiles fine with -Werror with and without libapparmor.

(I'm not sure it should be fixed here but I think it should be fixed one way or another).

andrewfader · 2024-01-11T07:21:44Z

looks like arch is adopting dbus-broker as default. any chance this can be merged?

Maryse47 · 2024-01-13T16:13:22Z

Arch kernels don't have apparmor dbus mediation patches so included so this is irrelevant.

andrewfader · 2024-01-13T17:53:18Z

Arch kernels don't have apparmor dbus mediation patches so included so this is irrelevant.

I don't see how that can be, but I'm not an expert. Per https://wiki.archlinux.org/title/AppArmor I have it enabled and running, a service loaded, a kernel option enabled, aa-enabled returns true, I see "DENIED" in my logs sometimes, etc.

dvdhrm · 2024-01-15T15:29:55Z

The required AppArmor features to get D-Bus message security policies working are not part of upstream linux kernels. Basic AppArmor support has been included in upstream kernels, though.

So to get AppArmor working with dbus-broker, you would definitely need downstream patches. Ubuntu applies them to all their kernel builds.

sre force-pushed the apparmor-support branch from d0c2b01 to 92e9e23 Compare March 21, 2022 17:42

dvdhrm requested changes May 6, 2022

View reviewed changes

sre force-pushed the apparmor-support branch from 92e9e23 to 5d65d82 Compare May 11, 2022 17:35

dvdhrm reviewed May 17, 2022

View reviewed changes

sre force-pushed the apparmor-support branch from 5d65d82 to 1a2f8da Compare May 17, 2022 16:59

sre force-pushed the apparmor-support branch from 1a2f8da to 7833ece Compare May 17, 2022 17:11

dvdhrm reviewed May 18, 2022

View reviewed changes

dvdhrm requested changes May 18, 2022

View reviewed changes

sre force-pushed the apparmor-support branch from 7833ece to f1b5264 Compare May 18, 2022 18:18

dvdhrm requested changes Jul 12, 2022

View reviewed changes

setharnold reviewed Aug 10, 2022

View reviewed changes

sre force-pushed the apparmor-support branch from f1b5264 to 158e879 Compare August 11, 2022 12:34

sre force-pushed the apparmor-support branch from 158e879 to 3372194 Compare August 25, 2022 11:33

gegarcia reviewed Dec 12, 2022

View reviewed changes

sre added 4 commits December 22, 2022 16:00

launch/launcher: enable AppArmor support

75a9eec

All required AppArmor support code has landed, so enable it in the launcher. Signed-off-by: Sebastian Reichel <[email protected]>

sre force-pushed the apparmor-support branch from 3372194 to 45fa235 Compare December 22, 2022 16:45

eslerm approved these changes Jun 12, 2023

View reviewed changes

LDprg mentioned this pull request Apr 3, 2024

Use dbus broker per default NixOS/nixpkgs#299476

Open

		@@ -1,3 +1,4 @@
		option('apparmor', type: 'boolean', value: false, description: 'AppArmor support')
		option('audit', type: 'boolean', value: false, description: 'Audit support')

-        if (r)
-                return r;
+        else if (r)
+                return error_fold(r);

	return registry->bustype ? true : false;
	return registry->bustype;

Apparmor support #286

Are you sure you want to change the base?

Apparmor support #286

Conversation

sre commented Mar 18, 2022

dvdhrm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sre commented May 6, 2022

Conan-Kudo commented May 10, 2022

sre commented May 11, 2022

dvdhrm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sre commented May 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvdhrm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvdhrm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvdhrm commented Jul 13, 2022

setharnold left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sre Dec 22, 2022 • edited Loading

Choose a reason for hiding this comment

gegarcia commented Mar 9, 2023

eslerm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evverx commented Jun 16, 2023

andrewfader commented Jan 11, 2024

Maryse47 commented Jan 13, 2024

andrewfader commented Jan 13, 2024

dvdhrm commented Jan 15, 2024

sre Dec 22, 2022 •

edited

Loading