Ubuntu Core key generation slowness

ogra 2017-04-10 14:41:10 UTC #1

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.

Proposed fix:

tyhicks 2017-04-10 14:49:08 UTC #2

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?

niemeyer 2017-04-10 14:56:05 UTC #3

@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?

ogra 2017-04-10 14:59:19 UTC #4

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)

tyhicks 2017-04-10 15:19:55 UTC #5

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.

niemeyer 2017-04-10 15:42:45 UTC #6

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.

tyhicks 2017-04-10 16:19:32 UTC #7

@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.

niemeyer 2017-04-10 16:41:07 UTC #8

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

tvoss 2017-04-11 06:46:55 UTC #9

PR is updated to reflect the state of the discussion here: https://github.com/snapcore/snapd/pull/3130