Opened 8 years ago

Last modified 9 months ago

#5130 new enhancement

Allow obfsproxy to daemonize

Reported by: ericpaulbishop Owned by:
Priority: Low Milestone:
Component: Archived/Obfsproxy Version:
Severity: Normal Keywords: obfsproxy daemon
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I've been working on setting up obfsproxy and I ran into a minor issue: It doesn't seem possible to send the obfsproxy process into the background (i.e. daemonize it) if I'm running it as an external server and not in managed mode. If I want to be able to specify the listening port for a bridge server I don't think there's a way to do this without running obfsproxy as an external server (please correct me if I'm wrong). However, there's no option to fork/detach from the terminal in the case of an external server, which seems a rather significant issue.

I've written a small patch for main.c that seems to correct this problem (see attached). This implements two command line arguments, "--daemonize", which, when specified will just send the program into the background and "--daemonize_with_pid=<pid_file>", which will daemonize and write the process id to the specified file. This patch only works on unix systems, but I've added #ifdef directives that only enable this code if unistd.h is defined, otherwise it will be completely ignored.

Child Tickets

Attachments (3)

obfsproxy-allow-daemonize.patch (3.9 KB) - added by ericpaulbishop 8 years ago.
Patch introducing option to daemonize obfsproxy
0001-Implement-daemon-feature.patch (4.4 KB) - added by dazo 8 years ago.
Implement a daemon mode (patch 1/2)
0002-Implement-PID-tracking.patch (3.8 KB) - added by dazo 8 years ago.
Implement PID tracking (patch 2/2)

Download all attachments as: .zip

Change History (13)

Changed 8 years ago by ericpaulbishop

Patch introducing option to daemonize obfsproxy

comment:1 Changed 8 years ago by nickm

Status: newneeds_review

comment:2 Changed 8 years ago by asn

Thanks for the patch, sorry for not having enough time to review it yet.

comment:3 Changed 8 years ago by dazo

I've reviewed the patch as I've hit the same issue. There are a few things which I personally don't like as much in the patch; mostly coding style related but also some things which I consider not needed or complicated.

For example, calling getdtablesize() this early in a program makes little sense to me - as we're in the initialisation phase of the program. That means it's not much extra fds opened which needs to be closed, other than std{in,out,err}. Further that logic doesn't consider if those file descriptors have been opened or not. And std{in,out,err} is reopened and redirected to /dev/null again after this block. This logic will also close the log file too.

I also don't like that the pid file tracking is depended on the daemon mode. And further that you keep a file descriptor open the whole time too, with a file lock I find complicated when you have the O_EXCL flag to open() which takes care of not overwriting an existing file.

But I had something already written, so I merged that stuff into this model. I have split the daemon code into one patch and the pid-file handling as a second patch. The pid-file patch depends on the daemon patch, though. But this will hopefully make it easier to review the code.

Changed 8 years ago by dazo

Implement a daemon mode (patch 1/2)

Changed 8 years ago by dazo

Implement PID tracking (patch 2/2)

comment:4 Changed 6 years ago by asn

Hi,

the codebase of obfsproxy is nowadays in Python:
https://gitweb.torproject.org/pluggable-transports/obfsproxy.git

We still don't daemonize in external-mode, but hopefully most people are using managed mode by now.

I would still accept a patch for the Python-based obfsproxy to daemonize, provided that the patch is simple enough.

Thanks!

comment:5 Changed 6 years ago by asn

Status: needs_reviewneeds_revision

comment:6 Changed 6 years ago by asn

Status: needs_revisionnew

This is still a useful ticket but low priority.

Work that has been done in this ticket was for C obfsproxy.
We have now migrated to Python-obfsproxy so we need new code.

comment:7 Changed 6 years ago by asn

meejah says that this might not be too hard to do because Twisted can do these things easily.

comment:8 Changed 2 years ago by teor

Severity: Normal

Set all open tickets without a severity to "Normal"

comment:9 Changed 10 months ago by teor

Owner: asn deleted
Status: newassigned

asn does not need to own any obfuscation tickets any more. Default owners are trouble.

comment:10 Changed 9 months ago by cohosh

Status: assignednew

tickets are unassigned, reverting to 'new'

Note: See TracTickets for help on using tickets.