openssh-portable icon indicating copy to clipboard operation
openssh-portable copied to clipboard

Pkcs11 Edwards support

Open beldmit opened this issue 4 years ago • 27 comments

This patch provides basic support of Ed25519 keys via PKCS#11 tokens

beldmit avatar Feb 25 '21 13:02 beldmit

Force-pushed changes related to build without openssl and related to the comments by @Jakuje

Don't see the MasOS output so can't do anything with it :(

beldmit avatar Feb 25 '21 17:02 beldmit

I reached out to Jakub about having trouble converting OPENSSL Ed25519 key to OPENSSH format when he pointed me out to the commit "Allow import Edwards keys from OpenSSL". I tried those changes by cloning the pkcs11_edwards branch and the conversion worked for me.

$ openssl genpkey -algorithm Ed25519 -out ed25519key.pem $ ssh-keygen -y -f ed25519key.pem Load key "ed25519key.pem": invalid format $

NOW USING branch: pkcs11_edwards $ ../openssh-portable/ssh-keygen -y -f ed25519key.pem ssh-ed25519 AAAA..............(ommitted)........................................ZB $

Thanks for the help Jakub.

rahsinha2 avatar Mar 31 '21 20:03 rahsinha2

Hi Dmitry, Do you know where is this pkcs11_edwards branch now ?

Thanks, Nitin

nisamuel avatar May 24 '21 10:05 nisamuel

Hi Nitin

https://github.com/beldmit/openssh-portable/tree/pkcs11_edwards

beldmit avatar May 24 '21 10:05 beldmit

Hi Dmitry, I get this error when a client tries to connect to sshd (OpenSSH_8.0p1) using ed25519 hostkeys. Any clues ?

debug2: ssh_ed25519_verify: crypto_sign_ed25519_open failed: -1 ssh_dispatch_run_fatal: Connection to 10.196.156.163 port 22022: incorrect signature openssh-8.5p1:=> ./ssh -V OpenSSH_8.5p1, OpenSSL 1.0.2k-fips 26 Jan 2017

Thanks, Nitin

nisamuel avatar May 24 '21 11:05 nisamuel

No clues. Which token do you use?

beldmit avatar May 24 '21 11:05 beldmit

this PR exposes ssh-add's 1024-byte signature test's incompatibility with OpenSC's 512-byte internal limit, since the data to be signed is passed to OpenSC undigested (consequently, ssh-add -T fails). it would be nice if both sides could agree on a limit, so that ssh-add -T works w/ opensc-pkcs11 eddsa keys.

martelletto avatar May 24 '21 12:05 martelletto

this PR exposes ssh-add's 1024-byte signature test's incompatibility with OpenSC's 512-byte internal limit, since the data to be signed is passed to OpenSC undigested (consequently, ssh-add -T fails). it would be nice if both sides could agree on a limit, so that ssh-add -T works w/ opensc-pkcs11 eddsa keys.

Hmm. Did not know about this feature of ssh-add. This is indeed useful. For normal keys, it is not an issue as they go through hashing. We have the following PR on OpenSC side to allow large inputs so we should make sure it will get into the next release: https://github.com/OpenSC/OpenSC/pull/2314

Jakuje avatar May 24 '21 16:05 Jakuje

Hi Dmitry, Here is the story... I have a PEM key for Ed25519 from which I wanted to extract the pub key. I took your patch "Allow import Edwards keys from OpenSSL" alone (1 from the 8 commits) to get ssh-keygen -f <pem> -y working. Thanks for that !!

  The pair now looks like --

firepower-2110:~# cat /mnt/disk0/.private/lina_sshd/ctxt0/ssh_host_eddsa_key.pub ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIO6kAVZLmePdXfsca2MGuGSjrzvT7QvVZ3cbaFZy3EXx

firepower-2110:~# cat /mnt/disk0/.private/lina_sshd/ctxt0/ssh_host_eddsa_key // input PEM -----BEGIN PRIVATE KEY----- MC4CAQAwBQYDK2VwBCIEIG2Z9l0q+9au2Mun6dA2dwPXVs8mDcYAQSPuo+/1IXae -----END PRIVATE KEY-----

  The above pvtkey is loaded into  OpenSSH_8.0p1  sshd which it used to sign the kex HASH and sent in kex reply.
  The problem now is that a remote client trying to connect to this sshd fails to verify the signature  using the pubkey, as in --

CLIENT: debug2: ssh_ed25519_verify: crypto_sign_ed25519_open failed: -1 ssh_dispatch_run_fatal: Connection to x.y.z.z port 22022: incorrect signature

A private ed25519 key on a normal linux host looks larger than the one generated from PEM above--

linux:~# cat /etc/ssh/ssh_host_ed25519_key -----BEGIN OPENSSH PRIVATE KEY----- b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW QyNTUxOQAAACBhArefq3UNoXUQzWm84lNXZn0khT4OfVyfqvJSzLMwmwAAAJiIKZ2BiCmd gQAAAAtzc2gtZWQyNTUxOQAAACBhArefq3UNoXUQzWm84lNXZn0khT4OfVyfqvJSzLMwmw AAAECgfh9gYXZZFTD99yiE6RssIfgALC5/i6NX61a1rJgK+2ECt5+rdQ2hdRDNabziU1dm fSSFPg59XJ+q8lLMszCbAAAAEXJvb3RAaW50ZWwteDg2LTY0AQIDBA== -----END OPENSSH PRIVATE KEY-----

Is there something amiss in the pair that ssh-keygen generated from the PEM I provided ?

Thanks, Nitin

nisamuel avatar May 26 '21 02:05 nisamuel

Hello Nitin

First of all I hope you didn't publish your real keys, just experimental ones.

I'll try to check but not immediately

beldmit avatar May 26 '21 05:05 beldmit

Note the headers:

-----BEGIN OPENSSH PRIVATE KEY-----

and

-----BEGIN PRIVATE KEY-----

The first one is OpenSSH format and the other is PEM.

The PEM file looks fine:

[jjelen@t490s Downloads]$ openssl pkey -in - -text
-----BEGIN PRIVATE KEY-----
MC4CAQAwBQYDK2VwBCIEIG2Z9l0q+9au2Mun6dA2dwPXVs8mDcYAQSPuo+/1IXae
-----END PRIVATE KEY-----
-----BEGIN PRIVATE KEY-----
MC4CAQAwBQYDK2VwBCIEIG2Z9l0q+9au2Mun6dA2dwPXVs8mDcYAQSPuo+/1IXae
-----END PRIVATE KEY-----
ED25519 Private-Key:
priv:
    6d:99:f6:5d:2a:fb:d6:ae:d8:cb:a7:e9:d0:36:77:
    03:d7:56:cf:26:0d:c6:00:41:23:ee:a3:ef:f5:21:
    76:9e
pub:
    ee:a4:01:56:4b:99:e3:dd:5d:fb:1c:6b:63:06:b8:
    64:a3:af:3b:d3:ed:0b:d5:67:77:1b:68:56:72:dc:
    45:f1

But for some reason, the values in the OpenSSH format look different

$ base64 -d | hexdump -C
b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW
QyNTUxOQAAACBhArefq3UNoXUQzWm84lNXZn0khT4OfVyfqvJSzLMwmwAAAJiIKZ2BiCmd
gQAAAAtzc2gtZWQyNTUxOQAAACBhArefq3UNoXUQzWm84lNXZn0khT4OfVyfqvJSzLMwmw
AAAECgfh9gYXZZFTD99yiE6RssIfgALC5/i6NX61a1rJgK+2ECt5+rdQ2hdRDNabziU1dm
fSSFPg59XJ+q8lLMszCbAAAAEXJvb3RAaW50ZWwteDg2LTY0AQIDBA==
00000000  6f 70 65 6e 73 73 68 2d  6b 65 79 2d 76 31 00 00  |openssh-key-v1..|
00000010  00 00 04 6e 6f 6e 65 00  00 00 04 6e 6f 6e 65 00  |...none....none.|
00000020  00 00 00 00 00 00 01 00  00 00 33 00 00 00 0b 73  |..........3....s|
00000030  73 68 2d 65 64 32 35 35  31 39 00 00 00 20 61 02  |sh-ed25519... a.|
00000040  b7 9f ab 75 0d a1 75 10  cd 69 bc e2 53 57 66 7d  |...u..u..i..SWf}|
00000050  24 85 3e 0e 7d 5c 9f aa  f2 52 cc b3 30 9b 00 00  |$.>.}\...R..0...|
00000060  00 98 88 29 9d 81 88 29  9d 81 00 00 00 0b 73 73  |...)...)......ss|
00000070  68 2d 65 64 32 35 35 31  39 00 00 00 20 61 02 b7  |h-ed25519... a..|
00000080  9f ab 75 0d a1 75 10 cd  69 bc e2 53 57 66 7d 24  |..u..u..i..SWf}$|
00000090  85 3e 0e 7d 5c 9f aa f2  52 cc b3 30 9b 00 00 00  |.>.}\...R..0....|
000000a0  40 a0 7e 1f 60 61 76 59  15 30 fd f7 28 84 e9 1b  |@.~.`avY.0..(...|
000000b0  2c 21 f8 00 2c 2e 7f 8b  a3 57 eb 56 b5 ac 98 0a  |,!..,....W.V....|
000000c0  fb 61 02 b7 9f ab 75 0d  a1 75 10 cd 69 bc e2 53  |.a....u..u..i..S|
000000d0  57 66 7d 24 85 3e 0e 7d  5c 9f aa f2 52 cc b3 30  |Wf}$.>.}\...R..0|
000000e0  9b 00 00 00 11 72 6f 6f  74 40 69 6e 74 65 6c 2d  |.....root@intel-|
000000f0  78 38 36 2d 36 34 01 02  03 04                    |x86-64....|

I was looking into the following part, which should be a public key

61 02 b7 9f ab 75 0d a1 75 10  cd 69 bc e2 53 57 66 7d 24 85 3e 0e 7d 5c 9f aa  f2 52 cc b3 30 9b

so indeed if the PEM key was generated from this OpenSSH keye, there is probably something wrong as the values do not match

Jakuje avatar May 26 '21 08:05 Jakuje

I will check as the self-tests passed with my patch.

beldmit avatar May 26 '21 08:05 beldmit

openssl pkey -in test_ed25519.pem -text gives me the output

MC4CAQAwBQYDK2VwBCIEIG2Z9l0q+9au2Mun6dA2dwPXVs8mDcYAQSPuo+/1IXae
-----END PRIVATE KEY-----
ED25519 Private-Key:
priv:
    6d:99:f6:5d:2a:fb:d6:ae:d8:cb:a7:e9:d0:36:77:
    03:d7:56:cf:26:0d:c6:00:41:23:ee:a3:ef:f5:21:
    76:9e
pub:
    ee:a4:01:56:4b:99:e3:dd:5d:fb:1c:6b:63:06:b8:
    64:a3:af:3b:d3:ed:0b:d5:67:77:1b:68:56:72:dc:
    45:f1

After extracting the pubkey with locally built openssh via ./ssh-keygen -y -f test_ed25519.pem I get

ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIO6kAVZLmePdXfsca2MGuGSjrzvT7QvVZ3cbaFZy3EXx

Decoding the value, I get

$ base64 -d|hexdump -C
AAAAC3NzaC1lZDI1NTE5AAAAIO6kAVZLmePdXfsca2MGuGSjrzvT7QvVZ3cbaFZy3EXx

00000000  00 00 00 0b 73 73 68 2d  65 64 32 35 35 31 39 00  |....ssh-ed25519.|
00000010  00 00 20 ee a4 01 56 4b  99 e3 dd 5d fb 1c 6b 63  |.. ...VK...]..kc|
00000020  06 b8 64 a3 af 3b d3 ed  0b d5 67 77 1b 68 56 72  |..d..;....gw.hVr|
00000030  dc 45 f1                                          |.E.|

that are exactly the same bytes.

beldmit avatar May 28 '21 15:05 beldmit

@martelletto would you mind to test the attached patch with opensc? opensc.txt

beldmit avatar May 29 '21 12:05 beldmit

@beldmit The diff works and looks good at a glance. I will give it a closer look early next week.

martelletto avatar May 29 '21 14:05 martelletto

Great!

beldmit avatar May 29 '21 14:05 beldmit

@beldmit Just like we call d2i_ASN1_OCTET_STRING() on CKA_EC_POINT in pkcs11_fetch_eddsa_pubkey(), we need to call i2d_ASN1_OCTET_STRING() on CKA_EC_POINT when looking up the keys in eddsa_do_sign_pkcs11(). I had this modification on my local tree when I applied your patch, which is why it worked for me. Two questions (or more):

  1. Which PKCS#11 provider did you use to test the functionality, where CKA_EC_POINT is not an ASN1-encoded octet string? Shouldn't we make sure this PKCS#11 provider and OpenSC align their interfaces first, to avoid implementing compatibility quirks in ssh-pkcs11.c? This inconsistency on the format of CKA_EC_POINT will make eddsa_do_sign_pkcs11() particularly (and unnecessarily) complex to implement;
  2. Is there a reason eddsa_do_sign_pkcs11() populates key_filter inside the TAILQ_FOREACH() loop, or could we move the code to the function's prelude and perform those steps once?

martelletto avatar Jun 01 '21 09:06 martelletto

@beldmit Just like we call d2i_ASN1_OCTET_STRING() on CKA_EC_POINT in pkcs11_fetch_eddsa_pubkey(), we need to call i2d_ASN1_OCTET_STRING() on CKA_EC_POINT when looking up the keys in eddsa_do_sign_pkcs11(). I had this modification on my local tree when I applied your patch, which is why it worked for me. Two questions (or more):

May I ask you to commit it against the top of my patch? Totally missed it.

  1. Which PKCS#11 provider did you use to test the functionality, where CKA_EC_POINT is not an ASN1-encoded octet string? Shouldn't we make sure this PKCS#11 provider and OpenSC align their interfaces first, to avoid implementing compatibility quirks in ssh-pkcs11.c? This inconsistency on the format of CKA_EC_POINT will make eddsa_do_sign_pkcs11() particularly (and unnecessarily) complex to implement;

I tested it against SoftHSM as it was the only available implementation I had.

  1. Is there a reason eddsa_do_sign_pkcs11() populates key_filter inside the TAILQ_FOREACH() loop, or could we move the code to the function's prelude and perform those steps once?

Unfortunately yes. For RSA and ECDSA this information is cached via the RSA_METHOD/EC_KEY_METHOD structures. See pkcs11_rsa_wrap and pkcs11_ecdsa_wrap for the details. The Edwards curves don't have such specifics structures to store the data so I had to implement some search.

beldmit avatar Jun 01 '21 09:06 beldmit

May I ask you to commit it against the top of my patch? Totally missed it.

I've placed a commit with suggestions here: https://github.com/martelletto/openssh-portable/commit/afe336c9880bac84c70db1f07c3b633ccc23da7e. I couldn't push to your branch (permission denied). Please feel free to squash/cherry-pick/modify as you see fit.

I tested it against SoftHSM as it was the only available implementation I had.

I was a bit puzzled by this, so I had a look. There's a difference in the way ECDSA and EDDSA keys are imported by SoftHSM2's softhsm2-util: ECDSA keys have their CKA_EC_POINT attribute set to an DER-encoded blob, while EDDSA keys have their CKA_EC_POINT attribute set to a raw octet string. AFAICT, the PKCS#11 3.0 spec is clear: CKA_EC_POINT should be DER-encoded in both cases. I suggest we bring this to the attention of SoftHSM's developers, drop support for it in this PR, and move on (that's what's implemented in my 'suggestions' commit above).

martelletto avatar Jun 02 '21 11:06 martelletto

@martelletto, do you have any contact with SoftHSM developers?

beldmit avatar Jun 02 '21 14:06 beldmit

@beldmit No. :(

martelletto avatar Jun 02 '21 15:06 martelletto

Hi @beldmit , Thanks for your response 2 weeks back -- " openssl pkey -in test_ed25519.pem -text gives me the output ... ... "

Have you tried importing an Ed25519 PEM key or mine on your sshd, followed by testing a client connecting to it ? My observation is that the client fails to verify the kex signature from the sshd that imported these keys. Any help is appreciated.

nisamuel avatar Jun 09 '21 06:06 nisamuel