Project

General

Profile

Bug #6205

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

Added by Jason Montleon almost 5 years ago. Updated 10 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Authorization
Target version:
Difficulty:
easy
Triaged:
Bugzilla link:
Team Backlog:
Fixed in Releases:
Found in Releases:

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 (diff)
Added by Andrew N almost 5 years ago

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

Revision add3bbdd (diff)
Added by Andrew N over 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 almost 5 years ago

  • Category set to Authorization

#2 Updated by Dominic Cleal almost 5 years ago

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

#3 Updated by Dominic Cleal almost 5 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 almost 5 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 almost 5 years ago

  • Assignee set to Ori Rabin

#6 Updated by Ori Rabin almost 5 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 5 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 5 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 5 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
  • Pull request deleted ()

#10 Updated by Andrew N almost 5 years ago

Pull request #1678 created.

#11 Updated by Dominic Cleal almost 5 years ago

  • Legacy Backlogs Release (now unused) set to 10

#12 Updated by Andrew N almost 5 years ago

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

Also available in: Atom PDF