Opened 8 months ago

Closed 6 months ago

Last modified 4 weeks ago

#31834 closed defect (fixed)

Make obfs4 Docker image more usable

Reported by: phw Owned by: phw
Priority: Medium Milestone:
Component: Circumvention Version:
Severity: Normal Keywords: docker, s30-o24a2
Cc: cohosh, phw Actual Points: 1.1
Parent ID: #31281 Points: 1
Reviewer: cohosh Sponsor: Sponsor30-can

Description (last modified by phw)

Here is some feedback we got from an operator (see this blog post for the full story):

  • Make it easier to get the bridge's fingerprint and/or bridge line. At the moment, users have to spawn a shell in the container, which is tedious.
  • Maybe provide a docker-compose file.
  • Improve our official setup instructions. These instructions were more helpful to an operator.
  • Add a note that operators can run docker logs <container> to check if it's up and running.
  • Mention concerns regarding permanence: Ideally, a container should run as long as possible.
  • Allow running a bridge on a port <1024 (as per mrphs's request in comment:2).

Child Tickets

Change History (27)

comment:1 Changed 8 months ago by phw

Keywords: s30-o24a2 added
Parent ID: #31281

comment:2 Changed 8 months ago by mrphs

It would be awesome if this docker image provided a way to run obfs4 on privileged ports as well

comment:3 Changed 8 months ago by phw

Description: modified (diff)

comment:4 Changed 8 months ago by phw

Description: modified (diff)

comment:5 in reply to:  description Changed 7 months ago by thymbahutymba

Replying to phw:

During these days I figure out some solutions about some problems pointed by Philipp.

  • Make it easier to get the bridge's fingerprint and/or bridge line. At the moment, users have to spawn a shell in the container, which is tedious.

For make easier get not only the fingerprint but all the log available (with docker logs CONTAINER) I added to the start-tor.sh file one more log line Log notice stdout.

  • Maybe provide a docker-compose file.

I had to make a choice between docker-compose and Makefile, I chose the Makefile. The reason that convince me in this choice was the fact that each container, that are not related each other, provides an instance of tor (they don't be part of a whole service which is the purpose of docker-compose). Using the Makefle give also others advantages like embed the build command and the config target. Just to be more clear here the Makefile that I wrote:

FLAGS=-d --restart unless-stopped --log-opt "max-size=30m"
EMAIL=
VOLUME=/var/lib/tor

.PHONY: build
build:
	docker build -t obfs4-proxy

.PHONY: deploy
deploy: DockerObfs4Proxy-1 DockerObfs4Proxy-2

DockerObfs4Proxy-%: config-%
	docker run \
		-e  "OR_PORT=${OR_PORT}" -e "PT_PORT=${PT_PORT}" -e "EMAIL=${EMAIL}" \
		-p "${OR_PORT}":"${OR_PORT}" -p "${PT_PORT}":"${PT_PORT}" \
		-v "$@-vol":"${VOLUME}" \
		--name $@ 	\
		${FLAGS} 	\
		obfs4-proxy

config-1:
	$(eval OR_PORT = 993)
	$(eval PT_PORT = 443)

config-2:
	$(eval OR_PORT = 143)
	$(eval PT_PORT = 995)

In this case can be even replaced the deploy-container.sh file also due to the fact that the user have to be able to chose the ports that he prefers.
Is worth to notice that using this approach the user can deploy as many containers as he wants just changing few things: what is required by the deploy target and adding the respective config-X target.

  • Mention concerns regarding permanence: Ideally, a container should run as long as possible.

I also added a volume for the /var/lib/tor directory keeping the seniority earned by the bridge. In that way if an update is required is easy to build the new image and deploy it.

I would also like to say that just editing the section about the torrc file in start-tor.sh there is the chance to deploy container for guard, middle and exit nodes.

Last edited 7 months ago by thymbahutymba (previous) (diff)

comment:6 Changed 6 months ago by phw

Description: modified (diff)

Here's a brief update with what I've managed to address so far:

Make it easier to get the bridge's fingerprint and/or bridge line. At the moment, users have to spawn a shell in the container, which is tedious.


Commit d2335c91 adds a script that determines the bridge line. Users can run it like this:

$ docker exec 9d66b756b3cc get-bridge-line
obfs4 1.2.3.4:1234 A177E491C751488E7ADA397C7E47E4B3155723BD cert=KrQlXDh826TGTSywmtRaAZkq/dLI45m3Jl/drkYeaVD1ykghcJeFjyubff6hf1ZMG7ujeA iat-mode=0


Improve our official setup instructions. These instructions were more helpful to an operator.


I improved our official instructions in commit bfe821bc.

Add a note that operators can run docker logs <container> to check if it's up and running.


Documented in commit bfe821bc and made possible in commit 1f5fd1e8.

Allow running a bridge on a port <1024 (as per mrphs's request in comment:2).


Fixed in commit aceb0c10.

comment:7 Changed 6 months ago by phw

Description: modified (diff)

Maybe provide a docker-compose file.


thymbahutymba's comment convinced me that a Makefile is a better solution than a docker-compose file. I replaced the script deploy-container.sh with a Makefile in commit 2dce6dc7.

Mention concerns regarding permanence: Ideally, a container should run as long as possible.


I adopted thymbahutymba's idea of using a volume by adding docker's --volume argument to the Makefile.

comment:8 Changed 6 months ago by phw

Reviewer: cohosh
Status: assignedneeds_review

This is finally ready for a review. Cecylia, could you please review the commits starting at and including d2335c91?

comment:9 Changed 6 months ago by phw

Reminder to myself: once the changes are reviewed, we need to update our instructions and push a new image version.

comment:10 Changed 6 months ago by cohosh

Status: needs_reviewmerge_ready

It looks really good! I like the changes that were made. I left a few comments on the commits but it's good to go as is.

comment:11 in reply to:  10 ; Changed 6 months ago by phw

Actual Points: 1.1
Resolution: fixed
Status: merge_readyclosed

Replying to cohosh:

It looks really good! I like the changes that were made. I left a few comments on the commits but it's good to go as is.


Thanks, your comments were very helpful. I addressed your feedback in commit 543e5db2, a13b1d79, and 2bb69bd2. I also updated our setup instructions and pushed a new image tag, 0.3.

comment:12 in reply to:  11 Changed 6 months ago by thymbahutymba

Replying to phw:

Replying to cohosh:

It looks really good! I like the changes that were made. I left a few comments on the commits but it's good to go as is.


Thanks, your comments were very helpful. I addressed your feedback in commit 543e5db2, a13b1d79, and 2bb69bd2. I also updated our setup instructions and pushed a new image tag, 0.3.

I would like to let you know about an issue in your Makefile, with that solution you can't deploy more that one container in the whole system; therefore if you would to run more the one bridge (relay in general) you can't due to limitation about the volume's name that is the same every time. Maybe, if you don't like my solution with config-X, provide a kind of env file or whatever else that keep track about the number of container deployed. I hope I was clear enough.

comment:13 in reply to:  6 Changed 6 months ago by thymbahutymba

Replying to phw:

Make it easier to get the bridge's fingerprint and/or bridge line. At the moment, users have to spawn a shell in the container, which is tedious.


Commit d2335c91 adds a script that determines the bridge line.

Remember also that since now the directory is mapped as docker volume the path

PT_STATE=/var/lib/tor/pt_state/obfs4_bridgeline.txt

is no longer meaningful and should be replaced with something like

PT_STATE=/var/lib/docker/volumes/<VOLUME NAME>/_data/

Edit:
Nevermind, I thought the script was outside the container.

Last edited 6 months ago by thymbahutymba (previous) (diff)

comment:14 Changed 6 months ago by phw

Resolution: fixed
Status: closedreopened

I'm reopening this ticket, so we can make our Makefile support more than one bridge.

comment:15 Changed 6 months ago by phw

Status: reopenedneeds_review

Here's a patch that derives docker's volume name from the two given ports, making it possible to deploy more than one container on a machine:
https://dip.torproject.org/torproject/anti-censorship/docker-obfs4-bridge/compare/master...fix%2F31834

I know that thymbahutymba is no fan of this but I find it intuitive that each call to make deploy results in a new container being deployed. As for the volume names: I believe that thymbahutymba prefers the volume name to contain the bridge name instead of the ports. I'm fine with this too but I find it a little awkward because we'd be introducing a new constraint (unique bridge names) which don't exist in Tor.

comment:16 in reply to:  15 ; Changed 6 months ago by thymbahutymba

Replying to phw:

Here's a patch that derives docker's volume name from the two given ports, making it possible to deploy more than one container on a machine:
https://dip.torproject.org/torproject/anti-censorship/docker-obfs4-bridge/compare/master...fix%2F31834

I know that thymbahutymba is no fan of this but I find it intuitive that each call to make deploy results in a new container being deployed. As for the volume names: I believe that thymbahutymba prefers the volume name to contain the bridge name instead of the ports. I'm fine with this too but I find it a little awkward because we'd be introducing a new constraint (unique bridge names) which don't exist in Tor.

Actually my point is not about bridge name but how to identify in a smart way the name of the container and the volume connected with. I thought that having external config file can solve this (and others) problem. In this case the user can keep the configuration file separated by the Makefile, moreover if the user need to re-deploy the container, due to image update or whatever else, this make the process quite easy and faster than run deploy multiple time, which requires to have in mind which are the ports associated to that. After saying that just consider something like what follows.

# env
BRIDGE_NAME=DockerObfs4
BRIDGE_LIST=${BRIDGE_NAME}-{1..2}

${BRIDGE_NAME}-1-OR_PORT=993
${BRIDGE_NAME}-1-PT_PORT=143

${BRIDGE_NAME}-2-OR_PORT=443
${BRIDGE_NAME}-2-PT_PORT=25

# Makefile
include env

deploy: $(shell echo ${BRIDGE_LIST})

${BRIDGE_NAME}-%:
    @[ ${$@-OR_PORT} ] || ( echo "Env var OR_PORT for $@ is not set."; exit 1 )
    @[ ${$@-PT_PORT} ] || ( echo "Env var PT_PORT for $@ is not set."; exit 1 )
    @docker run \
        ....    \
        --volume $@-vol:/var/lib/tor \
        ....

Just to summarize what I think are the advantages of that:

  • Is not required to remember the configuration each time;
  • Easy to identify at which container is associate one volume;
  • No need to run multiple times make deploy <SOMETHING> for each instance that we would run that's means speedup on deployment phase.
Last edited 6 months ago by thymbahutymba (previous) (diff)

comment:17 in reply to:  16 ; Changed 6 months ago by phw

Replying to thymbahutymba:

Just to summarize what I think are the advantages of that:

  • Is not required to remember the configuration each time;
  • Easy to identify at which container is associate one volume;
  • No need to run multiple times make deploy <SOMETHING> for each instance that we would run that's means speedup on deployment phase.


It does, however, require modifying the Makefile, to select your own ports. That's something I would like to avoid. And for what it's worth, according to the principle of least surprise, I think that one call to make deploy should result in one bridge being spawned.

comment:18 in reply to:  17 ; Changed 6 months ago by thymbahutymba

Replying to phw:

It does, however, require modifying the Makefile, to select your own ports. That's something I would like to avoid.

Actually not, where I wrote # env is a different file as you can see a bit below where the Makefile starts:

# Makefile
include env

env is the name of the file where the configuration is located.

comment:19 in reply to:  18 Changed 6 months ago by phw

Resolution: fixed
Status: needs_reviewclosed

Replying to thymbahutymba:

env is the name of the file where the configuration is located.


You are right, I missed that.

After giving this some more thought, I decided to move forward with a tradeoff. I agree with thymbahutymba that the bridge information should be stored in a file; otherwise operators will have to dig up their ports each time they're upgrading the image. Our improved instructions now suggest to write your bridge configuration to a separate file, which is sourced before invoking the Makefile. This makes it possible to run multiple bridges (by having several bridge config files) while at the same time making it easy to upgrade to new image versions. Also note that running multiple bridges per machine is an edge case that we don't encourage.

comment:20 Changed 6 months ago by thymbahutymba

I don't want mess everything up but I think that there are still one issue. For understand what I mean: "What if you want/need to change your ports?" You volume name still remains related with the previous one, so once you deploy the first time the container you are not able to change your ports for whatever reason.

comment:21 in reply to:  20 ; Changed 6 months ago by phw

Replying to thymbahutymba:

I don't want mess everything up but I think that there are still one issue. For understand what I mean: "What if you want/need to change your ports?" You volume name still remains related with the previous one, so once you deploy the first time the container you are not able to change your ports for whatever reason.


Don't worry, I really appreciate hearing your thoughts on this!

Hmm, you are right but this may not necessarily be an issue. When a bridge changes its port, it's losing all its users because there is no way for a user to learn that their bridge is now available at a new port. As a result, I think it's fine if a port change involves creating a new data directory. Am I missing something?

comment:22 in reply to:  21 ; Changed 6 months ago by thymbahutymba

Replying to phw:

Am I missing something?

I'm point just the fact that if somebody for some reason has to change the port he loose not only its users but also its seniority.

I would also let you know that there is an issue in the get-bridge-line script, due to the fact that the log might contains multiple "reboot" of the docker container with the same line repeated multiple times, this is a mess once someone has to run the script because ends up in a plenty of meaningless lines before the final result. I think that in order to do fix this after the sed command can be piped the tail command in order to get the last line. For example:

$(grep '...' "$TOR_LOG" | sed "..." | tail -1)

comment:23 in reply to:  22 ; Changed 6 months ago by phw

Replying to thymbahutymba:

Replying to phw:

Am I missing something?

I'm point just the fact that if somebody for some reason has to change the port he loose not only its users but also its seniority.


Right, apart from users, the operator would also give up the bridge's statistics (if they were sent to metrics).

I would also let you know that there is an issue in the get-bridge-line script, due to the fact that the log might contains multiple "reboot" of the docker container with the same line repeated multiple times, this is a mess once someone has to run the script because ends up in a plenty of meaningless lines before the final result. I think that in order to do fix this after the sed command can be piped the tail command in order to get the last line. For example:

$(grep '...' "$TOR_LOG" | sed "..." | tail -1)


Nice catch! I added a fix in this commit. Does it look good to you?

comment:24 in reply to:  23 Changed 6 months ago by thymbahutymba

Replying to phw:

Does it look good to you?

I guess should be fine.

I would also share with you a modification that I've made in order to let the user to chose which bridge distribution deploy, for this purpose I've added a new environmental variable called BridgeDistribution.

# Makefile
@[ ${$@-BridgeDistribution} ] || ( $(eval $@-BridgeDistribution=any) echo "Env var BridgeDistribution for $@ set to any." )

[...]

--env "BRIDGE_DISTRIBUTION=${$@-BridgeDistribution}"

If in the config file there isn't the BridgeDistribution variable set then it will be set as any.

comment:25 Changed 4 months ago by thymbahutymba

Wrong IP in case of dynamic ip which may change.
I'm not good with bash and so I don't know if something better could be proposed but this should solve the problem:

addr=$(grep 'Our IP Address has changed from' "$TOR_LOG" | \
    sed 's/.* \([0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\); .*/\1/' | \
    tail -1)

if [ -z "$addr" ]; then
    addr=$(grep 'Guessed our IP address' "$TOR_LOG" | \
        sed 's/.* \([0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\) .*/\1/' | \
        tail -1)
fi

comment:26 Changed 3 months ago by thymbahutymba

I would like to propose to you few changes for the deploy. I made some changes in order to use docker-compose for the deployment.

The changes are available at my repository in the docker-compose branch at this commit. Using docker-compose makes easier the deploying once there is a new image available. This is possible, after pulling the new image which can be done even using docker-compose pull, just doing: docker-compose up -d obfs4-bridge which is part of make deploy. In such way there is no need to stop the container, remove it and then deploy it again, which it takes three steps.

I'm not only quite sure about .env file which in case can be easily replaced by the current bridge.sh.

Last edited 3 months ago by thymbahutymba (previous) (diff)

comment:27 in reply to:  26 Changed 4 weeks ago by phw

Replying to thymbahutymba:

The changes are available at my repository in the docker-compose branch at this commit. Using docker-compose makes easier the deploying once there is a new image available. This is possible, after pulling the new image which can be done even using docker-compose pull, just doing: docker-compose up -d obfs4-bridge which is part of make deploy. In such way there is no need to stop the container, remove it and then deploy it again, which it takes three steps.


For the record, I merged these suggestions in commit 543a19af.

Note: See TracTickets for help on using tickets.