During boot of an Ubuntu Core image we generate a device key on firstboot setup. This uses the rsa.GenerateKey() function from Go currently.
Tests have shown that this routine is significantly slower than using something like ssh-keygen or direct processing via libssl. This happens across all arches but since x86 is in general much faster here it has only been a real problem on ARM images where it is more noticeable (the Go routine is about 5.5 times slower than ssh-keygen in general, lots of test results are linked in the bug and PR below) and a key generation can take from 2-20 minutes.
While the initial PR was just shelling out to ssh-keygen, the security team had concerns about the nature of the setup:
“…ssh-keygen is an external program and has a configuration file that we’re not in control of.”
Due to this the code was instead ported to use libssl directly now via cgo. The question that came up during a meeting today was if it is really that bad to use the ssh-keygen setup here given that we can have full control over it by hard-coding commandline options to make it not use any potential external configuration file.
What is the concern with using libssl directly?
When the security team requested that libssl be used, we stated that we weren’t sure if ssh-keygen handles any corner cases or other unexpected complexity and left it up to Thomas to skim the ssh-keygen code in order to re-raise the issue with us so that we could all reconsider the use of shelling out to ssh-keygen. Is there a lot of complexity to carry over from ssh-keygen?
@tyhicks Security wise, it feels wiser to use a proven tool than to reimplement it, even more when that tool is supposed to be doing exactly what we’d have to do if we were reimplementing the same logic ourselves.
So the question feels reversed: what is the concern with not reimplementing ssh-keygen?
one of the possible issues is that ssh-keygen cant just be used to pipe the key content through via stdout but requires to write out a key file. so in case we need the key content in code we would need to read the key file from disk (not sure if this is a concern at all but definitely a difference towards using libssl to hold the key content in a variable)
Shelling out to another program that may have its own set of ssh-specific workarounds and behaviors for key generation is not the obvious choice from a security perspective.
If all of the magic is hidden behind the libssl API, then I think it is much more clean to shed all the extra ssh-keygen code around configuration handling, argument parsing, interactive I/O handling, etc., and simply use libssl directly.
I haven’t yet had a chance to review the PR. I’ll go do that now so that I can see if there is a lot of complexity involved. Until then, I don’t have enough detailed knowledge to claim which way is the best for us.
I still feel the opposite if there’s no further data backing it. We’re talking about a very well tested tool which is security sensitive and used by everybody, versus cooking our own usage of libssl to do the same job.
@niemeyer I’m not sure that there’s any hard data available to back either side of this discussion. It is mostly gut feeling and personal preference. @tvoss asked for the security team’s opinion on the PR and, after discussing it with two other team members, I recommended that libssl be used directly based on what we felt was the cleanest implementation that carried the least risk.
After an initial look at the PR and some reading of the ssh-keygen sources, I don’t see any reason to change my original recommendation. Outside of a little bit of setup, libssl does all of the heavy lifting. ssh-keygen contains quite a bit of code for functionality that snapd simply doesn’t need.
That said, it is your project and i’m only acting as an advisor here. ssh-keygen will likely be fine to use if special care is taken to ensure that external configuration is not consulted. I feel better about that now that I’ve looked at its source.
Thanks for the insights into the recommendation process, @tyhicks.
Based on those details, my own recommendation is that we move forward and integrate with ssh-keygen, for the following reasons:
- We don’t link with libssl at all in snapd today
- We’d need to link with libssl for the benefit of a very seldom used routine
- ssh-keygen is security sensitive and widely used
- ssh-keygen is already included in the core snap
- it’s easy to shell out for key creation
PR is updated to reflect the state of the discussion here: https://github.com/snapcore/snapd/pull/3130