Opened 17 months ago

Last modified 7 days ago

#5117 reviewing enhancement

MySQL database backends with SSL

Reported by: fdupont Owned by: fdupont
Priority: medium Milestone: Kea1.5
Component: database-mysql Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Subtickets

Change History (16)

comment:1 Changed 17 months ago by tomek

  • Milestone changed from Kea-proposed to Kea1.3

As discussed on 2017-01-26 call, moving to 1.3.

comment:2 Changed 13 months ago by tomek

  • Milestone changed from Kea1.3 to Kea 1.4

Database improvements are focus of upcoming 1.4. 1.3 is about shared subnets, moving to appropriate milestone.

comment:3 Changed 9 months ago by tomek

  • Milestone changed from Kea 1.4 to Kea1.4

Milestone renamed

comment:4 Changed 6 months ago by fdupont

Note on some platforms the mysql library requires to link with OpenSSL (*) so the inconvenience is already here...

(*) ./configure LIBS=-L/usr/local/opt/openssl/lib ... on my macOS.

comment:5 Changed 3 months ago by tomek

  • Component changed from database-all to database-mysql

comment:6 Changed 7 weeks ago by tomek

  • Milestone changed from Kea1.4 to Kea1.4-final

As discussed on 2018-04-26 call, moving low and some med priority tickets to 1.4-final.

comment:7 Changed 4 weeks ago by fdupont

I divide this in two parts:

-1- fix configure.ac to add OpenSSL library directory sooner in LIBS

-2- add new keywords to database setup (and perhaps PostgreSql? and Cassandra too?)

I create a ticket for the first point as my macOS requires it (my full setup is:

setenv CXX "g++ --std=c++11"
autoreconf -i |& tee loga
./configure LIBS=-L/usr/local/opt/openssl/lib --enable-generate-docs --enable-logger-checks --enable-shell --with-log4cplus=/usr/local --with-gtest-source=/usr/local/src/googletest --with-werror --with-openssl=/usr/local/opt/openssl --with-mysql=/usr/local/bin/mysql_config --with-pgsql=/usr/local/bin/pg_config --with-cql=/tmp/kea/tools/cql_config --with-tier1 --with-tier2 |& tee logc

comment:8 Changed 4 weeks ago by fdupont

Depends on #5631

comment:9 Changed 2 weeks ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:10 Changed 2 weeks ago by fdupont

MySQL and PostgreSQL have a similar parameter named ssl-mode (MySQL) and sslmode (PostgreSQL, BTW the only ssl parameter for this database). Should I configure to accept both spelling?

comment:11 Changed 2 weeks ago by fdupont

After looking different SSL settings I decided to use per backend keywords. Note there is a pending ticket to specialize more parsing to backends, including dynamically loaded backends...

comment:12 Changed 2 weeks ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

Some comments:

  • adding tests using SSL/TLS makes things very complex for a low benefit. Even scripts will need to be forked and updated...
  • for PostgreSQL and Cassandra it is easy to add testswhere SSL/TLS is disabled (note it is not the default in particular for PostgreSQL so with bad server setup you can get the overhead without the security...) so I did that.
  • PR 67 includes support of MySQL protocol parameter. In fact it is mainly useful on Windows and for Unixes the choice is between Unix sockets (host=localhost, the default) and TCP (not local server or host=127.0.0.1). As it can be controlled from the configuration without changing the code IMHO we should not integrate this part of the PR.
  • in theory the same work should be done for the forensic logger hook library...

Ready for review.

comment:13 Changed 7 days ago by tmark

  • Owner changed from UnAssigned to tmark

comment:14 follow-up: Changed 7 days ago by tmark

  • Owner changed from tmark to fdupont

I fixed a missing blank in cql code and some minor edits in the admin guide,
so please pull.
Builds and unit tests pass under MacOS (Sierra).

cql_connection.cc


readFileContent() - this method silently fails if it cannot open the file
While the calling code will emit an error log because the value is empty,
there is no direct indication why the value is empty.

I understand that unit tests are problematic. Have you done manual,
tests? We cannot very well release it, unless we have at least done that.

comment:15 in reply to: ↑ 14 Changed 7 days ago by fdupont

Replying to tmark:

I understand that unit tests are problematic. Have you done manual,
tests? We cannot very well release it, unless we have at least done that.

=> no and if it is a concern we can simply move this ticket to 1.5...

comment:16 Changed 7 days ago by tomek

  • Milestone changed from Kea1.4-final to Kea1.5
  • Priority changed from low to medium

Given we have a code freeze today, we won't be able to test is throughly. Let's be on the safe side and move this to 1.5 and bump its priority to medium.

Note: See TracTickets for help on using tickets.