Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ufw rules are not cleaned when multiple containers are killed #11

Closed
mlollo opened this issue Feb 6, 2021 · 4 comments
Closed

Ufw rules are not cleaned when multiple containers are killed #11

mlollo opened this issue Feb 6, 2021 · 4 comments

Comments

@mlollo
Copy link
Contributor

mlollo commented Feb 6, 2021

  • docker-compose
version: '2.4'
services:
  nginx1:
    image: nginx:alpine
    container_name: nginx1
    restart: unless-stopped
    labels:
      - UFW_MANAGED=true
      - UFW_ALLOW_FROM=192.168.0.0/24
      - UFW_DENY_OUTGOING=true
      - UFW_ALLOW_TO=any:53;dl-cdn.alpinelinux.org:80/tcp
    ports:
      - 8180:80

  nginx2:
    image: nginx:alpine
    container_name: nginx2
    restart: unless-stopped
    labels:
      - UFW_MANAGED=true
      - UFW_ALLOW_FROM=192.168.0.0/24
      - UFW_DENY_OUTGOING=true
      - UFW_ALLOW_TO=any:53;dl-cdn.alpinelinux.org:80/tcp
    ports:
      - 8280:80

  nginx3:
    image: nginx:alpine
    container_name: nginx3
    restart: unless-stopped
    labels:
      - UFW_MANAGED=true
      - UFW_ALLOW_FROM=192.168.0.0/24
      - UFW_DENY_OUTGOING=true
      - UFW_ALLOW_TO=any:53;dl-cdn.alpinelinux.org:80/tcp
    ports:
      - 8380:80
  • commands :
docker-compose up -d
docker-compose down
  • syslogs :
ufw-docker-automated: Adding UFW rule: allow from 192.168.0.0/24 to container nginx1 on port 80/tcp
ufw-docker-automated: Adding UFW rule: allow reply from container nginx1 on port 80/tcp to 192.168.0.0/24
ufw-docker-automated: Adding UFW rule: allow outgoing from container nginx1 to any on port 53
ufw-docker-automated: Adding UFW rule: allow outgoing from container nginx1 to 151.101.122.133/32 on port 80/tcp
ufw-docker-automated: Adding UFW rule: deny outgoing from container nginx1 to any
ufw-docker-automated: Adding UFW rule: allow from 192.168.0.0/24 to container nginx2 on port 80/tcp
ufw-docker-automated: Adding UFW rule: allow reply from container nginx2 on port 80/tcp to 192.168.0.0/24
ufw-docker-automated: Adding UFW rule: allow outgoing from container nginx2 to any on port 53
ufw-docker-automated: Adding UFW rule: allow outgoing from container nginx2 to 151.101.122.133/32 on port 80/tcp
ufw-docker-automated: Adding UFW rule: deny outgoing from container nginx2 to any
ufw-docker-automated: Adding UFW rule: allow from 192.168.0.0/24 to container nginx3 on port 80/tcp
ufw-docker-automated: Adding UFW rule: allow reply from container nginx3 on port 80/tcp to 192.168.0.0/24
ufw-docker-automated: Adding UFW rule: allow outgoing from container nginx3 to any on port 53
ufw-docker-automated: Adding UFW rule: allow outgoing from container nginx3 to 151.101.122.133/32 on port 80/tcp
ufw-docker-automated: Adding UFW rule: deny outgoing from container nginx3 to any
ufw-docker-automated: Cleaning UFW rule: container nginx3 deleted rule 'route allow from 192.168.0.0/24 to 172.17.0.4 port 80 proto tcp'
ufw-docker-automated: Cleaning UFW rule: container nginx3 deleted rule 'route allow from 172.17.0.4 port 80 to 192.168.0.0/24 proto tcp'
ufw-docker-automated: Cleaning UFW rule: container nginx3 deleted rule 'route allow from 172.17.0.4 to any port 53'
ufw-docker-automated: Cleaning UFW rule: container nginx3 deleted rule 'route allow from 172.17.0.4 to 151.101.122.133 port 80 proto tcp'
ufw-docker-automated: Cleaning UFW rule: container nginx3 deleted rule 'route deny from 172.17.0.4'

I think the script can handle one event at a time, when it needs to continue to listen to other events. Maybe with subprocess the script could open a dedicated child process for cleaning rules for a container and continue to listen on the main process ?

@mlollo
Copy link
Contributor Author

mlollo commented Feb 6, 2021

I think I've got something working on branch ufw-threads.

I'm not sure this is the right approach, from what I am understanding subprocesses are running at the same time ufw commands and ufw can't handle requests at the same time. Some time rules were updated and some time not.
With threading and a lock rules are updated correctly.

For readability I could put the lock at the beginning and the end of the handler handle_ufw_rules. This will make ufw rules to appear in order when running ufw status command.

@shinebayar-g
Copy link
Owner

shinebayar-g commented Feb 6, 2021

That's a great observation ! and makes sense.

@mlollo
Copy link
Contributor Author

mlollo commented Apr 5, 2021

ufw-threads branch is up to date with ufw-comment idea.
It was tested on debian 10.8 docker 20.10.5.
One can argue that ufw-docker-automated is quite slow to keep up if docker containers restart at a fast pace.
This delay depends on the number of ufw rules and spawned containers.
But there is a stability in the long run (could be in minutes if you spawn 50/100 containers).
This could be tested at a large scale when spawning hundreds of containers.

@mlollo
Copy link
Contributor Author

mlollo commented Apr 17, 2021

This issue is a consequence of using docker API to retrieve container information.
When a container dies, the container doesn't exists anymore. The docker API call fails and returns nothing. Hence ufw rules are not cleaned properly. This issue is fixed in PR #14

I abandoned ufw-threads branch because there is no point of using threading if it can't update ufw rules simultanously (I had to use locks). ufw-update-onrestart branch is the most recent version that I use currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants