Arksine / moonraker

Web API Server for Klipper
https://moonraker.readthedocs.io
GNU General Public License v3.0
1.08k stars 414 forks source link

Symlink paths are not resolved in config #460

Closed djessup closed 2 years ago

djessup commented 2 years ago

What happened

I'm using LetsEncrypt to automate SSL certificates on my Pi, which runs Moonraker/Mainsail/Klipper. Since I had a "real" cert handy I plugged the details into Moonraker for ssl_certificate_path and ssl_key_path, but quickly found this wasn't working properly.

After checking the Moonraker log the problem started to become evident...

2022-07-11 20:19:39,252 [moonraker.py:main()] - Moonraker Error
Traceback (most recent call last):
  File "/home/pi/moonraker/moonraker/moonraker.py", line 454, in main
    server = Server(app_args, file_logger, event_loop)
  File "/home/pi/moonraker/moonraker/moonraker.py", line 89, in __init__
    self.moonraker_app = app = MoonrakerApp(config)
  File "/home/pi/moonraker/moonraker/app.py", line 179, in __init__
    config, 'ssl_certificate_path')
  File "/home/pi/moonraker/moonraker/app.py", line 240, in _get_path_option
    f"Invalid path for option '{option}', "
utils.ServerError: Invalid path for option 'ssl_certificate_path', /etc/letsencrypt/live/example.com-0001/fullchain.pem does not exist
2022-07-11 20:19:39,253 [moonraker.py:main()] - Server Shutdown

Of course I double checked the path and permissions, and it did in fact exist. LetsEncrypt organises files using symlinks, so /etc/letsencrypt/live/example.com-0001/fullchain.pem is just a symlink/pointer to /etc/letsencrypt/archive/example.com-0001/fullchain1.pem (private key is a similar arrangement).

I had a poke around the Moonraker code and tracked the issue down to the function _get_path_option in app.py (line 237):

        expanded = os.path.abspath(os.path.expanduser(path))

The abspath function will resolve and normalise relative paths just fine, but it has no awareness of symlinks. Fortunately realpath does handle symlinks, and otherwise behaves pretty much identically to abspath, so the fix to support symlinks is simply:

        expanded = os.path.realpath(os.path.expanduser(path))

Aside from working nicely with LetsEncrypt SSL certificates, I expect this fix will also add symlink support to any other config options that are read via the __get_path_option method 😜

I'll send through a PR which implements this, I just want to add some unit tests first.

Client

Mainsail, Other

Browser

Other or N/A

How to reproduce

  1. Create a symlink to an SSL certificate - e.g. ln -s ~/ssl/example.com.crt ~/ssl-symlinked/example.com.symlink.crt
  2. Update moonraker.conf to point ssl_certificate_path at your symlink
  3. Restart Moonraker
  4. Watch moonraker.log and refresh your Moonraker client (e.g. Mainsail) to see an error logged that the symlink path does not exist

Additional information

No response

Arksine commented 2 years ago

That seems strange. It shouldn't be necessary to resolve the symbolic link, as long as the link isn't broken os.path.exists() should return True. FWIW, when I create a symbolic link to a local certificate I can't reproduce this behavior.

djessup commented 2 years ago

Possibly something else is at play as well then, though my setup is pretty vanilla... ¯_(ツ)_/¯

I'll do some more testing and see if I can narrow it down some more, in case I'm conflating a permissions problem or the like!

Arksine commented 2 years ago

I would recommend launching the interpreter on your Pi then run os.stat(path_to_symlink). See if it raises an exception. That is all os.path.exists() does

Arksine commented 2 years ago

Any updates with this issue? It appears that it would be caused by a permissions issue, but if its not I'd be open to a fix in Moonraker. I just need to know what the cause of the issue is to decide on what the appropriate fix should be.

djessup commented 2 years ago

Apologies, ended up rebuilding the Pi for unrelated reasons and forgot to revisit this! I'll do some quick tests now and report back shortly.

djessup commented 2 years ago

Ok, looks like I was mistaken or had a broken setup earlier. It's working fine now w/o any modifications to Moonraker.

FWIW the reason I rebuilt was because I ran into issues trying to upgrade to Debian 11 and opted for a clean build rather than try to clean up the mess I'd made.

I guess it's possible some change in Deb11 fixed this, but I doubt it. More likely it was something permissions-related that I'd missed, though you'd think that would be an issue regardless of abspath vs realpath..?

Anyhow, I'll close this issue since I can no longer reproduce the problem. Sorry for wasting your time @Arksine .