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

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

Open
ThomDietrich opened this issue Jun 9, 2019 · 8 comments
Open

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

ThomDietrich opened this issue Jun 9, 2019 · 8 comments

Comments

@ThomDietrich
Copy link
Contributor

ThomDietrich commented Jun 9, 2019

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
Copy link
Contributor Author

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

@github-actions
Copy link

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.

@github-actions github-actions bot added the Stale label Jan 29, 2022
@ThomDietrich
Copy link
Contributor Author

Still relevant.

@jacobalberty
Copy link
Owner

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
Copy link
Contributor

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
Copy link
Contributor Author

ThomDietrich commented Oct 24, 2022

@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
Copy link
Contributor Author

ThomDietrich commented Feb 11, 2023

@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
Copy link
Contributor Author

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

@ThomDietrich ThomDietrich changed the title Certs import missed after backup restore Certs import flawed, no import of new certs during runtime Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants