Skip to content
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

Drop certs and ensure ownership of keys #351

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Jun 17, 2020

Draft changes from playing around with new proof of concept certificate handling tool. The hardcoded defaults will need updates and exposure of these certificates to be configure directly. This falls in line with the proposed changes to refactor foreman-installer entry points:

theforeman/foreman-installer#421

@ehelms ehelms force-pushed the ownership-certs branch from d375697 to b4699d4 Compare July 6, 2020 14:50
Comment on lines +41 to 54
file { $pulp_client_key:
group => $foreman::group,
mode => '0640',
} ~>
foreman_config_entry { 'pulp_client_cert':
value => $pulp_client_cert,
ignore_missing => false,
require => Foreman::Rake['db:seed'],
} ~>
foreman_config_entry { 'pulp_client_key':
value => $certs::pulp_client::client_key,
value => $pulp_client_key,
ignore_missing => false,
require => [Class['certs::pulp_client'], Foreman::Rake['db:seed']],
require => Foreman::Rake['db:seed'],
}
Copy link
Member

Choose a reason for hiding this comment

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

Just a thought: can we configure Pulp 3 to use the internal CA for client auth just like we do with Foreman? That way we can use the same Foreman setting to connect to Smart Proxies and simplifies the application overall.

Perhaps @jlsherrill can share his thoughts as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

The Pulp client certificates use a CN of pulp to identify the user who is authenticating. There might be room to switch this to whatever the full DN of the client certificates is. We have client certificates needed for Candlepin Artemis connection as well, and this follows the DN style: https://github.com/theforeman/puppet-certs/blob/master/manifests/candlepin.pp#L32

In the newer certificate setup, I have simplified a lot of this down to really just setting CN since the rest of the attributes are largely arbitrary crap we were putting in there.

So I think this could work, if Pulp/Django will allow a "hostname" for a user.

Copy link
Member

Choose a reason for hiding this comment

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

Do we only allow a single user (pulp) or are there more? If there's just 1, we can let Apache validate and statically pass it to the backend.

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +36 to 39
file { $candlepin_events_ssl_key:
group => $foreman::group,
mode => '0640',
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we also reuse the Foreman client cert, at least by default? Perhaps it needs application changes, but it makes the application easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants