Issue 68080043: SSL for ostack and a service CA

Can't Edit
Can't Publish+Mail
Start Review

Created:
10 years, 2 months ago by hazmat

Modified:
10 years, 2 months ago

Reviewers:
jamespage, mp+207896

Visibility:
Public.

Description

SSL for ostack and a service CA Provides additional functionality for openstack AMQP and SharedDb contexts to allow for ssl usage from rabbitmq and mysql. Also provides a certificate authority implmentation used for a service to manage its own ca (for itself and its clients). https://code.launchpad.net/~hazmat/charm-helpers/ssl-everywhere/+merge/207896 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 14

Patch Set 2 : SSL for ostack and a service CA #

Created: 10 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+332 lines, -10 lines)			Patch
A	[revision details]	View	1	1 chunk	+2 lines, -0 lines	0 comments	Download
M	charmhelpers/contrib/openstack/context.py	View	1	7 chunks	+53 lines, -6 lines	0 comments	Download
A	charmhelpers/contrib/ssl/service.py	View	1	1 chunk	+267 lines, -0 lines	0 comments	Download
M	tests/contrib/openstack/test_os_contexts.py	View	1	1 chunk	+10 lines, -4 lines	0 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

jamespage

In addition this merge proposal introduces quite a few test failures (make test). https://codereview.appspot.com/68080043/diff/1/charmhelpers/contrib/openstack/context.py File ...

10 years, 2 months ago (2014-02-24 14:27:55 UTC) #2

hazmat

Thanks for the review, looking at test failures now. Inline for the rest https://codereview.appspot.com/68080043/diff/1/charmhelpers/contrib/openstack/context.py File ...

10 years, 2 months ago (2014-02-24 17:05:19 UTC) #3

Thanks for the review, looking at test failures now. Inline for the rest

https://codereview.appspot.com/68080043/diff/1/charmhelpers/contrib/openstack...
File charmhelpers/contrib/openstack/context.py (right):

https://codereview.appspot.com/68080043/diff/1/charmhelpers/contrib/openstack...
charmhelpers/contrib/openstack/context.py:148: 'database_host':
rdata.get('db_host'),
On 2014/02/24 14:27:56, jamespage wrote:
> I should probably know this but does .get on a dict return None if not found?

It does return None on not found.

https://codereview.appspot.com/68080043/diff/1/charmhelpers/contrib/openstack...
charmhelpers/contrib/openstack/context.py:161: 'database_ssl_cert': ''})
On 2014/02/24 14:27:56, jamespage wrote:
> I'd be wary of setting these defaults - context_complete scans for None values
> to ensure that everything has been found.

context_complete all checks empty strings.. all the ssl stuff happens post
context_complete, cause its optional. i was doing empty value initialization
cause i wasn't sure about jinja2's behavior.. they have a separate 'defined'
command in template. i'll check and remove if that works okay.

https://codereview.appspot.com/68080043/diff/1/charmhelpers/contrib/openstack...
charmhelpers/contrib/openstack/context.py:168: log("Charm not setup for ssl
support but ssl ca found")
On 2014/02/24 14:27:56, jamespage wrote:
> I'm not sure I understand this - does the charm consuming mysql services have
to
> be setup for SSL as well?

The context helper needs to be instantiated with the ssl_dir at min, this was to
warn against usage without ssl_dir, but then getting passed ssl relation data
and not being able to handle it. ie the charm needs an update.

https://codereview.appspot.com/68080043/diff/1/charmhelpers/contrib/openstack...
charmhelpers/contrib/openstack/context.py:175: time.sleep(60)
On 2014/02/24 14:27:56, jamespage wrote:
> What else would be trying to create this?

So this comes up when mysql is operating as its own ca in ssl: only mode, where
it will create client certs to hand to clients. MySQL is pretty picky about its
certs signing, i was only able to get it to play nice with the x509 subcommand,
but that command doesn't let you specify a startDate (the option is display only
there) which leads to the scenario that a user gets a cert, and they can't use
it till the minute on  the startDate of the cert rolls over, hence the sleep
here which happens only the first time we get a client cert (ie doesn't already
exist on disk). If mysql is being handed external certs, it won't do the client
cert thing.

https://codereview.appspot.com/68080043/diff/1/charmhelpers/contrib/openstack...
charmhelpers/contrib/openstack/context.py:181:
fh.write(b64decode(rdata['ssl_key']))
On 2014/02/24 14:27:56, jamespage wrote:
> I'm assuming the ssl_key is a client key for access? In which case the
> permissions on this file should be limited as appropriate - probably to a
> relevant group but this could be a little fiddly as the openstack user who
needs
> access will vary by charm.

Yeah.. its a client cert and cert private key, it probably should be protected,
which means growing additional params for user at min for writing to disk given
the variance on charm users.

https://codereview.appspot.com/68080043/diff/1/charmhelpers/contrib/openstack...
charmhelpers/contrib/openstack/context.py:280: ctxt.setdefault('rabbit_ssl_ca',
'')
On 2014/02/24 14:27:56, jamespage wrote:
> Ditto comment about using empty strings for values - leaving these unset is OK
> IMHO - the jinja2 templates deal with this ok

yeah.. i'll double check the jinja template behavior and yank if extraneous.

hazmat

tests passing (minus one failure from a gui test that also fails for me on ...

10 years, 2 months ago (2014-02-24 17:40:36 UTC) #4

Please take a look.

Expand All Messages | Collapse All Messages