Add firewall section to the networking page #8032

larskarlitski · 2017-11-05T01:40:06Z

First look at a what a firewall module might look like.

It operates on firewalld's default zone and there are no plans to expose zones, as they don't seem to be used very often on servers.

The delete buttons don't work yet, but the UI updates itself when adding and removing services from the default zone with one of:

sudo firewallctl zone '' remove service ssh
sudo firewallctl zone '' add service ssh

This will implement the minimal version of the design and iterate in later pull requests.

show the firewall section on the default page when firewalld is installed
show the list of services that are currently active
allow to remove a service
allow to add one of firewalld's predefined services
tests
add doc/guide page
feature video for release notes: https://youtu.be/yR5riNcIDbc

garrett · 2017-12-05T09:50:49Z

Related links:

design page: https://github.com/cockpit-project/cockpit/wiki/Feature:-Firewall
issue: RFE: Firewall integration #1094

q2dg · 2017-12-05T12:20:11Z

Ei, there's any still-alpha firewall module available to download for trying it? Thanks!

larskarlitski · 2017-12-22T16:52:00Z

First update towards @garrett's design: move the page over to the network page.

Small bug (in addition to all the stuff missing): cockpit.location.go() in the breadcrumb doesn't do what it should.

larskarlitski · 2018-02-07T16:31:29Z

Quick update: make the breadcrumb work correctly, hook up the switch to starting/stopping firewalld.service, and hide the whole section if firewalld is not installed.

larskarlitski · 2018-03-07T13:42:19Z

Update with a refactor, because my initial approach of talking to firewalld turned out to make too many D-Bus calls and cause too many re-renders. Removing services is now supported. Also lots of tiny fixes to get it close to the design.

andreasn · 2018-03-08T13:30:47Z

pkg/networkmanager/firewall.jsx

+        return (
+            <div className="container-fluid page-ct">
+                <ol className="breadcrumb">
+                    <li><a onClick={go_up}>{_("Network")}</a></li>


Just a flyby comment that this is called "Networking" in the menu and on the breadcrumb on the other sub-pages.

Indeed. Thanks for the catch! It'll be fixed with the next push.

larskarlitski · 2018-03-08T18:45:01Z

Update with minor fixes pointed out by @martinpitt and @andreasn. Also add a first version of the "Add Services" dialog. It's still missing the search box and some layout fixes to qualify for this PR, so leaving WIP.

Depends on #8794 now.

larskarlitski · 2018-03-12T21:13:32Z

Fix a couple of things we discussed on IRC:

"N Active Rules" on the main networking page is now clickable (and blue on hover)
services on the firewall page and in the "Add Services" dialog are now (actually) sorted alphabetically
clicking anywhere on a row in the "Add Services" dialog toggles its checkbox
fix alignment and spacing issues

larskarlitski · 2018-03-13T21:29:15Z

Another update, which I think brings it to a state for a first round of review (removed WIP):

add search box in the Add Services Dialog
skip the tests on some images that don't have firewalld
fix networking test that used ambiguous selector now, because of the new elements

martinpitt

Thanks for working on this! The code layout and structure is very clean and easy to follow (kudos! it's a nice read), and the scope is just right for the initial landing. I have lots of nitpicks, but the vast majority should be trivial to fix.

Vagrantfile

@@ -46,6 +46,7 @@ Vagrant.configure(2) do |config|
            atomic \
            docker \
            etcd \
+            firewalld \


martinpitt · 2018-03-14T07:19:52Z

pkg/networkmanager/firewall-client.es6

+cockpit.event_target(firewall);
+
+var firewalld_service = service.proxy('firewalld');
+var firewalld_dbus = cockpit.dbus('org.fedoraproject.FirewallD1', { superuser: 'try' });


These two should be const.

superuser "try" might work because firewalld has polkit rules? But the default policy (rightfully) does not allow users to change the firewall rules without authenticating as admin. Does reading the state work as user? Then the page would be useful to non-admins, provided that permission errors are handled sanely (e. g. by not showing the "modify" buttons for non-admins).

martinpitt · 2018-03-14T07:21:46Z

pkg/networkmanager/firewall-client.es6

+var firewalld_service = service.proxy('firewalld');
+var firewalld_dbus = cockpit.dbus('org.fedoraproject.FirewallD1', { superuser: 'try' });
+
+firewalld_service.addEventListener('changed', function () {


Nitpick: you mix arrow functions and classic functions for this kind of "inline" event handlers. Arrow functions are a bit more ES6-y I think. (totally not a merge blocker, of course)

martinpitt · 2018-03-14T07:23:08Z

pkg/networkmanager/firewall-client.es6

+var firewalld_dbus = cockpit.dbus('org.fedoraproject.FirewallD1', { superuser: 'try' });
+
+firewalld_service.addEventListener('changed', function () {
+    var installed = !!firewalld_service.exists;


Isn't exists already a boolean property?

It's set to null before the first update. I've been bitten by JavaScript's weird type coercion too many times, so I've left the !!.

martinpitt · 2018-03-14T07:23:52Z

pkg/networkmanager/firewall-client.es6

+            .then(reply => {
+                var [ , name, description, ports ] = reply[0];
+
+                var info = {


Please use let in this kind of arrow functions for proper scoping.

martinpitt · 2018-03-14T08:36:06Z

test/verify/check-networking-firewall

+            raise Exception("timed out waiting for {} to become {}".format(unit, state))
+
+
+class TestNetworking(MachineCase):


Please rename to TestFirewall, to avoid confusion.

These tests need to be blacklisted on at least rhel-7-4 (where networking is in base, and thus don't get this feature). There are also some images (like the Debians) which don't have firewalld, I suggest the test skips them for now.

martinpitt · 2018-03-14T08:39:51Z

test/verify/check-networking-firewall

+        wait_unit_state(m, "firewalld", "active")
+
+        active_rules = m.execute("firewallctl zone '' list services").split()
+        b.wait_in_text("#networking-firewall-summary", "{} Active Rules".format(len(active_rules)))


Corner case: Use "Rule" here instead of "Rules", for the corner case that there is really just one. However, our images should have at least ssh and cockpit, so I guess it's fine.

Wow, awesome catch!

martinpitt · 2018-03-14T08:40:34Z

test/verify/check-networking-firewall

+        b.wait_present("#networking-firewall-switch label.active:not(.disabled)")
+        b.wait_in_text("#networking-firewall-switch label.active", "Off")
+        wait_unit_state(m, "firewalld", "inactive")
+        b.wait_in_text("#networking-firewall-summary", "0 Active Rules")


This behaviour feels a bit odd. If the firewall is disabled, then "0 active rules" feels like "enabled, but unconfigured". Shouldn't this be hidden completely, as the firewall state is already shown as "off"?

This is as designed. I agree that we might want something better here. I'll ask @garrett.

martinpitt · 2018-03-14T08:43:10Z

test/verify/check-networking-firewall

+        b.enter_page("/network/firewall")
+
+        b.click(".breadcrumb li:first a")
+        b.enter_page("/network")


I suggest stopping and removing firewalld.service from disk (don't forget systemctl daemon-reload), and verify the expected behaviour of "not installed".

martinpitt · 2018-03-14T08:45:04Z

test/verify/check-networking-firewall

+
+        self.login_and_go("/network/firewall")
+
+        # Add a service that's probably not enabled by default anywhere


... so do the equivalent wait_not_present()?

larskarlitski · 2018-03-14T19:41:34Z

Thanks very much for the detailed review. Lots of good stuff!

To not spam you too much, I haven't commented "Fixed" on the suggestions that I accepted exactly as you wrote them.

The one thing I haven't fixed is showing the UI read-only when we're not superuser, as that's a bit more involved. How do we usually find this out? Ask polkit directly?

martinpitt · 2018-03-15T11:25:55Z

@larskarlitski : We spoke about this in IRC, but for the record: You can query https://www.freedesktop.org/software/polkit/docs/latest/eggdbus-interface-org.freedesktop.PolicyKit1.Authority.html#eggdbus-method-org.freedesktop.PolicyKit1.Authority.CheckAuthorization to see if your process is allowed to authorize. Of course you still need to know whether the user ticked the "use my password for admin tasks", I hope we store that somewhere?

larskarlitski · 2018-03-29T19:52:28Z

Added a commit that disables buttons when the current login doesn't want their password remembered. This removes superuser: try (because it always works with that).

I ended up calling pkcheck directly, because there's no way to get at the bridge's PID from cockpit, and the PolicyKit D-Bus API expects that.

This is a bit weird, because I couldn't find a way to get a signal when the "Lock" button in the menubar is clicked, in which case we'd have to rerender the UI.

martinpitt

This works well enough, thanks for that! The "On/Off" button on the /network/firewall page still needs the same "disable" treatment as "Add services..". I can't actually operate it in unprivileged mode - it stays at "off" even though the firewall is really on.

I don't see the firewall section on /network at all in unprivileged mode. I think this is right - the /firewall page is next to useless then anyway.

martinpitt · 2018-04-03T10:13:12Z

pkg/networkmanager/firewall-client.es6

@@ -120,6 +121,12 @@ firewalld_dbus.subscribe({
    firewall.dispatchEvent('changed');
 });

+cockpit.spawn(['sh', '-c', 'pkcheck --action-id org.fedoraproject.FirewallD1.all --process $$ -u 2>&1'])


A bit ugly indeed, but I can't think of a better way. If we encounter this more often in the future, we might instead query/store the bridge pid as a global variable somewhere.

I agree. If we start doing this in more places, we probably also want (internal) library functions for that.

martinpitt · 2018-04-03T10:16:51Z

pkg/networkmanager/firewall.jsx

+        var addServiceAction;
+        var classes = "btn btn-primary pull-right";
+        if (this.state.firewall.readonly)
+            addServiceAction = <button className={classes} disabled title={_("You are not authorized to modify the firewall.")}>{_("Add Services…")}</button>;


This is using the standard browser tooltips (white on black) which are "wrong". I suggest using update_privileged() for that, or if that somehow is impractical, use .tooltip('fixTitle') (but that requires jQuery) or our Tooltip react component.

update_privileged() also depends on jQuery, which I don't want to pull into firewall.jsx. I've used the React component, which unfortunately needed some minor tweaking. I've added the two commits on here as well.

martinpitt · 2018-04-03T10:18:06Z

pkg/networkmanager/firewall.jsx

@@ -65,6 +65,8 @@ function ServiceRow(props) {
            { udp.map(p => p.port).join(', ') }
        </div>,
        <button className="btn btn-danger pficon pficon-delete"
+                title={props.readonly ? _("You are not authorized to modify the firewall.") : null}


Also the wrong tooltip. Here it seems mostly irrelevant as an unprivileged user cannot even see the current firewall rules. But there is the theoretical possibility that an admin changes the polkit rules to allow that to users, but not modification.

I can see the firewall rules as an unprivileged user on fedora. Fixed as above.

larskarlitski · 2018-04-03T13:20:12Z

This works well enough, thanks for that! The "On/Off" button on the /network/firewall page still needs the same "disable" treatment as "Add services..". I can't actually operate it in unprivileged mode - it stays at "off" even though the firewall is really on.

As discussed on IRC: I can operate the button even in unprivileged mode. I guess there's something wrong with superuser: "try", but that's unrelated to this pull request.

I don't see the firewall section on /network at all in unprivileged mode. I think this is right - the /firewall page is next to useless then anyway.

I do, but I've left it the way it is for the reason above.

I've added two more commits to fix our Tooltip component, but left the extra commit for the unprivileged stuff so that it's easier for you to review. I'll squash it into the main commit after the review.

martinpitt · 2018-04-03T17:43:09Z

I can operate the button even in unprivileged mode

Actually now I see that too, not sure what was different. However, does that actually work for you? What happens here is that the firewall is shown as "off", then I can click it to "on" (but does not actually start firewalld.service), and then it moves to the disabled state. From then on it stays disabled.

Another issue is that in privileged mode the On/Off switch is a bit confusing. It takes about two seconds after clicking to actually take effect, but during that time the button stays enabled and does not change state. This feels like "I mis-clicked", and when clicking again it interferes with the first click.

Compare this with the "Automatic Updates" on/off switch on software updates. This immediately switches state, and gets disabled until the action takes place. This was the result of some involved discussions with our designers, so I think we should have the same behaviour here.

Both are minor issues, so I would be happy if these become follow-ups if you prefer. The rest looks good, great job with figuring out the tooltip issues! I confirm that the changes work well, also on the kdump/playground pages.

martinpitt · 2018-04-04T07:21:40Z

The firewall tests fail on several OSes.

larskarlitski · 2018-04-20T17:01:07Z

I've changed the switch on the firewall page to behave in the same way as the other one: it is disabled while the state change is pending.

The tests had two issues: one was a logic mixup (!= vs ==) and the other one that we need to wait for the Add Services button to become enabled before we can click it. The latter happened only after I added the read-only mode, which is the default and the button can be disabled for a few ms.

Also folded the read-only mode commit into the main one.

Sorry for the delay.

larskarlitski · 2018-04-20T17:35:02Z

Still some test failures.

larskarlitski · 2018-04-24T17:21:54Z

Finally got around to look at the failing tests. Sorry for the delay.

Don't use systemctl is-active, because of this bug. Use firewall-cmd instead of firewallctl, because the latter is not available on all systems we support (rhel-7). Also, skip the images which don't have firewalld installed.

I tried to make this work on fedora-atomic and rhel-atomic. They do have firewalld installed, but it's not enabled by default. The tests fail as soon as its enabled, because there's no hole for cockpit in their firewall. I've tried adding that but ran into more issues and decided this is ok for a follow-up.

martinpitt · 2018-04-24T17:34:48Z

Hmm, this collided with #9027. Maybe this was too aggressive and we should revert the "indent" stuff..

martinpitt · 2018-04-24T17:51:49Z

@larskarlitski : Force-pushed with running npm run eslint:fix to get the indentation right, and with capitalizing the commit messages.

larskarlitski · 2018-04-24T18:17:23Z

Ah, I rebased and fixed a conflict, it didn’t build again, because I had to run. Thanks!

martinpitt

Tests look good now, nice! There are a few outstanding issues/questions from the previous review round, and this still needs a feature page and a video or some screen shots. Getting close!

martinpitt · 2018-04-24T19:49:08Z

pkg/networkmanager/firewall.jsx

+                       rowId={props.service.id}
+                       columns={columns}
+                       tabRenderers={tabs}/>;
+}


It's fine for me, just wanted to point it out.

martinpitt · 2018-04-24T19:49:40Z

pkg/networkmanager/firewall.jsx

+            enabled: firewall.enabled,
+            services: firewall.services,
+            enabledServices: firewall.enabledServices
+        };


I'd still like to discuss the above.

apollo13 · 2018-04-25T09:51:38Z

pkg/networkmanager/firewall-client.es6

+    firewall.dispatchEvent('changed');
+});
+
+cockpit.spawn(['sh', '-c', 'pkcheck --action-id org.fedoraproject.FirewallD1.all --process $$ -u 2>&1'])


Maybe replace -u with --allow-user-interaction -- short options are nice for users to type; but not really in code imo.

Yeah that's a good point. Changed. Thanks!

larskarlitski · 2018-04-25T16:20:13Z

Added a feature page.

martinpitt · 2018-04-26T06:37:55Z

eslint spotted a bug and has another complaint:

/build/cockpit/pkg/networkmanager/firewall.jsx
  179:36  error  Unknown property 'for' found, use 'htmlFor' instead  react/no-unknown-property
  253:64  error  Arrow function should not return assignment          no-return-assign

martinpitt

Great work! I found a few small bugs and nitpicks, but this is really close now :)

martinpitt · 2018-04-26T06:39:17Z

doc/guide/feature-firewall.xml

+    services in the default zone.</para>
+
+  <para>To perform similar tasks from the command line, use
+    <ulink url="http://www.firewalld.org/documentation/man-pages/firewall-cmd.html">firewalld-cmd</ulink>:


Can we please use https:// URLs? (Same above) I checked that they work.

I had tried https://firewalld.org and that didn't work. It does with the www though. Good catch!

martinpitt · 2018-04-26T06:39:49Z

doc/guide/feature-firewall.xml

+<programlisting>
+$ <command>sudo firewall-cmd --list-services</command>
+dhcpv6-client samba-client mdns ssh cockpit
+</programlisting>


Maybe another command to enable a service? That's a very common use case.

Yeah good point.

martinpitt · 2018-04-26T06:41:08Z

pkg/networkmanager/firewall-client.es6

+     */
+            .then(function () {
+                return Array.prototype.slice.call(arguments);
+            });


It's fine like that if you prefer; but you use arrow functions everywhere else, and they are more concise.

There's a comment above these lines which explains why this is: cockpit.all returns results as individual arguments, so we need access to the arguments object, which doesn't exist in arrow functions.

Also, eslint gets the indentation wrong here. Do you know why?

I don't, I'm afraid. You mean the alignment of the });, right? If some rule gets it wrong, feel free to disable it for the time being.

martinpitt · 2018-04-26T06:43:03Z

pkg/networkmanager/firewall-client.es6

+
+    if (!firewall.enabled) {
+        firewall.services = {};
+        firewall.enabledServices = new Set();


I wonder if these two lines of initialization shouldn't happen always. Say you get an owner event while the firewall is enabled, then the code below would only add to the current state, leaving potentially obsolete old state. As long as you don't dispatch a changed in between initialization and finishing the getServices() call below, there shouldn't be any actual re-rendering (i. e. screen flicker)?

Indeed! Very nice catch, thanks.

martinpitt · 2018-04-26T06:44:52Z

pkg/networkmanager/firewall-client.es6

+
+firewall.enable = () => {
+    return cockpit.all(firewalld_service.enable(), firewalld_service.start());
+};


Arrow function with a single-line return can be simplified.

martinpitt · 2018-04-26T06:52:59Z

pkg/networkmanager/firewall.jsx

+        if (!this.state.firewall.installed) {
+            return (
+                <EmptyState title={_("Firewall is not available")} icon="fa fa-exclamation-circle">
+                    <p>{cockpit.format(_("Please install the {0} package"), "firewalld")}</p>


Hah, another nice candidate for on-demand installation :-)

martinpitt · 2018-04-26T06:57:38Z

pkg/networkmanager/interfaces.js

+            $('#networking-firewall-switch').onoff('value', firewall.enabled);
+
+            var n = firewall.enabledServices.size;
+            var summary = cockpit.ngettext('1 Active Rule', n + ' Active Rules', n);


These strings cannot be translated. You need to use ngettext("$0 Active Rule", "$0 Active Rules", n).

martinpitt · 2018-04-26T06:59:30Z

test/verify/check-networking-firewall

+def wait_unit_state(machine, unit, state):
+
+    def active_state(unit):
+        # don't use `systemctl is-active` here because of https://bugzilla.redhat.com/show_bug.cgi?id=1073481


Can you please prepend this with HACK:, so that our hacks stay easily greppable?

martinpitt · 2018-04-26T07:02:38Z

test/verify/check-networking-firewall

+    def active_state(unit):
+        # don't use `systemctl is-active` here because of https://bugzilla.redhat.com/show_bug.cgi?id=1073481
+        # outputs "ActiveState=state\n"
+        line = machine.execute("systemctl show -p ActiveState {}".format(unit))


Note: --value does not yet exist in current CentOS 7.4. Once that moves to 7.5, we can use it.

Ah I didn't know about that. Thank you!

martinpitt · 2018-04-26T07:04:11Z

test/verify/check-networking-firewall

+    wait(lambda: active_state(unit) == state, delay=0.2)
+
+
+@skipImage("firewalld not installed", "rhel-7-5", "centos-7", "fedora-atomic", "rhel-atomic", "debian-stable", "debian-testing", "ubuntu-stable", "ubuntu-1604")


Follow-up: We should definitively install it on rhel-7-5 and centos-7 at least. So on rhel-8 it's already present apparently, good!

Pass through the `className` property to the outermost div of the Tooltip component. This avoids wrapping the tooltip when using it with positioning classes (such as `pull-right`).

Use onmouseleave instead of onmouseout events, because the latter has different bubbling behavior on Chrome, which doesn't work when the containing element is floating. Also, use onmouseenter instead of onmouseover events to avoid unnecessary re-renders and make capitalization more consistent.

larskarlitski · 2018-04-26T19:12:10Z

Thanks very much for the review! The ones I've left uncommented are fixed as well. Didn't want to spam you too much ;)

martinpitt

Very nice, great work!

martinpitt · 2018-04-26T20:46:36Z

Oops, the test still fails: The screenshot shows that it's literally displayed $0 Active Rules.

martinpitt · 2018-04-26T20:50:29Z

pkg/networkmanager/interfaces.js

+            $('#networking-firewall-switch').onoff('value', firewall.enabled);
+
+            var n = firewall.enabledServices.size;
+            var summary = cockpit.ngettext('$0 Active Rule', '$0 Active Rules', n);


Sorry, this needs to be wrapped in cockpit.format(cockpit.ngettext(...), n) to actually resolve the $0.

Introduce a panel on the networking page, which allows toggling the firewall and shows some quick information. Also add a firewall page that shows currently enabled services (as defined by firewalld) and allows adding and removing these services. Custom ports are not yet supported. Fixes #1094 Closes #8032

larskarlitski · 2018-04-27T09:05:42Z

Added cockpit.format() (and a hack, because that cannot handle 0 as an argument) and reordered the location with the weird indent to make that more readable.

martinpitt

Nice, thanks!

martinpitt · 2018-04-27T09:41:23Z

Corresponding semaphore run: https://semaphoreci.com/cockpit/cockpit/branches/pull-request-8032/builds/20 (wasn't added as test here for some reason)

larskarlitski changed the title ~~WIP: Add firewall page~~ WIP: Add firewall section to the networking page Feb 7, 2018

andreasn reviewed Mar 8, 2018

View reviewed changes

larskarlitski changed the title ~~WIP: Add firewall section to the networking page~~ Add firewall section to the networking page Mar 13, 2018

larskarlitski requested a review from martinpitt March 13, 2018 21:30

martinpitt requested changes Mar 14, 2018

View reviewed changes

martinpitt requested changes Apr 3, 2018

View reviewed changes

martinpitt added the needswork label Apr 4, 2018

larskarlitski removed the needswork label Apr 20, 2018

larskarlitski added the needswork label Apr 20, 2018

martinpitt requested changes Apr 24, 2018

View reviewed changes

apollo13 reviewed Apr 25, 2018

View reviewed changes

martinpitt requested changes Apr 26, 2018

View reviewed changes

larskarlitski added 2 commits April 26, 2018 20:39

lib: Allow custom classes on Tooltip

c778da7

Pass through the `className` property to the outermost div of the Tooltip component. This avoids wrapping the tooltip when using it with positioning classes (such as `pull-right`).

martinpitt approved these changes Apr 26, 2018

View reviewed changes

martinpitt reviewed Apr 26, 2018

View reviewed changes

martinpitt approved these changes Apr 27, 2018

View reviewed changes

martinpitt removed the needswork label Apr 27, 2018

martinpitt merged commit 08ed421 into cockpit-project:master Apr 27, 2018

larskarlitski deleted the firewall branch April 30, 2018 09:53

		raise Exception("timed out waiting for {} to become {}".format(unit, state))


		class TestNetworking(MachineCase):


		self.login_and_go("/network/firewall")

		# Add a service that's probably not enabled by default anywhere

		wait(lambda: active_state(unit) == state, delay=0.2)


		@skipImage("firewalld not installed", "rhel-7-5", "centos-7", "fedora-atomic", "rhel-atomic", "debian-stable", "debian-testing", "ubuntu-stable", "ubuntu-1604")

Add firewall section to the networking page #8032

Add firewall section to the networking page #8032

Uh oh!

Conversation

larskarlitski commented Nov 5, 2017 • edited by martinpitt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garrett commented Dec 5, 2017

Uh oh!

q2dg commented Dec 5, 2017

Uh oh!

larskarlitski commented Dec 22, 2017

Uh oh!

larskarlitski commented Feb 7, 2018

Uh oh!

larskarlitski commented Mar 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

larskarlitski commented Mar 8, 2018

Uh oh!

larskarlitski commented Mar 12, 2018

Uh oh!

larskarlitski commented Mar 13, 2018

Uh oh!

martinpitt left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

larskarlitski commented Mar 14, 2018

Uh oh!

martinpitt commented Mar 15, 2018

Uh oh!

larskarlitski commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martinpitt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

larskarlitski commented Apr 3, 2018

Uh oh!

larskarlitski commented Nov 5, 2017 •

edited by martinpitt

Loading

larskarlitski commented Mar 29, 2018 •

edited

Loading