Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(6893)

Issue 68120043: SSL Support for MySQL

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 2 months ago by hazmat
Modified:
10 years, 2 months ago
Reviewers:
mp+207904, Marco Ceppi
Visibility:
Public.

Description

SSL Support for MySQL Adds a few additional configuration options for SSL. 'ssl' is the primary one, it can be 'off', 'on', or 'only'. If its off, no changes. If its 'on' then ssl support is enabled and ssl_ca cert is sent to clients along the relation. If its only, then all clients must connect using ssl. The server cert/key and ca cert can be provided via config. If they are not, the service will act as its own ca, and also in only mode will also generate client certs and pass them along the relation (the client use of those certs is not required atm). https://code.launchpad.net/~hazmat/charms/precise/mysql/ssl-everywhere/+merge/207904 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+483 lines, -135 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M config.yaml View 1 chunk +18 lines, -0 lines 1 comment Download
M hooks/common.py View 5 chunks +49 lines, -11 lines 0 comments Download
M hooks/config-changed View 6 chunks +37 lines, -6 lines 0 comments Download
M hooks/db-relation-joined View 3 chunks +32 lines, -26 lines 1 comment Download
M hooks/shared_db_relations.py View 3 chunks +88 lines, -91 lines 0 comments Download
M hooks/slave-relation-changed View 1 chunk +3 lines, -0 lines 0 comments Download
A hooks/sslca.py View 1 chunk +253 lines, -0 lines 0 comments Download
M revision View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 2
hazmat
Please take a look.
10 years, 2 months ago (2014-02-24 11:48:16 UTC) #1
Marco Ceppi
10 years, 2 months ago (2014-02-27 14:22:55 UTC) #2
I have some concerns about the way this affects the mysql interface below.
Overall, this is a great change and will bring security to communication within
a deployed environment, I'm open to discussion about the points below - I simply
want to make sure the charm stays as compatible as possible as it grows.

https://codereview.appspot.com/68120043/diff/1/config.yaml
File config.yaml (right):

https://codereview.appspot.com/68120043/diff/1/config.yaml#newcode38
config.yaml:38: ssl_key:
I don't like that these are _'d while the majority of the configuration options
for MySQL are hypenated (with the exception of vip_iface, and vip_cidr which I
would have rejected had I maintained the charm at the time). If there isn't a
hard requirement for these to be _ then I'd prefer if they followed the
convention of the majority of the charm's config.

https://codereview.appspot.com/68120043/diff/1/hooks/db-relation-joined
File hooks/db-relation-joined (right):

https://codereview.appspot.com/68120043/diff/1/hooks/db-relation-joined#newco...
hooks/db-relation-joined:91: cmd.append("%s=%s" % (k, v))
With this, you're now overloading the mysql interface to the point where, if a
user deploys the MySQL charm, configures it for SSL, then joins to a charm that
doesn't know how to speak to MySQL on SSL with on and off, no change (I assume),
with ssl=only that charm just won't work.

Given the nature of this change, it seem more apt that this should be
implemented as a new interface, maybe mysql-ssl - so when you connect on that
interface and no ssl cert/ca/key are provided MySQL generates one, otherwise
uses the values provided by the configuration. In doing so services can expose
their compat with MySQL over SSL without having cases that charms will break if
MySQL is configured in a manner that's too restrictive.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b