Bug #6205

Custom SSL client cert for smart proxy based auth doesn't split CN correctly

Added by Jason Montleon about 4 years ago. Updated 8 days ago.

Status:Closed
Priority:Normal
Assignee:Ori Rabin
Category:Authorization
Target version:1.6.0
Difficulty:easy Team Backlog:
Triaged: Fixed in Releases:
Bugzilla link:1108740 Found in Releases:
Pull request:https://github.com/theforeman/foreman/pull/1678

Description

https://bugzilla.redhat.com/show_bug.cgi?id=1108740

in app/controllers/concerns/foreman/controller/smart_proxy_auth.rb

dn is evaluating with this format on RHEL 6:
/C=US/ST=North Carolina/O=FOREMAN/OU=PUPPET/CN=satellite1.montleon.intra

but on RHEL 7 it is coming up as:
CN=satellite2.montleon.intra,OU=PUPPET,O=FOREMAN,ST=North Carolina,C=US

so on:
https://github.com/theforeman/foreman/blob/develop/app/controllers/concerns/foreman/controller/smart_proxy_auth.rb#L44

this is causing $1 one from the match above to be:
"satellite2.montleon.intra,OU=PUPPET,O=FOREMAN,ST=North"

by changing request_hosts = [$1] to request_hosts = [$1.gsub(/,(\S+)/i, '')] it seems to work around the issue. I'm not sure if this is the best approach to fixing it or if someone can foresee a better way.

Associated revisions

Revision 2821b5e2
Added by Andrew N almost 4 years ago

fixes #6205 Changed regex to parse CNs from SSL DNs on separator chars

Revision add3bbdd
Added by Andrew N almost 4 years ago

fixes #6205 Changed regex to parse CNs from SSL DNs on separator chars

(cherry picked from commit 2821b5e250d2f311e2070c41879720f8745507cf)

History

#1 Updated by Dominic Cleal about 4 years ago

  • Category set to Authorization

#2 Updated by Dominic Cleal about 4 years ago

  • Bugzilla link set to https://bugzilla.redhat.com/show_bug.cgi?id=1108740

#3 Updated by Dominic Cleal about 4 years ago

  • Subject changed from Need to set restrict_registered_puppetmasters=false in foreman settings under auth in order for puppet runs to succeed on EL7 to Custom SSL client cert for smart proxy based auth doesn't split CN correctly

#4 Updated by Andrew N about 4 years ago

I'm trying to get Foreman installed at a client site and have been running into the above bug, but for different reasons. If you generate the PKI certs on windows, it will use "/" as the separation character. In addition the default regex will not pull only the CN entry, but anything after the CN as well. This was causing strange errors like the following:

/var/log/foreman/production.log:No smart proxy server found on ["foreman.linux.lab.local/emailAddress=user@example.com"] and is not in trusted_puppetmaster_hosts

The DN for the cert in question which was signed by a Windows CA is:

"/C=US/ST=NC/L=City/O=Example/OU=IT/CN=foreman.linux.lab.local/emailAddress=user@example.com" 


if https://github.com/theforeman/foreman/blob/develop/app/controllers/concerns/foreman/controller/smart_proxy_auth.rb#L41 is changed to the string below, the parse works for SSL certs which use "/" and "," as the separator.

dn =~ /CN=([^\s\/,]+)/i

#5 Updated by Ohad Levy about 4 years ago

  • Assignee set to Ori Rabin

#6 Updated by Ori Rabin about 4 years ago

Andrew, it looks like your solution is a good fix
would you like to send the pull request?

#7 Updated by Andrew N almost 4 years ago

Ori,
I'll see what I can do to put a pull together for it. Are there any appropriate pages I should read first?

#8 Updated by Andrew N almost 4 years ago

Ohad, Ori

I've almost got a pull request done, but I'm wondering if we might be going about this all wrong. I know with the openssl command line utilities it will often display the CN entry with "," and "/" characters as separators using it's default mode. There is an option to display the CN using only "," characters, I think the option option is an RFC related one, it's been a few weeks I don't exactly recall. Perhaps it would be best to first extract the CN using the appropriate format, then parse on that.

#9 Updated by The Foreman Bot almost 4 years ago

  • Status changed from New to Ready For Testing
  • Target version set to 1.7.5
  • Pull request https://github.com/theforeman/foreman/pull/1678 added

#10 Updated by Andrew N almost 4 years ago

Pull request #1678 created.

#11 Updated by Dominic Cleal almost 4 years ago

  • Legacy Backlogs Release (now unused) set to 10

#12 Updated by Andrew N almost 4 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF