upgrade_by_passwd.py can brick the key on single invocation

[nakato]

A single invocation of "upgrade_by_passwd.py" with the wrong admin key will brick the key in a single run if factory_reset=no

I would not expect the tool to try a single pin 3 times in a row without prompting, it would probably be best to make this less aggressive.

[alex-nitrokey]

@szszszsz I think, a fix would be to delete this loop. In my opinion a user, not the script should restart if something failed.

But this may should be fixed upstream, don't you think? Shall I ask NIIBE on the gnuk-mailingslist?

[szszszsz]

@nakato Agreed.

@alex-nitrokey That is a good candidate for the cause indeed. The loop should only continue in a case, where the connection issues had occurred, not when the PIN had been incorrect. It would be the safest to remove it for now.
Could you prepare a patch for NIIBE and send him via the mailing list? It would be faster to explain the problem, when the fix is presented as well, and perhaps it will be merged directly. We should apply the patch in our repository anyway, until the handling is improved.

[alex-nitrokey]

I created a PR in #13

I added a check if auth attempt actually worked. The loop breaks if the authentication failed. This helps to keep the feature of killing scdaemon automatically intact.

[jans23]

Did you check if latest Gnuk contains a fix for that issue already, perhaps?

[alex-nitrokey]

Did you check if latest Gnuk contains a fix for that issue already, perhaps?

Well... actually I should have, but I didn't.

[alex-nitrokey]

Doesn't look like it.