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

Allow to configure other valids referrers for HTTPS #7344

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Et7f3
Copy link

@Et7f3 Et7f3 commented May 16, 2024

This check was added to protect against possible CRSF

However it doesn't works well behind a proxy like we saw here. This change allow to define multiple valid referers (that would solve the issue) but also related issued https://lists.fedorahosted.org/archives/list/freeipa-users@lists.fedorahosted.org/thread/VN3RXS36GFK4JMZCCSHPJ3DKLSBEXDE4/ in the precedent message the user have to comment this check to have a working freeipa. Better to accept multiple referers so it works.

I remember comment about kerberos might not work for the added domains. Since we don't use kerberos we are not affected. It seems still possible to configure it.

User with kerberos won't see change because they will keep using the classical name.

possible improvements:

  • use only one field ? I see it in many place so I think it is safest to add a non-conflicting field. It also don't overload usage.
  • Is it really optional ?
  • Do we extend config.Env to support list ?
  • strip space between value ? Or don't and apply like the manual show ?

This check was added to protect [against possible CRSF](freeipa@2d6eeb2#diff-21d951ac2d07631c0818b056e289cd02d980b05545f9eabb18b407178da0af0c)

However it doesn't works well behind a proxy like [we saw here](haproxy/haproxy#2555 (comment)). This change allow to define multiple valid referers (that would solve the issue) but also related issued https://lists.fedorahosted.org/archives/list/freeipa-users@lists.fedorahosted.org/thread/VN3RXS36GFK4JMZCCSHPJ3DKLSBEXDE4/
in the precedent message the user have to comment this check to have a working freeipa. Better to accept multiple referers so it works.

I remember comment about kerberos might not work for the added domains. Since we don't use kerberos we are not affected.
It seems still possible to [configure it](https://freeipa-users.redhat.narkive.com/hClHC8Ny/ipa-server-ui-behind-proxy).

User with kerberos won't see change because they will keep using the classical name.
Comment on lines +103 to +104
.IP
allowed_referers = ipa.demo1.freeipa.org:443,ipa.demo1.local,ipa.demo1.freeipa.org:443/sub_folder
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might not show the sub_folder aspect to be future proof ?

@abbra
Copy link
Contributor

abbra commented May 16, 2024

Frankly speaking, I don't like to add a half-baked solution. A proper support for multi-host setup (including reverse proxying) would need to take into account more than just a referrer aliases. Please see https://vda.li/en/posts/2023/08/16/Support-multi-homed-FreeIPA-Server/ for an analysis I did.

More comments:

  • env constant definition is missing. This means if allowed_referers is not present in the config, api.env.allowed_referers will throw an exception:
>>> api.env.allowed_referers
Traceback (most recent call last):
  File "<console>", line 1, in <module>
AttributeError: 'Env' object has no attribute 'allowed_referers'

The discussion you are linking to has a reference to my old draft PR (https://github.com/abbra/freeipa/pull/9/files) which implements most part of the aliasing support already.

@abbra
Copy link
Contributor

abbra commented May 17, 2024

This is actually can be seen in the CI tests:

[Thu May 16 18:23:27.406873 2024] [wsgi:error] [pid 7089:tid 7403] [remote 2001:db8:1:1::2:60302] ipa: ERROR: WSGI jsonserver_kerb.__call__():
[Thu May 16 18:23:27.406901 2024] [wsgi:error] [pid 7089:tid 7403] [remote 2001:db8:1:1::2:60302] Traceback (most recent call last):
[Thu May 16 18:23:27.406916 2024] [wsgi:error] [pid 7089:tid 7403] [remote 2001:db8:1:1::2:60302]   File "/usr/lib/python3.12/site-packages/ipaserver/rpcserver.py", line 498, in __call__
[Thu May 16 18:23:27.406921 2024] [wsgi:error] [pid 7089:tid 7403] [remote 2001:db8:1:1::2:60302]     response = self.wsgi_execute(environ)
[Thu May 16 18:23:27.406925 2024] [wsgi:error] [pid 7089:tid 7403] [remote 2001:db8:1:1::2:60302]                ^^^^^^^^^^^^^^^^^^^^^^^^^^
[Thu May 16 18:23:27.406930 2024] [wsgi:error] [pid 7089:tid 7403] [remote 2001:db8:1:1::2:60302]   File "/usr/lib/python3.12/site-packages/ipaserver/rpcserver.py", line 394, in wsgi_execute
[Thu May 16 18:23:27.406934 2024] [wsgi:error] [pid 7089:tid 7403] [remote 2001:db8:1:1::2:60302]     allowed_referers = self.api.env.allowed_referers.split(',')
[Thu May 16 18:23:27.406938 2024] [wsgi:error] [pid 7089:tid 7403] [remote 2001:db8:1:1::2:60302]                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Thu May 16 18:23:27.406943 2024] [wsgi:error] [pid 7089:tid 7403] [remote 2001:db8:1:1::2:60302] AttributeError: 'Env' object has no attribute 'allowed_referers'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants