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

Don't set proxy_bind and instead rely on default behaviour. #1072

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

Irishsmurf
Copy link
Contributor

@Irishsmurf Irishsmurf commented Sep 10, 2023

Setting proxy_bind to $server_addr can cause issues if the connecting client is attempting to connect to a different IP protocol address than the backend supports (IPv6 vs IPv4)

This removes explicitly setting the proxy_bind setting, and instead relies on the default nginx behaviour (of binding to the same protocol as the upstream service)

Description

calibreweb currently doesn't work with IPv6 as internally, CalibreWeb binds to an IPv4 address 127.0.0.1 - and if you attempt to hit Nginx with a client using IPv6 - $server_addr will be an IPv6 address - preventing it to talk to the upstream IPv4 address

Fixes issues:

Proposed Changes:

  • Changing /scripts/nginx/calibreweb.sh to remove setting proxy_bind, and instead rely on default behaviour.

Change Categories

  • Bug fix

Checklist

  • [✅] Branch was made off the develop branch and the PR is targetting the develop branch
  • [✅] Docs have been made OR are not necessary
  • [✅] Changes to panel have been made OR are not necessary
  • [✅] Code conforms to project structure (See more)
  • [✅] Prints to terminal are handled (See more)
  • [✅] I have commented my code, particularly in hard-to-understand areas
  • [✅] Testing was done
    • [✅] Tests created or no new tests necessary
    • [✅] Tests executed

Test scenarios

I manually modified the Nginx Configuration.

  1. Without the change, it errors with 500 Internal Error
  2. With the change, the page loads over both IPv4 and IPv6

Architectures

amd64 armhf arm64 Unspecified
Jammy
Focal
Bookworm
Bullseye
Buster
Raspbian ⚫️ ⚫️ ⚫️

✅❎ Passed

🛠🛠 TODO

❌❌ Currently failing

Setting `proxy_bind` to `$server_addr` can cause issues if the connecting client is attempting to connect to a different IP protocol address than the backend supports (IPv6 vs IPv4)

This removes explicitly setting the `proxy_bind` setting, and instead relies on the default nginx behaviour (of binding to the Calibre Web localhost)
@liaralabs
Copy link
Member

Thanks for doing all the legwork on this.

Can you please supply a script for the update directory and add a case to the existing calibreweb.sh?

I'll leave the exact mechanism up to you, but the gist is that we should check wither proxy_bind is in the nginx conf file and set the config correctly.

A simple mechanism would be to simply trigger the nginx/calibreweb.sh script as it should do what we need to already

@Irishsmurf
Copy link
Contributor Author

Hey,

I've updated calibreweb.sh which adds a case which will overwrite an existing nginx config for CalibreWeb with one which removes the proxy_bind setting.

I took inspiration from mango.sh - which also overwrites an existing nginx config.

Is there something specific that's needed to be done to trigger this, or will this just run when calling box update?
I took a look at the update directory and it seems everything is merely tied to an app-specific shell script - and there's no triggers that I can see outside of that.

Removes `proxy_bind` from CalibreWeb's nginx configuration by checking if the line
exists.

If it exists, remove it using `sed`.
@liaralabs
Copy link
Member

Yeah, you don't need to worry about that -- the update/upgrade hooks run all scripts in the update directory so nothing further than writing the hook is involved.

@liaralabs liaralabs merged commit a67c1ca into swizzin:develop Nov 7, 2023
1 check passed
@Irishsmurf Irishsmurf deleted the develop branch November 7, 2023 10:33
sMtzDczDq added a commit to sMtzDczDq/swizzin that referenced this pull request Nov 15, 2023
commit 3dcce8c
Merge: a474a46 5253f36
Author: sMtzDczDq <75165692+sMtzDczDq@users.noreply.github.com>
Date:   Wed Nov 15 02:18:59 2023 +0100

    Merge branch 'swizzin:master' into master

commit 5253f36
Author: liara <liara@swizzin.ltd>
Date:   Mon Nov 6 18:42:31 2023 -0800

    SECURITY(rslsync): bump to 3.10.0. See changelog

commit 42f8ae6
Author: siedenburg2 <mirco@legionis-stuff.de>
Date:   Tue Nov 7 03:31:06 2023 +0100

    Update to support qBittorrent version 4.6 (swizzin#1088)

    Added lines for version 4.6

commit 1528982
Author: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date:   Mon Nov 6 18:30:37 2023 -0800

    [pre-commit.ci] pre-commit autoupdate (swizzin#1085)

    updates:
    - [github.com/shellcheck-py/shellcheck-py: v0.9.0.5 → v0.9.0.6](shellcheck-py/shellcheck-py@v0.9.0.5...v0.9.0.6)
    - [github.com/pre-commit/pre-commit-hooks: v4.4.0 → v4.5.0](pre-commit/pre-commit-hooks@v4.4.0...v4.5.0)

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

commit daedc5a
Author: stickz <stickman002@mail.com>
Date:   Mon Nov 6 21:30:01 2023 -0500

    rTorrent: Add latest development fixes (swizzin#1084)

    * rTorrent: Fix scgi requests

    Fixes a problem where xmlrpc-c information fails to update if user is running too many torrents.

    * rTorrent: Fix session files

    Resolves a data corruption issue with torrent session files during a power loss.

    * rTorrent: Fix xmlrpc crash

    Resolves a rTorrent crash caused by sending invalid xmlrpc logic.

    * rTorrent: Fix memory leaks

    Resolves various memory leaks with rtorrent software.

    * rTorrent: Fix Piece boundaries

    Resolves an annoying bug with rtorrent where torrent progress is reported as 200%.

    * rTorrent: Improve UDNS performance

    Implement lookup cache for torrent hashes. Improve torrent session loading speed.

commit a67c1ca
Author: David K <dave@paddez.com>
Date:   Tue Nov 7 02:29:18 2023 +0000

    Don't set `proxy_bind` and instead rely on default behaviour. (swizzin#1072)

    * Don't set `proxy_bind` and instead rely on default behaviour.

    Setting `proxy_bind` to `$server_addr` can cause issues if the connecting client is attempting to connect to a different IP protocol address than the backend supports (IPv6 vs IPv4)

    This removes explicitly setting the `proxy_bind` setting, and instead relies on the default nginx behaviour (of binding to the Calibre Web localhost)

    * Remove proxy_bind setting via the update script.

    Removes `proxy_bind` from CalibreWeb's nginx configuration by checking if the line
    exists.

    If it exists, remove it using `sed`.

commit 1b58511
Author: Brett Petch <brettpetch@icloud.com>
Date:   Mon Nov 6 21:27:49 2023 -0500

    npm: depreciate nodesource script (swizzin#1090)

    Removes nodesource script due to depreciation notice.

commit be244c0
Author: liaralabs <30909703+liaralabs@users.noreply.github.com>
Date:   Mon Nov 6 18:27:11 2023 -0800

    rslsync: don't skip eula (skips forced passwd) (swizzin#1091)

    * rslsync: don't skip eula (skips forced passwd)

    * [pre-commit.ci] auto fixes from pre-commit.com hooks

    for more information, see https://pre-commit.ci

    * Fix syntax

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

commit 77e973e
Author: Wyall <lars.klompmaker@googlemail.com>
Date:   Tue Oct 3 23:53:03 2023 +0200

    qbittorrent: Fix interface grep  (swizzin#1086)

    * qbittorrent: Fix interface grep

    '*' needs to be escaped

    * qbittorrent: Fix interface grep in nginx scripts

    Escape *
sMtzDczDq added a commit to sMtzDczDq/swizzin that referenced this pull request Nov 15, 2023
* Update qbittorrent to support Version 4.6

Added lines for version 4.6

* letsencrypt.sh

* Update letsencrypt.sh

* jfoaijfie

* Revert "jfoaijfie"

This reverts commit fa5ddad.

* Revert "fix(letsencrypt): Set LE as default certificate authority"

This reverts commit 2cd8545.

Conflicts:
	scripts/install/letsencrypt.sh

* Update letsencrypt.sh and zerossl.sh

* Update .gitignore

* Update .gitignore

* Squashed commit of the following:

commit 3dcce8c
Merge: a474a46 5253f36
Author: sMtzDczDq <75165692+sMtzDczDq@users.noreply.github.com>
Date:   Wed Nov 15 02:18:59 2023 +0100

    Merge branch 'swizzin:master' into master

commit 5253f36
Author: liara <liara@swizzin.ltd>
Date:   Mon Nov 6 18:42:31 2023 -0800

    SECURITY(rslsync): bump to 3.10.0. See changelog

commit 42f8ae6
Author: siedenburg2 <mirco@legionis-stuff.de>
Date:   Tue Nov 7 03:31:06 2023 +0100

    Update to support qBittorrent version 4.6 (swizzin#1088)

    Added lines for version 4.6

commit 1528982
Author: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date:   Mon Nov 6 18:30:37 2023 -0800

    [pre-commit.ci] pre-commit autoupdate (swizzin#1085)

    updates:
    - [github.com/shellcheck-py/shellcheck-py: v0.9.0.5 → v0.9.0.6](shellcheck-py/shellcheck-py@v0.9.0.5...v0.9.0.6)
    - [github.com/pre-commit/pre-commit-hooks: v4.4.0 → v4.5.0](pre-commit/pre-commit-hooks@v4.4.0...v4.5.0)

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

commit daedc5a
Author: stickz <stickman002@mail.com>
Date:   Mon Nov 6 21:30:01 2023 -0500

    rTorrent: Add latest development fixes (swizzin#1084)

    * rTorrent: Fix scgi requests

    Fixes a problem where xmlrpc-c information fails to update if user is running too many torrents.

    * rTorrent: Fix session files

    Resolves a data corruption issue with torrent session files during a power loss.

    * rTorrent: Fix xmlrpc crash

    Resolves a rTorrent crash caused by sending invalid xmlrpc logic.

    * rTorrent: Fix memory leaks

    Resolves various memory leaks with rtorrent software.

    * rTorrent: Fix Piece boundaries

    Resolves an annoying bug with rtorrent where torrent progress is reported as 200%.

    * rTorrent: Improve UDNS performance

    Implement lookup cache for torrent hashes. Improve torrent session loading speed.

commit a67c1ca
Author: David K <dave@paddez.com>
Date:   Tue Nov 7 02:29:18 2023 +0000

    Don't set `proxy_bind` and instead rely on default behaviour. (swizzin#1072)

    * Don't set `proxy_bind` and instead rely on default behaviour.

    Setting `proxy_bind` to `$server_addr` can cause issues if the connecting client is attempting to connect to a different IP protocol address than the backend supports (IPv6 vs IPv4)

    This removes explicitly setting the `proxy_bind` setting, and instead relies on the default nginx behaviour (of binding to the Calibre Web localhost)

    * Remove proxy_bind setting via the update script.

    Removes `proxy_bind` from CalibreWeb's nginx configuration by checking if the line
    exists.

    If it exists, remove it using `sed`.

commit 1b58511
Author: Brett Petch <brettpetch@icloud.com>
Date:   Mon Nov 6 21:27:49 2023 -0500

    npm: depreciate nodesource script (swizzin#1090)

    Removes nodesource script due to depreciation notice.

commit be244c0
Author: liaralabs <30909703+liaralabs@users.noreply.github.com>
Date:   Mon Nov 6 18:27:11 2023 -0800

    rslsync: don't skip eula (skips forced passwd) (swizzin#1091)

    * rslsync: don't skip eula (skips forced passwd)

    * [pre-commit.ci] auto fixes from pre-commit.com hooks

    for more information, see https://pre-commit.ci

    * Fix syntax

    ---------

    Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

commit 77e973e
Author: Wyall <lars.klompmaker@googlemail.com>
Date:   Tue Oct 3 23:53:03 2023 +0200

    qbittorrent: Fix interface grep  (swizzin#1086)

    * qbittorrent: Fix interface grep

    '*' needs to be escaped

    * qbittorrent: Fix interface grep in nginx scripts

    Escape *

---------

Co-authored-by: siedenburg2 <mirco@legionis-stuff.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants