#5502 closed defect (fixed)

Cassandra explicitly depends on openssl

Reported by: tomek Owned by: fdupont
Priority: medium Milestone: Kea1.4
Component: crypto 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

Description

The Cassandra code that was contributed github37 has explicit dependency on openssl. This is unacceptable, as Kea supports two crypto libraries: botan and openssl, with libcryptolink providing an abstraction layer.

The code in cql_host_data_source.cc should use libcryptolink rather than use openssl directly. Without this fix, Kea will faill to build with Cassandra and botan.

Note: address this ticket after github37 is on master.

Subtickets

Change History (13)

comment:1 Changed 18 months ago by fdupont

I agree but on some platforms MySQL depends on openssl too. So I propose to change this in the code should not introduce explicit and required dependency on openssl, and of course we have to cope with implicit dependency (as far as I know from a build option in the package).
BTW I am not sure it will fail to build but at least it won't be consistent so it must be avoided.

comment:2 Changed 18 months ago by razvan.becheriu

  • Owner set to razvan.becheriu
  • Status changed from new to accepted

comment:3 Changed 17 months ago by fdupont

I fixed the Makefile.am in #5487 to be able to build Kea with Cassandra on macOS where OpenSSL is brewed (vs native).
There are two ways to solve the use of MD5:

  • switch to cryptolink functions
  • switch to another faster hash as FNV (*)

IMHO the second is better but requires more work. If we agree about one of these solutions I'll steal the ticket and implement.

(*): I used crypto hash for a not crypto purpose so I learnt I was wrong, cf ISC DHCP ticket about hash in address allocation and bind9 work about FNV. To summary a crypto hash is convenient (available without effort), does the job but is clearly a performance pig.

comment:4 Changed 17 months ago by fdupont

After reading the code I am not convinced at all a hash is required: it seems the only thing required is an unique ID!
I had a similar problem in #5420 because Cassandra wants a primary (so unique) key per table. Fortunately Cassandra provides some specific types to do this, in #5420 I choose timeuuid which was the evident solution. In your case you can do the same or just use uuid type and function.

comment:5 Changed 17 months ago by razvan.becheriu

hi

I do not agree.
the md5sum is used for generating an unique id for a specific combination of the reservation data.
this combination should always generate the same id.
if you generate an uuid, that uuid is time dependent and is not related what so ever to the actual reservation data.
I suggest using the md5sum from cryptolib to make this library 'independent' code.

comment:6 Changed 17 months ago by fdupont

So you need a hash to get the same result from the same input. In this case I strongly suggest something like FNV which is far faster and has reasonable properties for a not crypto hash function.
cf https://en.wikipedia.org/wiki/Fowler–Noll–Vo_hash_function
I can add a FNV implementation in the util library if you'd like: just give this ticket to me with the interface (hash function type).

comment:7 Changed 17 months ago by fdupont

Put in fdfnv branch (*) the code (and unit tests) for FNV-1a 64 bit (likely the hash you want). BTW in hashIntoId() IMHO you should exchange paddings by separators (note I agree paddings or separators are needed, but in what I believe is the common case separators are more efficient and give a more readable code).

(*) I can push it on GitHub if you have no access to ISC repository.

comment:8 Changed 17 months ago by fdupont

I'll steal the ticket when #5487 will be merged.

comment:9 Changed 16 months ago by fdupont

  • Owner changed from razvan.becheriu to fdupont

#5487 merged. Stealing the ticket to change the hash from MD5.

comment:10 Changed 16 months ago by fdupont

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

Added FNV-1a 64 bit and use it to replace MD5 in the Cassandra CQL host data source.
Now the Cassandra CQL code has no more direct dependency on OpenSSL (I say direct because it can have on indirect one by supporting communications over SSL/TLS. Note for that case there is already a ticket for that feature for MySQL).
Ready for review.

comment:11 Changed 16 months ago by tomek

  • Owner changed from UnAssigned to tomek

comment:12 Changed 16 months ago by tomek

  • Owner changed from tomek to fdupont

That's a nice piece of code. Please merge.

You need a changelog for this. If you don't have any better proposal, here's mine:

13xx.	[func]		fdupont
	Implemented FNV hashing function. Cassandra backend no longer
	explicitly depends on OpenSSL.
	(Trac #5502, git tbd)

comment:13 Changed 16 months ago by fdupont

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged. Closing. Note as the hash is (IMHO incorrectly) used as signed I am opening a followup ticket.

Note: See TracTickets for help on using tickets.