jacobalberty / unifi-docker

Unifi Docker files
MIT License
2.13k stars 454 forks source link

Certs import flawed, no import of new certs during runtime #207

Open ThomDietrich opened 5 years ago

ThomDietrich commented 5 years ago

Hey guys! After setting up the system and playing around a bit (everything seems fine, incl. SSL certificate by Let's Encrypt) I went to test the emergency case. Deleted everything (docker-compose down -v --rmi all --remove-orphans) and restored as if I migrated to a new host.

The containers came back up and I was able to restore an autobackup. So far perfect. Sadly the previously provided certificate is not used anymore. I am back at the snakeoil certificate.

In the logs I can find:

controller_1  | [2019-06-09 18:05:56,743] <docker-entrypoint> Cert has not changed, not updating controller.

Assumption/Question: After checking the sourcecode of the import_cert script I realize that I could have deleted the md5 file to solve the "bug" on my system. Please be aware that I changed the default docker-compose.yml to mount the certs folder locally, see below. To summarize: This seems to be a functional bug. The cert file shouldn't only be checked against the md5 but also against the internal cert. Is there any reason to doing the md5 check instead of always importing the cert? What if the md5 file existed from another source?

All the best!

Host operating system

Ubuntu

What tag are you using

latest (UniFi 5.10.24)

Complete docker-compose.yml

Note: Pay attention to the local mount of cert, might be related.

version: '2.2'
services:
  mongo:
    image: mongo:3.4
    networks:
      - unifi
    restart: always
    volumes:
      - db:/data/db
  controller:
    image: "jacobalberty/unifi:${TAG:-latest}"
    depends_on:
      - mongo
    init: true
    networks:
      - unifi
    restart: always
    volumes:
      - data:/unifi/data
      - log:/unifi/log
      #- cert:/unifi/cert
      - ./cert/:/unifi/cert/
      - init:/unifi/init.d
      - ./backup/:/unifi/data/backup/
    environment:
      DB_URI: mongodb://mongo/unifi
      STATDB_URI: mongodb://mongo/unifi_stat
      DB_NAME: unifi
      TZ: "Europe/Berlin"
      RUNAS_UID0: "false"
    ports:
      - "3478:3478/udp" # STUN
      - "6789:6789/tcp" # Speed test
      - "8080:8080/tcp" # Device/ controller comm.
      - "443:8443/tcp" # Controller GUI/API as seen in a web browser
      - "8880:8880/tcp" # HTTP portal redirection
      - "8843:8843/tcp" # HTTPS portal redirection
      - "10001:10001/udp" # AP discovery
  logs:
    image: bash
    depends_on:
      - controller
    command: bash -c 'tail -f /unifi/log/*.log'
    restart: always
    volumes:
      - log:/unifi/log

volumes:
  db:
  data:
  log:
  #cert:
  init:

networks:
  unifi:
ThomDietrich commented 4 years ago

Any comments? This is a real bug we should try to solve.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

ThomDietrich commented 2 years ago

Still relevant.

jacobalberty commented 2 years ago

Putting it on my list to look at. With the new 7.0.x release coming soon I'm doing some housekeeping as I prepare for it.

markdoliner commented 1 year ago

One thought: Maybe the .md5 file could be saved to a location that gets discarded when the container is destroyed? Maybe /tmp/? It's not as good as checking the source of truth (the key store), but may be an easy improvement?

ThomDietrich commented 1 year ago

@jacobalberty did you ever find the chance to think about this in more detail?

Beyond the issue here I wonder if the whole method of importing a certificate could be improved so that the provided certificates are the primary source of truth, i.e. are regularly checked and re-imported.

This would solve another issue I had to face over the years: Expired and reissued certificates (Let's Encrypt certs are only valid for 3m) are not automatically imported by the running docker container. Of course the container restart can be handled from outside but in good faith of containerization an internal less-destructive solution would be a lot cleaner and prefered.

Proposed solution:

  1. Run import script at startup and as part of a daily "cron job"
    • Alternative: Provide and document a convenient way to trigger the re-import with an exec from outside the container
  2. An md5 is stored in /tmp/ as @markdoliner suggested (Alternatively do not maintain an md5 file, why do we need to limit the keystore process?)
  3. The import script checks the certificate files against the md5 file and against the keystore. If check fails:
    • Import certificate and create/replace md5 file
    • Reload Unifi controller software inside the container (Q: Is that needed and what's the best way to do so?)

What do you think? I'd be happy to provide a PR.

ThomDietrich commented 1 year ago

@jacobalberty another ping. Not sure why this issue doesn't receive more attention by other users. Short summary of the two issues discussed here:

  1. In some cases the md5 logic is flawed, let's get rid of it.
  2. Certificate files are loaded into the keystore once upon container start, not regularly as they should. This is a considerable issue with providers like Let's Encrypt, who issues certificates valid three months only. I am unsure how this can be resolved within the current image design
ThomDietrich commented 11 months ago

Posting this here for reference. I am using dnsrobocert to generate certificate files and a script similar to the one linked can be added to autocmd to bring certificate files into place and restart the unifi container. This is a good and proper workaround for a function the unifi container should deal with directly. https://gist.github.com/aaccioly/409205f5b87228cae7a69aafa31a0924