albertito / chasquid

SMTP (email) server with a focus on simplicity, security, and ease of operation [mirror]
https://blitiri.com.ar/p/chasquid/
Other
868 stars 56 forks source link

Handle symlinks under the `certs/` directory #39

Closed erjoalgo closed 1 year ago

erjoalgo commented 1 year ago

I'm not sure if I am missing something but symlinks under certs/* weren't resolving for me despite following the recommendations for symlinking to the certbot directory and setting up permissions with setfacl.

I added some logging and noticed that isDir was false when traversing my site's certificate symlink in `chasquid.go:

       for _, info := range mustReadDir("certs/") {
                if !info.IsDir() {
                        // Skip non-directories.                                                               
                        continue
                }

And I also saw from the documentation(https://pkg.go.dev/io/fs#DirEntry) that the os.DirEntry from os.ReadDir, or at least os.DirEntry.Info, represents info about the symlink rather than the target:

...
// Info returns the FileInfo for the file or subdirectory described by the entry.
    // The returned FileInfo may be from the time of the original directory read
    // or from the time of the call to Info. If the file has been removed or renamed
    // since the directory read, Info may return an error satisfying errors.Is(err, ErrNotExist).
    // If the entry denotes a symbolic link, Info reports the information about the link itself,
    // not the link's target.
    Info() ([FileInfo](https://pkg.go.dev/io/fs#FileInfo), [error](https://pkg.go.dev/builtin#error))

After implementing this change I was able to follow certs/* symlinks.

albertito commented 1 year ago

Thanks for reporting this and sending a patch!

You're absolutely correct that currently symlinks inside of certs/ are not well handled. That is a bug.

Note that the suggestion in the how-to guide is to make the certs/ directory itself be a symlink, and that does work fine; however that is just an option, and symlinks inside should work.

About your patch, thanks again for sending it, it's always really appreciated. I think this can be done in a simpler way by not trying to detect directories entirely, and just skipping paths if there are no keys inside of them.

I will work on that approach for a little, and send you an alternative patch for this to see if it works for you.

albertito commented 1 year ago

I've written commit 888b2df4c1d833bc56e6cb39f691c06e16290da4 to fix this.

Would you mind giving it a try and see if it works well for you?

Thanks!

erjoalgo commented 1 year ago

Thanks for the quick response, I've tried out 888b2df and it works as expected.

albertito commented 1 year ago

Thanks a lot!