Allow to configure other valids referrers for HTTPS #7344
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Et7f3
commented
May 16, 2024
client/man/default.conf.5
Outdated
Comment on lines
103
to
104
| .IP | ||
| allowed_referers = ipa.demo1.freeipa.org:443,ipa.demo1.local,ipa.demo1.freeipa.org:443/sub_folder |
There was a problem hiding this comment.
We might not show the sub_folder aspect to be future proof ?
|
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:
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.
|
|
This is actually can be seen in the CI tests: |
84b4099 to
2815ee9
Compare
Et7f3
commented
Jul 17, 2024
Comment on lines
+100
to
+112
| .TP | ||
| .B allowed_referers [<hostname>[,<hostname>][,<hostname>:<exotic_port>][,<hostname>:<exotic_port>[/path]][...]] | ||
| Specifies additional allowed hostnames that can be referred to in the Referer HTTP header. This setting allows to access the application from multiple leg (behind reverse-proxy or multiple FQDN). Please note that TLS server certificate used by IPA web server must also include these hostnames or otherwise direct HTTPS connection using the alternative names will not be trusted by your browers. | ||
| .RS | ||
| .IP | ||
| allowed_referers = ipa.demo1.freeipa.org:443,ipa.demo1.local,ipa.demo1.freeipa.org:443/sub_folder | ||
| .RE | ||
| .IP | ||
| Here it is an example of the X509v3 Subject Alternative Name extension filled for certificate request using OpenSSL/LibreSSL: | ||
| .RS | ||
| .IP | ||
| subjectAltName = DNS:ipa.demo1.freeipa.org,DNS:ipa.demo1.local | ||
| .RE |
There was a problem hiding this comment.
I have added an example how to update the certificate and added searchable word/example.
|
When you get May be replace that by |
Et7f3
commented
Jul 19, 2024
Et7f3
commented
Jul 19, 2024
|
I fixed Lint warning: The Tox failure seems ot be related to Lint failure ? but weird it triggered only on my line. |
|
Tox failure: Basically, default config returns |
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. Signed-off-by: Et7f3 <cadeaudeelie@gmail.com>
|
Since the default value is now I see we can have string as default value: Line 206 in 58c1fdd None Line 212 in 58c1fdd I searched for usage and found this: So it seems only filling constants.py should be enough. I also see this file (that seems to define value for pylint ?). Since it is pylint that is not happy should I add here also a line ? Line 478 in 58c1fdd Line 486 in 58c1fdd Or maybe I should modify something in test_config.py so default value is not |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: