Mailing List Archive

Just want to follow up on this again and see if anybody has any
opinions. I have included a small patch to fix it:


-- >8 --
Subject: [PATCH] gpg: Don't use EdDSA algo ID for ECDSA curves.

* g10/keygen.c (ask_curve): Change algo ID to ECDSA if it changed from
an EdDSA curve.
---

Fixes a bug that causes a slot to get stuck reporting itself as EdDSA
when the curve has changed to ECDSA. If a smartcard did not override
the algo that gpg sent it, gpg would get confused about an ECDSA curve
with an EdDSA algo ID, and would generate invalid public keys.
---
g10/keygen.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/g10/keygen.c b/g10/keygen.c
index 64fefd231..943b40110 100644
--- a/g10/keygen.c
+++ b/g10/keygen.c
@@ -2507,14 +2507,25 @@ ask_curve (int *algo, int *subkey_algo, const
char *current)
else
{
/* If the user selected a signing algorithm and Curve25519
- we need to set the algo to EdDSA and update the curve name. */
- if ((*algo == PUBKEY_ALGO_ECDSA || *algo == PUBKEY_ALGO_EDDSA)
- && curves[idx].eddsa_curve)
+ we need to set the algo to EdDSA and update the curve name.
+ If switching away from EdDSA, we need to set the algo back
+ to ECDSA. */
+ if (*algo == PUBKEY_ALGO_ECDSA || *algo == PUBKEY_ALGO_EDDSA)
{
- if (subkey_algo && *subkey_algo == PUBKEY_ALGO_ECDSA)
- *subkey_algo = PUBKEY_ALGO_EDDSA;
- *algo = PUBKEY_ALGO_EDDSA;
- result = curves[idx].eddsa_curve;
+ if (curves[idx].eddsa_curve)
+ {
+ if (subkey_algo && *subkey_algo == PUBKEY_ALGO_ECDSA)
+ *subkey_algo = PUBKEY_ALGO_EDDSA;
+ *algo = PUBKEY_ALGO_EDDSA;
+ result = curves[idx].eddsa_curve;
+ }
+ else
+ {
+ if (subkey_algo && *subkey_algo == PUBKEY_ALGO_EDDSA)
+ *subkey_algo = PUBKEY_ALGO_ECDSA;
+ *algo = PUBKEY_ALGO_ECDSA;
+ result = curves[idx].name;
+ }
}
else
result = curves[idx].name;
--
2.20.1

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

Trevor Bentley wrote:
> Just want to follow up on this again and see if anybody has any
> opinions. I have included a small patch to fix it:

I see the problem. I think that it is the call of ask_curve from
ask_card_keyattr in card-util.c. Isn't it?

(Looking your patch with the context of keygen.c, I didn't understand
why ask_curve matters.)

I'll apply your patch.
--

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

> I see the problem. I think that it is the call of ask_curve from
> ask_card_keyattr in card-util.c. Isn't it?
>
> (Looking your patch with the context of keygen.c, I didn't understand
> why ask_curve matters.)

Yes, exactly, it's the call in ask_card_keyattr() that is a problem. It
gets 'stuck' on the EdDSA algo ID when it passes in the current curve
parameters.

Thanks,

Trevor

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

Trevor Bentley <trevor@yubico.com> wrote:
> Yes, exactly, it's the call in ask_card_keyattr() that is a problem. It
> gets 'stuck' on the EdDSA algo ID when it passes in the current curve
> parameters.

Thanks, applied (STABLE-BRANCH-2-2 and master).
--

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

> Thanks, applied (STABLE-BRANCH-2-2 and master).

Thanks! What is the master->stable branch process like? Should git
commit af3efd149f555d36a455cb2ea311ff81caf5124c also have gone into a
stable branch, or will it be moved over eventually? That commit is
arguably more important, as their is no workaround.

-Trevor

_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel

On Wed, 27 Mar 2019 10:29, gnupg-devel@gnupg.org said:

> Thanks! What is the master->stable branch process like? Should git

We cherry-pick bug fixes and some other useful commits from master. Bug
fixes are usually psuhed to 2.2 soon after the master. Others backports
are selected before we do a new 2.2 release.

> commit af3efd149f555d36a455cb2ea311ff81caf5124c also have gone into a

Unfortunately this one was forgotten. I just pushed it but it has to
wait for the 2.2.16 release.


Salam-Shalom,

Werner

--
Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.