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

shlink 2.5 broke down /.well-known subfolder access #988

Closed
jonathandhn opened this issue Jan 25, 2021 · 6 comments · Fixed by #997
Closed

shlink 2.5 broke down /.well-known subfolder access #988

jonathandhn opened this issue Jan 25, 2021 · 6 comments · Fixed by #997
Labels
Milestone

Comments

@jonathandhn
Copy link

  • Shlink Version: 2.5.1
  • PHP Version: 7.4.14 with lsphp API
  • How do you serve Shlink: Self-hosted LiteSpeed
  • Database engine used: MariaDB 10.3

Summary

acme.sh and letsencrypt can't load pages from shlink root directory .well-known sub-folder and issues certificates anymore.

Current behavior

404 http error code is returned when trying loading ressources on shlink root directory .well-known sub-folder.

Expected behavior

.well-known url are not managed by Shlink, any service using .well-known sub-folder can works.

Fix

Disable rewrite mod and php on .well-known sub-folder to avoid shlink control.

The server config didn't change the last 90 days but both it and Shlink were working flawless 90 days ago on /.well-known subfolder access. Shlink was up to date at that time.

@acelaya
Copy link
Member

acelaya commented Jan 25, 2021

I'm not very familiar with lsphp or LiteSpeed. Could you try if it works with shlink 2.4.x with your current setup?

@Roy-Orbison
Copy link
Contributor

Litespeed is an Apache-compatible server. The problem is not the server, but the rewriting rules packaged with shlink, specifically this condition on the first rule

RewriteCond %{REQUEST_FILENAME} -s [OR]

should be

RewriteCond %{REQUEST_FILENAME} -f [OR]

because the -s excludes 0-byte files that certbot uses during HTTP challenges.

@acelaya
Copy link
Member

acelaya commented Feb 1, 2021

I was asking to try with an older version because that rule has been like that for ages and nobody has complained, so something has clearly changed in some external element.

This is the history of the file: https://github.com/shlinkio/shlink/commits/develop/public/.htaccess. Created almost 5 years ago and cero changes since then.

I took that file from here https://github.com/mezzio/mezzio-skeleton/blob/3.8.x/public/.htaccess, and there's no complains on that project either.

Also, in theory that file is for Apache. Does LiteSpeed read it as well?

@Roy-Orbison
Copy link
Contributor

LSWS reads Apache config files if configured to, and typically it is.

Regardless, the extra filesize check on top of an existence check is unwarranted, to me. I could not obtain a cert, so I consulted the .htaccess, a common source of such errors, and as soon as I changed it (#997), the certificate verification passed.

@acelaya
Copy link
Member

acelaya commented Feb 1, 2021

I'm wondering if maybe the certbot used to generate non-zero-bytes files in previous versions and that's why it used to work 🤔

Could you report this issue on the mezzio skeleton project and provide the PR there? https://github.com/mezzio/mezzio-skeleton

They have smarter people than me and I'll trust whatever they do 😅

@acelaya
Copy link
Member

acelaya commented Feb 1, 2021

Hey @Roy-Orbison. Thanks for creating the PR.

In the meantime I have looked a bit further in apache docs to understand those flags (I have to admit I basically trusted the file as it was), and it looks you are right on the change.

I'll add the link here for reference http://httpd.apache.org/docs/current/mod/mod_rewrite.html#rewritecond

-f
Is regular file.
Treats the TestString as a pathname and tests whether or not it exists, and is a regular file.
-s
Is regular file, with size.
Treats the TestString as a pathname and tests whether or not it exists, and is a regular file with size greater than zero.

I'll merge your PR. It seems safe to me.

@acelaya acelaya added this to the 2.6.0 milestone Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants