Skip to content
This repository has been archived by the owner on Nov 24, 2021. It is now read-only.

fixes #22196 - configure hammer SSL CA certificate bundle #574

Merged
merged 1 commit into from
Jan 10, 2018

Conversation

stbenjam
Copy link
Contributor

@stbenjam stbenjam commented Jan 9, 2018

No description provided.

@theforeman-bot
Copy link

Issues: #22196

@@ -0,0 +1,5 @@
if answers['foreman::cli'].is_a? Hash # If user has already specified options, set the SSL CA file
answers['foreman::cli']['ssl_ca_file'] = '/etc/pki/katello/certs/katello-server-ca.crt'
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it would be a good practice to extract this from the certs answers or if that'd create more problems because it might not be available yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did Kafo change? foreman::cli is at the top level of the installer, so I think it has to be. The certs are hardcoded like this for all the modules :-(

Copy link
Member

@ekohl ekohl Jan 10, 2018

Choose a reason for hiding this comment

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

It didn't, but I wondered if we could use answers to get this value and reduce duplication. Looks like it's done in code (https://github.com/theforeman/puppet-certs/blob/60fad4d1057fca79b718c43d16d633be75e3d42d/manifests/init.pp#L110) so it's not going to work anyway.

@ekohl ekohl merged commit a7ba5e8 into Katello:master Jan 10, 2018
@ekohl
Copy link
Member

ekohl commented Jan 10, 2018

@mbacovsky mentioned theforeman/puppet-foreman@58f1911 should fix this already.

@stbenjam stbenjam deleted the 22196 branch January 10, 2018 13:26
@stbenjam
Copy link
Contributor Author

stbenjam commented Jan 10, 2018

Oh I missed that. But, it picks the wrong cert anyway when using custom certs. default-ca is the katello-generated CA, and server-ca is the one from custom certs.

This is what it produces without this PR:

:ssl:
  :ssl_ca_file: '/etc/pki/katello/certs/katello-default-ca.crt'

Because of how we have answers set:

server_ssl_ca: /etc/pki/katello/certs/katello-default-ca.crt
server_ssl_chain: /etc/pki/katello/certs/katello-server-ca.crt

Can we reverse the CA and Chain? I'm honestly not sure how this works, I think we take the custom certs they provide and then sign them with the Katello generated CA. @ehelms would know

@ehelms
Copy link
Member

ehelms commented Jan 10, 2018

I don't mind if we reverse them, that shouldn't have an affect since both are checked. I think the current setting is technically correct though. According to Apache This directive sets the optional all-in-one file where you can assemble the certificates of Certification Authorities (CA) which form the certificate chain of the server certificate.

Whereas the server_ssl_ca is for CAs of the clients with whom you deal with and our client certificates (e.g. rhsm) are signed by the default-ca.

@ekohl
Copy link
Member

ekohl commented Jan 10, 2018

@stbenjam I think you're right that we should be preferring the chain since that's everything CA does + allows client certification authentication. See https://stackoverflow.com/a/5543737

@stbenjam
Copy link
Contributor Author

stbenjam commented Jan 10, 2018

@ekohl So do you think Foreman should send the CA chain to hammer instead (if there is one)?

@ehelms
Copy link
Member

ehelms commented Jan 10, 2018

Agreed with the chain file -- it signed the server certificates and hammer is wanting to verify the server.

@ekohl
Copy link
Member

ekohl commented Jan 10, 2018

https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#SSLCertificateChainFile is not that clear to me, but I think it supports that we should reverse the order:

This should be used alternatively and/or additionally to SSLCACertificatePath for explicitly constructing the server certificate chain which is sent to the browser in addition to the server certificate. It is especially useful to avoid conflicts with CA certificates when using client authentication. Because although placing a CA certificate of the server certificate chain into SSLCACertificatePath has the same effect for the certificate chain construction, it has the side-effect that client certificates issued by this same CA certificate are also accepted on client authentication.

@stbenjam
Copy link
Contributor Author

stbenjam commented Jan 10, 2018

I don't think so, whatever CA we put in SSLCACertificatePath will be allowed to use client certificates against the server, right?

That means for example, in the case of a corp with a PKI that issues client certificates to users too, they'll be able to authenticate to the Foreman with those certificates and it may not be desired. Worse, any client certificate from a large commercial CA would be acceptable in the case of something signed by Symantec.

@stbenjam
Copy link
Contributor Author

@ehelms
Copy link
Member

ehelms commented Jan 10, 2018

I agree with @stbenjam here. the Chain file should be the CA (or chain of CAs) that signed the server certificates.

@ekohl
Copy link
Member

ekohl commented Jan 10, 2018

@stbenjam yes, I think it could be that simple.

@stbenjam
Copy link
Contributor Author

I've opened PR's to reverse this and with the alternate fix to puppet-foreman. Sorry for the mess of PR's here, but I think this ends up being a better solution. Less hard coded paths in the answers file is better.

zjhuntin pushed a commit to zjhuntin/katello-installer that referenced this pull request Oct 30, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants