Added concurrent application launch prevention

[mdrose]

Resolves #44

Uses Unix domain sockets to see if the client is already running. If it's already running, the new process displays an error dialog and exits upon user acknowledgement.

[heartsucker]

Heya, so this seems like it would allow for races. My initial thoughts was to use the sdc_home directory to store a lockfile using a fcntl exclusive lock. I know this isn't a particularly Qt way of doing it, but since that happens at the kernel level, it would prevent races. The dialogue box could still be used in the same way, however.

Also it seems that your way would prevent parallel testing (not necessarily bad, but just a thought). My main reason for wanting a lock was not to have exactly one Qt application running ever, but to prevent a user from click the icon 50 times and opening way too many copies of the app. (Yes, also read/write/flag races).

Something like this:

import fcntl
from contextlib import contextmanager

@contextmanager
def acquire_lock(sdc_home: str) -> None:
    with open(path.join(sdc_home, '.app-lock'), 'w') as f:
        try:
            fcntl.flock(f, fcntl.LOCK_EX)
            yield True
        except (OSError, IOError):
            error('Failed to acquire lock. Are the tests already running?')
            yield False

I also generally worry about socket listeners in a high security environment.

Def open to hearing why your method would work better or if I'm missing something here.

[heartsucker]

Heya, so this seems like it would allow for races. My initial thoughts was to use the sdc_home directory to store a lockfile using a fcntl exclusive lock. I know this isn't a particularly Qt way of doing it, but since that happens at the kernel level, it would prevent races. The dialogue box could still be used in the same way, however.

Also it seems that your way would prevent parallel testing (not necessarily bad, but just a thought). My main reason for wanting a lock was not to have exactly one Qt application running ever, but to prevent a user from click the icon 50 times and opening way too many copies of the app. (Yes, also read/write/flag races).

Something like this:

import fcntl
from contextlib import contextmanager

@contextmanager
def acquire_lock(sdc_home: str) -> None:
    with open(path.join(sdc_home, '.app-lock'), 'w') as f:
        try:
            fcntl.flock(f, fcntl.LOCK_EX)
            yield True
        except (OSError, IOError):
            yield False

I also generally worry about socket listeners in a high security environment.

Def open to hearing why your method would work better or if I'm missing something here.

[mdrose]

@heartsucker Usual advantage would be portability, but since this is intended to run only on Qubes, that doesn't apply.

Another possibility is just checking to see if a name is bound in the abstract namespace. If it's already bound, the kernel throws an error. Pretty simple and straightforward. Avoids any filesystem involvement, like you would have with file locks.

I've pushed a commit to this PR with that implementation.

[heartsucker]

I prefer this implementation to your original since it seems to be more race-free than the other. I cannot, however, comment on the security implications of it.

I also do suggest we replace the identifier with the sdc_home variable that gets injected (see #43). This is really the exact scenario we want to prevent, and doing that would allow for parallel testing of the app in that we could bind to '\0' + APP + sdc_home in order to get many instance up.

[mdrose]

@heartsucker Not quite sure where your security concern lies. It literally just asks the kernel if another process has already taken a unique name.

Anyway, looking at #43, it looks like it would be best for that to be resolved first.

[heartsucker]

No actual concerns. Just stating that I can't quickly analyze it because I'm unfamiliar with it.

[mdrose]

@heartsucker updated

[heartsucker]

Two nits, but otherwise works as advertised. :D

[emkll]

I might be a bit late to this conversation, my understanding that the classic pattern is to use a lock file on the filesystem as @heartsucker suggests, since it’s easy to debug/delete if something goes wrong. I see that the socket is not explicitly closed, will it be reliably released? even if the application does not exit cleanly?

Just chiming in regarding the possible security implications raised, I am assuming the concern was arbitrary data sent to the socket bind command. Given that it’s a transliteration from the Unix implementation (https://docs.python.org/2/library/socket.html) I believe it should not have any further implications, especially given the the address is prefixed with the application name. Also, given that a potential attacker would need to control/modify this config file, I think in this case it shouldn’t present any further risk.

[mdrose]

@emkll If the process ends, the kernel releases the binding, regardless of how it ends, same as file locks. File locking is more common, since it's been around a lot longer, but also because the abstract namespace only exists in the Linux kernel, so you wouldn't use it if you want portability (BSD etc). Main advantage over file locking is it's immune to file system state. Honestly, I don't see there being a significant difference either way, though, in this particular situation.

[heartsucker]

Thanks :D :D