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

KSM database backend doesn't respect --key-handles #13

Open
michael-lazar opened this issue Mar 20, 2017 · 2 comments
Open

KSM database backend doesn't respect --key-handles #13

michael-lazar opened this issue Mar 20, 2017 · 2 comments

Comments

@michael-lazar
Copy link

Scenario

I'm setting up a distributed system that runs multiple KSMs using the python KSM included in this package (yubikey_ksm.py). I will be connecting each KSM to a dedicated yubiHSM. I'm using a centralized MySQL database to store the AEAD keys for all of the servers. I would like each of my KSMs to use a unique key handle for decryption. In my example I have 5 servers, so server 1 decrypts using key handle 1, server 2 decrypts using key handle 2, etc. The AEAD keys are encrypted with all 5 of the key handles and stored together in a single table on the database.

Problem

When I start a server with the --key-handle 3 argument, I expect that it will load the AEAD encrypted with key handle 3 from the database. This behavior is correct when the AEAD keys are stored using the file system backend. However, with the database backend, it doesn't respect the --key-handle argument and instead returns an AEAD with a random key handle.

Here's the offending logic:

https://github.com/Yubico/python-pyhsm/blob/master/pyhsm/ksm/yubikey_ksm.py#L220

s = sqlalchemy.select([self.aead_table]).where(self.aead_table.c.public_id == public_id)
result = connection.execute(s)

for row in result:
    kh_int = row['keyhandle']
    aead = pyhsm.aead_cmd.YHSM_GeneratedAEAD(None, kh_int, '')
    aead.data = row['aead']
    aead.nonce = row['nonce']
return aead

You can see that it's iterating over all of the AEADs that match the yubikey and returning the last one outside of the loop. The row that ends up being returned is random. If it doesn't match a key handle that is shared with the HSM, the decryption will fail.

Solution

I was able to fix this by updating the SQL query to filter on only the key handles that were specified via --key-handles.

s = sqlalchemy.select([self.aead_table]).where(
    (self.aead_table.c.public_id == public_id)
    & self.aead_table.c.keyhandle.in_([kh[1] for kh in self.key_handles]))
result = connection.execute(s)

for row in result:
    kh_int = row['keyhandle']
    aead = pyhsm.aead_cmd.YHSM_GeneratedAEAD(None, kh_int, '')
    aead.data = row['aead']
    aead.nonce = row['nonce']
return aead

I have a fork with my changes here https://github.com/michael-lazar/python-pyhsm/blob/ksm_db_backend/pyhsm/ksm/yubikey_ksm.py#L208. I would be happy to submit a pull request if you're interested in getting this upstream

@klali
Copy link
Member

klali commented Mar 21, 2017

I think that makes sense and should be fixed.
Does the query as constructed by sql alchemy hit the index for public_id + keyhandle?
Please submit it as a pull request and I'll get it merged.

@michael-lazar
Copy link
Author

Thank you for the quick response! Yes it does appear to hit the primary key index.

MariaDB [ykksm]> EXPLAIN SELECT aead_table.public_id, aead_table.keyhandle, aead_table.nonce, aead_table.aead FROM aead_table WHERE aead_table.public_id = 'djccccccccbb' AND aead_table.keyhandle IN (1, 2);
+------+-------------+------------+-------+---------------+---------+---------+------+------+-------------+
| id   | select_type | table      | type  | possible_keys | key     | key_len | ref  | rows | Extra       |
+------+-------------+------------+-------+---------------+---------+---------+------+------+-------------+
|    1 | SIMPLE      | aead_table | range | PRIMARY       | PRIMARY | 22      | NULL |    2 | Using where |
+------+-------------+------------+-------+---------------+---------+---------+------+------+-------------+
1 row in set (0.00 sec)

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

No branches or pull requests

2 participants