Add decryption authorization for clevis clients #92
Conversation
Codecov Report
@@ Coverage Diff @@
## master #92 +/- ##
==========================================
- Coverage 57.35% 55.60% -1.75%
==========================================
Files 3 3
Lines 401 419 +18
Branches 114 120 +6
==========================================
+ Hits 230 233 +3
- Misses 88 100 +12
- Partials 83 86 +3
Continue to review full report at Codecov.
|
doc/tang.8.adoc
Outdated
|
|
||
| By default, tang will respond to any recovery requests. This can be | ||
| controlled on a per client key by adding a second argument to the *tang* | ||
| command line. This argument need to be a path to an existing directory. |
doc/tang.8.adoc
Outdated
|
|
||
| # touch /var/db/tang/authorized/EyIEfKd-_3UFMI5PSAp64UAAKeQ | ||
|
|
||
| 6. Decrypting this will now works: |
doc/tang.8.adoc
Outdated
| directory. | ||
|
|
||
| ifdef::freebsd[] | ||
| # TODO |
There was a problem hiding this comment.
I will need some help here, how to best enable this on FreeBSD.
There was a problem hiding this comment.
The FreeBSD equivalent is really not much different (see units/tangd.rc.in where the tangd config is built from):
Alter the command_args line in the tangd file in the rc.d directory to be something like:
command_args="${_tangd_listen_args} SYSTEM:"${tangd_executable} ${tangd_jwkdir} /var/db/tang/authorized 2>> ${tangd_logfile} " &"
and possibly add that directory to the required dirs line in the same file, i.e.,
required_dirs="${tangd_jwkdir} /var/db/tang/authorized"
The default behaviour of tangd is to reply correctly to all key recovery requests. In some deployments it may be needed to control when a key recovery should be allowed or not. This patch extends the tangd server with a very simple authorization method. When tangd is started with a second argument, this need to point at a directory to be used for authorizations. When a key recovery request occurs, the tangd server will check if this directory contains the filename of the client key fingerprint (thp/kid). If a file (which can be empty) exists with the name of a client fingerprint, the request is authorized and the decryption can be performed successfully. Signed-off-by: David Sommerseth <[email protected]>
To enable the client authorization directory before this change, the main systemd service unit file needed to be modified. This is less ideal and we can use EnvironmentFile= feature in the unit file to read a file with environmental variables the [email protected] will use for the command line. Signed-off-by: David Sommerseth <[email protected]>
dsommers
force-pushed
the
dev/authorization
branch
from
ca82648
to
907ad36
Compare
|
I've updated this WIP pull-req with the suggested changes. I've also added an additional commit which enables using a configuration file with the sytemd unit file, instead of modifying the unit file directly. |
dsommers
changed the title
|
ping @sarroutbi Any chance to get this pull request reviewed? |
|
Hello @dsommers. Thanks for your PR. I reviewed your change to help on fixing some typos and related stuff, but I don't have enough technical background regarding tang for this PR to be approved. I am not sure about this kind of authorization to be something that should be kept inside tang code, or if this should be kept out of tang by leveraging some of the other multiple solutions that are available, being tang an HTTP server. I will check with other maintainer and will come back to you. |
|
this is exactly what i want! this would allow us to build scripts around placing the file and making our own rules/apps to manage what can decrypt! |
|
@sergio-correia : what's your opinion on this? IMHO, this functionality could be kept out of tang, by selecting other mechanisms that do not affect Tang's code, such as nginx or similar. However, I understand that for some scenarios this kind of filtering could help to avoid an extra message hop. |
|
While the reverse proxy solution could work in many cases, it would for some use cases require something else than nginx; or something in between nginx and tang. For use cases where you want to allow a key recovery call for only a selected host within a limited time window, the pure nginx approach would mean to update the nginx config, add/remove the appropriate This would then speak for another reverse proxy in front of tang, which can inspect the REST URL before deciding to reject the request or pass it on to tang. While this would work, it would be a more complicated overall setup, with more parts in the chain having a possibility to fail. Having this simple check inside tang instead gives a simpler overall setup and can work just as efficient. The check itself is very simple (provided in the new The rest of the changes is to make this easily configurable via a "configuration file" which is parsed by systemd when starting tangd, introduction of this new configuration file (meson.build, systemd unit changes + a new file) and finally documentation. So this change set shouldn't be too intrusive or breaking things easily. |
|
Hello @dsommers : are you trying to resolve the conflicting files? |
|
@sarroutbi Sorry for the late response. Yes I will look at this. Pretty full plate before the holidays, so I'll probably follow up in august. |
|
Hello @dsommers . Any update on this? Are you planning to resolve conflicts? |
|
Ouch! Sorry about dropping the ball here. Looking into this now. |
|
Okay, so I've spent some time looking into this. A lot of stuff has happened in the master branch, so I would prefer to re-implement it based on master instead of just resolving the conflict. The needed changes would be better to be split up into more commits, to have a more gradual set of changes. This will take a bit more time than what I have available today. But this will get some attention a bit later. That said ... how likely is it that this feature will be accepted? If I am going to spend time getting this ready I need to know that this feature will be included in the end. It does not mean it must be implemented exactly like this pull-request suggests, we can work on adopting it to the projects interests and goals. But if there is a little chance this feature will be accepted, it's better to just close this pull-request and not waste any more of our time on it. |
We are asking conflict resolution because we would like to merge this feature. It seems useful to other users of the clevis suite, as it can be observed in the conversation.
We try to be cautious when merging to tang, as it is in, somehow, an stable state. For that reason, we like to take the patches and test them before accepting them. We also like unit tests to be added, documentation to be updated, and so on. Sorry if it takes time for the changes to be reviewed, but we prefer advancing in a slow but sure pace. |
|
@sarroutbi Thanks for the feedback. 👍 I'll put this into my todo list then, and get it rebased. I'll also look into extending the existing test suite to test this feature and all the related changes needed. |
The default behaviour of tangd is to reply correctly to all key recovery
requests. In some deployments it may be needed to control when a key
recovery should be allowed or not. This patch extends the tangd server
with a very simple authorization method.
When tangd is started with a second argument, this need to point at a
directory to be used for authorizations. When a key recovery request
occurs, the tangd server will check if this directory contains the
filename of the client key fingerprint (thp/kid). If a file (which
can be empty) exists with the name of a client fingerprint, the
request is authorized and the decryption can be performed successfully.
This touches one of the challenges mentioned in issue #20. It doesn't fully solve that issue, as it provides no tooling to alert for requests or to tell the client to "come back later". But it provides a first step to control when a decryption can be allowed.