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 a container dies #7

Closed
mlollo opened this issue Jan 17, 2021 · 8 comments
Closed

Ufw rules are not cleaned when a container dies #7

mlollo opened this issue Jan 17, 2021 · 8 comments
Assignees

Comments

@mlollo
Copy link
Contributor

mlollo commented Jan 17, 2021

When a container dies and the user remove it. UFW rules are not cleaned.
Example :

version: '2.4'
services:
  debian:
    image: debian
    container_name: debian
    labels:
      - UFW_MANAGED=true
      - UFW_DENY_OUTGOING=true
      - UFW_TO=192.168.1.0/24
    command: sleep 1

Commands :

docker-compose up -d
docker-compose down

If we check ufw rules we can see that they are remaining.
If the container dies, in this case we can catch the container id in it. Here the event that we could handle :

 {'status': 'die', 'id': '284b554c5c2e8a411e27fd80be330576b496c9b2db1d86844ae79ed140c6b3a4', 'from': 'debian', 'Type': 'container', 'Action': 'die', 'Actor': {'ID': '284b554c5c2e8a411e27fd80be330576b496c9b2db1d86844ae79ed140c6b3a4', 'Attributes': {'UFW_DENY_OUTGOING': 'true', 'UFW_MANAGED': 'true', 'UFW_TO': '192.168.1.0/24', 'com.docker.compose.config-hash': 'eee765fc5954e91126235b19bd1fa7ab32c7671abe99923eef218fee254b0b0c', 'com.docker.compose.container-number': '1', 'com.docker.compose.oneoff': 'False', 'com.docker.compose.project': 'test', 'com.docker.compose.project.config_files': 'docker-compose.yml', 'com.docker.compose.project.working_dir': '/home/test', 'com.docker.compose.service': 'debian', 'com.docker.compose.version': '1.27.4', 'exitCode': '0', 'image': 'debian', 'name': 'debian'}}, 'scope': 'local', 'time': 1610910393, 'timeNano': 1610910393567662105}

Maybe we should watch only die events. Because after a kill event there is a die event.

@mlollo mlollo changed the title Container die event not handled Ufw rules are not cleaned when a container dies Jan 17, 2021
@mlollo
Copy link
Contributor Author

mlollo commented Jan 19, 2021

Rules are not cleared also when rebooting the machine and restart policy is set to 'unless-stopped'.
And at reboot the container is restarted with a new IP address and new rules are not created.

@shinebayar-g
Copy link
Owner

Thanks for reporting, I'll take a deeper look into these issues once I fix my laptop. I'm going to reinstall the OS this weekend so I'll able to back to work.

@mlollo
Copy link
Contributor Author

mlollo commented Jan 25, 2021

I have a solution for cleaning ufw rules when restarting or shutdown the machine.
I created an init.d script that calls at boot a start.py and stop.py at shutdown or reboot.
And it works ! It cleans rules and create them at boot when there is a container running :)
I'll make a branch ufw-update-onrestart

The problem is that the user needs to add 3 scripts and do some commands.
I think we should think of an install script that do everything for the user.
Usually I put all my scripts in /usr/lib/ufw-docker and I also tried with a virtualenv. Here the scripts :

  • /etc/init.d/ufw-docker
  • /lib/systemd/system/ufw-docker-automated.service
  • /usr/lib/ufw-docker
    • automated.py
    • start.py
    • stop.py
    • venv

As for cleaning the rules when a container dies.
I have no solution for now since the 'die' event doesn't have the ip address of the container.
Without a storage that keeps the association (container_id, container_ip) it is tricky to solve this.
Maybe a global vars could do the job, but when the service restarts it will loose the association.

@shinebayar-g
Copy link
Owner

shinebayar-g commented Jan 25, 2021

Indeed, we need some kind of container id, container ip mapping. I think ufw command actually has a feature for comments / description for rules. (which I never used)
So we could store container IDs in their related ufw rules. As soon as container dies, we could lookup ufw rules by their description and find the related container then delete it.

@mlollo
Copy link
Contributor Author

mlollo commented Jan 26, 2021

I agree this is the right solution for this mapping, I wasn't sure that ufw could do comments like iptables.
Also this die handler will it replace the kill handler ? Or we should keep kill and die handler, because the die handler won't do anything if rules were deleted before in the kill handler ?

@mlollo
Copy link
Contributor Author

mlollo commented Apr 5, 2021

Idea added in branch ufw-comment
I can submit a PR if the implementation is close from what you imagined.

I have also implemented this idea in branch ufw-threads.
But here I added a prefix tag 'container:' to simplify the cleaning of all ufw rules at shutdown.

@mlollo
Copy link
Contributor Author

mlollo commented Apr 5, 2021

I found an issue with handling die event instead of kill event, sometime docker is faster than ufw-docker-automated script and the container doesn't exist anymore when calling the docker API to retrieve information on the container.
I need to use data from the event to clean ufw rules instead of calling a second time docker API. More over for die event container ip is not used anymore.

The issue would be the same during a kill event or event a start event, ufw-docker-automated script could be really late in comparison with docker. And it could lead to trying to find a container that doesn't exist anymore (when doing start and stop multiple times). This script must be resilient when the container doesn't exist anymore.
It's not an issue for start event because if the container doesn't exist anymore it means that the script will process a die event just after. So at least for die events it must works without the docker API call on container. If not we could find some ghosts ufw rules remaining sometimes.

I also think the issue #11 is related to this, I tested the same scenario with this fix and I don't have the issue of ghosts rules anymore. So maybe the threading approach is not necessary anymore.

This issue was found on ufw-threads tests, case scenario :
docker-compose.yml

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

  nginx4:
    image: nginx:alpine
    container_name: nginx4
    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:
      - 8480:80

  nginx5:
    image: nginx:alpine
    container_name: nginx5
    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:
      - 8580:80


  nginx6:
    image: nginx:alpine
    container_name: nginx6
    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:
      - 8680:80

  nginx7:
    image: nginx:alpine
    container_name: nginx7
    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:
      - 8780:80

Step for reproducing the issue :

  • docker-compose up -d
  • sed -i 's/192.168.0.0/192.168.1.0/g' docker-compose.yml
  • docker-compose up -d
  • Results : sometimes some rules are not properly cleaned

@mlollo
Copy link
Contributor Author

mlollo commented Apr 17, 2021

Fixed in PR #14. In this PR we use die event for cleaning ufw rules and we don't call docker API anymore to retrieve container information when the container dies (explanation here)

@mlollo mlollo closed this as completed Apr 30, 2021
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