Skip to content

Key IDs have the http scheme #3567

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

Open
silverpill opened this issue Apr 30, 2025 · 2 comments
Open

Key IDs have the http scheme #3567

silverpill opened this issue Apr 30, 2025 · 2 comments

Comments

@silverpill
Copy link

Describe the bug

Identifiers of public keys have the http scheme.

To Reproduce

Example of a Person actor: https://books.infosec.exchange/user/ajn142

Its publicKey.id is http://books.infosec.exchange/user/ajn142/#main-key

Expected behavior

When an actor has an https ID, I expect the attached key to have an https ID too.

@silverpill silverpill added the bug Something isn't working label Apr 30, 2025
@hughrun
Copy link
Contributor

hughrun commented May 1, 2025

This will happen when using a reverse-proxy if at the time the user account was created, USE_HTTPS is False, or USE_HTTPS is not set and DEBUG is True:

# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = env.bool("DEBUG", True)
USE_HTTPS = env.bool("USE_HTTPS", not DEBUG)

That determines the PROTOCOL, which is part of BASE_URL:

PROTOCOL = "http"
if USE_HTTPS:
PROTOCOL = "https"
SESSION_COOKIE_SECURE = True
CSRF_COOKIE_SECURE = True
PORT = env.int("PORT", 443 if USE_HTTPS else 80)
if (USE_HTTPS and PORT == 443) or (not USE_HTTPS and PORT == 80):
NETLOC = DOMAIN
else:
NETLOC = f"{DOMAIN}:{PORT}"
BASE_URL = f"{PROTOCOL}://{NETLOC}"

And finally BASE_URL is used when setting the user's remote_id (which is used as ActivityPub's id):

        with transaction.atomic():
            # populate fields for local users
            self.remote_id = f"{BASE_URL}/user/{self.localname}"

https://github.com/bookwyrm-social/bookwyrm/blob/0cc5106297e901a2b9a2766bc42f7b920ecb1c34/bookwyrm/models/user.py#L369C1-L371C65

If you have HTTP => HTTPS forwarding, this isn't such a big deal and webfinger should return the correct value, but it's obviously not ideal.

@ilkka-ollakka @mouse-reeve I noticed this when I was reviewing #3543 and wondered if we should change the default value of DEBUG to False and potentially need to spell out more clearly how to correctly configure reverse-proxies so as not to accidentally do this. Defaulting DEBUG to False is safer in that you would have to change the value in order to run DEBUG in production (which obviously you should not), and also would therefore default USE_HTTPS to True. Or perhaps even more safely, we could simply get rid of USE_HTTPS and rely on the DEBUG value – though I guess if someone really needs to briefly run DEBUG in prod that would cause problems. USE_HTTPS = False is only ever going to be appropriate if someone is running a dev system through localhost. Thoughts?

@ilkka-ollakka
Copy link
Contributor

ilkka-ollakka commented May 1, 2025

I think it is good idea to change DEBUG=False as default value. I don't see big issues with USE_HTTPS=True either.

@hughrun hughrun added deployment and removed bug Something isn't working labels May 2, 2025
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