-
Notifications
You must be signed in to change notification settings - Fork 29
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
always compare CNs as downcase #364
Conversation
manifests/apache.pp
Outdated
@@ -53,7 +53,7 @@ | |||
] | |||
|
|||
$api_additional_request_headers = $pulpcore::api_client_auth_cn_map.map |String $cn, String $pulp_user| { | |||
"set ${remote_user_environ_header} \"${pulp_user}\" \"expr=%{SSL_CLIENT_S_DN_CN} == '${cn}'\"" | |||
"set ${remote_user_environ_header} \"${pulp_user}\" \"expr=%{tolower:SSL_CLIENT_S_DN_CN} == '${cn.downcase}'\"" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is another occurence of this in https://github.com/theforeman/puppet-foreman_proxy_content/blob/e24f94194ebd7858f4ce9c34e5a671a30009873c/manifests/container.pp#L26-L27 which I don't understand where it's used from
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks interesting. That code only looks correct for content proxies, not Foreman itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
container gateway is a proxy-only feature :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, that isn't pulpcore::plugin::container
. It's used here: https://github.com/theforeman/puppet-foreman_proxy_content/blob/e24f94194ebd7858f4ce9c34e5a671a30009873c/manifests/init.pp#L254-L256
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yesterday I looked at the same problem and came to the same solution, just didn't have time to test it.
The problem is that in our Puppet code we (mostly) use the # facter networking.fqdn
centos9-STREAM.tanso.example.com However, in some places we use # hostname -f
centos9-stream.tanso.example.com The circumstances being
Now, if you have a system that thinks it's |
116c7d6
to
a3f0786
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm debating a Redmine issue for proper bug tracking.
I can certainly create one, sure. Should we also try to fix up the installer to use networking.fqdn if possible? |
FWIW, the "uppercase letter in fqdn" should be guarded by |
Probably a good idea.
I still see that check as a workaround for known and unknown bugs. Ideally we'll fix cases like these over time and drop the check. |
Sometimes people end up with certificates that have uppercase letters in the CN, but pass lowercase in the auth map.
Sometimes people end up with certificates that have uppercase letters in
the CN, but pass lowercase in the auth map.