Horizon throws an invalid error when downloading a keypair that wasn't saved

[Matthias Runge]

Steps to Reproduce:
1. go to Access & Security page
2. under the Keypairs section, hit on "Create a keypair" and give it a name and click on Create Keypair
3. hit cancel on the "Save As.." window, to not save the *.pem file
4. click on the "Download keypair xxx" link

Actual results:

Error: Unable to create keypair: Key pair 'abc' already exists. (HTTP 409) (Request-ID: req-26876bc9-dcb6-4d5c-ad30-9995c2e2da11)

[Zhenguo Niu (niu-zglinux)]

The download link will generate keypar again if download window is closed.
here we can remove the download page, then set create success_url to generate keypar.

[David Lapsley (dlapsley)]

The error I see is slightly different. There is no "save as" dialog opened. The download page appears directly. With the following message:

"The keypair "test-keypaird3" should download automatically. If not use the link below."

If I click on the download link, I get the following:

Error: Unable to create keypair: Key pair 'test-keypair2' already exists. (HTTP 409) (Request-ID: req-77b6e6cc-6b4d-41ea-a4b9-94b563fc9b64)

It's a minor case, but would be cleaner to check before regenerating the key when the Download link is clicked...

[David Lapsley (dlapsley)]

After hacking around with the code bit more, the use case is clearer now. This seems more of a workflow question than implementation. If the user clicks on Download, then presumably, the original key was not downloaded. In this case, here are the different cases that come to mind:

1. Key was generated (but user indicates not downloaded by clicking on download link):

In this case we could either:
(a) generate the error above and let the user manually delete the key and regenerate.
(b) delete the key and automatically regenerate a new key and download to user.

2. Key was not generated:
In this case we can proceed as above and automatically regenerate a new key and download to user.

Does this make sense? Do we need to do anything here? I have code that will implement the cases above if that would be useful...

[Matthias Runge (mrunge)]

Yes, it makes sense to me, David. Would you mind to send your patch for review?

[Victoria Martinez de la Cruz (vkmc)]

I took a look at the code and could notice that this bug is caused because different use cases are considered to be the same.

The download link shouldn't redirect to the generate view. This view is accessed when the user creates a new key pair AND when the user selects that link.

A possible fix could be,

If the user creates a new key pair, then it should be checked that there is no other key with the same name. By default, Nova throws the corresponding exception. If there is no other key with the same name, the new key is generated correctly and the download starts.

If the user selects the download link, then it should be checked that the key is in the key pair list. If it's in the key pair list, then we should retrieve that key pair. If it's not there, then there is no way the user could reach that link, so I would show an error (e.g. 'The key pair 'foo' ... doesn't exist').

This can be done adding a different view for the download link.

[Kirill Izotov (enykeev)]

Correct me if i'm wrong, but it seems like you can not retrieve keypair's private key after it was generated (due to security reasons), so you have to serve it right after you have generated it. So, yes, the only option here is to either regenerate it automatically or notify the user that he need to do it manually. The second option seems preferable since it protects user from regenerating its keypair accidentally.

[Victoria Martinez de la Cruz (vkmc)]

Well, you cannot retrieve a keypair directly... but you can retrieve the list of keypairs and get the one you are looking for using its name. So, whereas its not the most efficient option, you could perform an exhaustive search for the key iterating over the keypairs list.

I'm not sure if regenerating a key pair is possible, so the second option seems more feasible to me.

Thanks for looking into it!

[Kirill Izotov (enykeev)]

It's a bit complicated than that. You can retrieve keypair by name if you add API for that (novaclient(request).keypairs.get is already here), but neither 'list' nor 'get' will return you the PRIVATE key (and this is what you need to return to user when he tries to redownload keypair).

Actually, the whole concept of generating PKI keypair on a server is very very wrong because then you have to download your private key through the open channels and it would compromise your security. You can say that you should use HTTPS for that, but a) horizon won't enforce you to do that and b) this is really not how security supposed to work. All this is actually not a horizon's fault and it just implements the API that was here long ago. And yes, it is convenient, but still wrong.

I had dig a bit deeper and found that you actually don't need :download view at all. If user can't redownload the key, than we shouldn't show him the link in the first place, then we have nothing to show at that page and should probably remove it completely and point user straight to :generate. And it would work flawlessly right until we would notice that our key wouldn't show up in key pair list until we reload the page. So, now we need the way to both reload the page and download the key. You can do that by intercepting form's onsubmit event and opening the popup window for download while at the same time reloading the page. But now you don't know where to point your window.open since {% url 'horizon:project:access_and_security:keypairs:generate' keypair_name %} populates itself before the user picked a name for keypair... It's like a Carol's Rabbit hole, gets bigger every step you make.

I'll take over the bug when i'll be sure i can handle it, but please stop me if i'm going the wrong way.

[Victoria Martinez de la Cruz (vkmc)]

Huh, you are right... I mistakenly though you could get the private key from nova.keypair_list. Fortunately that's not the case - that would be a huge security issue!

Maybe removing and generating a key with the same name? The key wouldn't be the same, but at that moment I assume that the key haven't been used anywhere yet.

It's not fancy at all... I know.

Let me know if you hit a more decent solution!

[Kirill Izotov (enykeev)]

Victoria, the problem with automatic regeneration is that we would end up with the link like http://localhost:8000/project/access_and_security/keypairs/111/generate/ that will regenerate keypair when user will hit it again. It will be fine if he do it right after the first attempt, but what happened if he hits it some times later (either through back button or or by someone's malicious intent)?

As of my approach, it seems like i have hit the race condition and i see no way to download the file and then reload the page after new key was generated.

The only way i see to do it right is by using RTC, but i don't see any progress on the related blueprint so we have to stop somewhere in the middle. For example, we could leave :download page be, but inform the user that to regenerate the key he needs to delete it first and replace "Download keypair" link with "Back to keypairs" which should point to :index.

[Victoria Martinez de la Cruz (vkmc)]

Yeah I thought so...

So bad to hear that your approach didn't work.

IMO this is an serious bug, so probably the best we could do right now is to inform the user as you suggest and wait for the RTC blueprint for a more elegant solution.

[Eric Peterson (ericpeterson-l)]

I think the automatic download that starts is really the problem.... if we just presented the link on the resulting page that would be the best imo.

I could see many first time users seeing the download window popup and being confused as to what is going on, then if they click cancel the private key I believe is lost forever. Then they need to start the process over again.

A better alternative (imo) is that we place the current page with the link in it, and also provide a warning or more text that users really need to take great care with the private key. (like don't loose it, keep it private, etc).

[Julie Pichon (jpichon)]

Unassigning due to inactivity.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/112513

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/112513
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=5b9ea17e515ec8a20d23f13787563d3e3a9f4d7c
Submitter: Jenkins
Branch: master

commit 5b9ea17e515ec8a20d23f13787563d3e3a9f4d7c
Author: nikunj2512 <email address hidden>
Date: Thu Aug 7 13:02:38 2014 +0530

    Fixes downloading a keypair

    This patch fixes the invalid error which user gets when downloading
    a keypair that wasn't saved from the keypair download link from the
    keypair download page.

    When user clicks the download link, i just sending that to a different
    url and then deleting the keypair and then again creating it with
    same name.

    Change-Id: I4f9166f112487a500d23f9cc98b98660be2ebe55
    Closes-bug: #1182797