Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for ECDSA and ED #48

Merged
merged 73 commits into from
Feb 11, 2021
Merged

Add support for ECDSA and ED #48

merged 73 commits into from
Feb 11, 2021

Conversation

adiroiban
Copy link
Member

@adiroiban adiroiban commented Jan 6, 2021

Scope

Fixes #42
Fixes #39
Fixes #38

This adds support for ECDSA and ED private keys.

It support OpenSSH and PuttyFormat.

Old OpenSSH format and SSH.COM format only support RSA and DSA.

This also removes the usage of pycrypto.

Changes

Use the code from upstream Twisted, revision 38a1ed6f9

As a side effect, this stops using pycrypto and uses crypography instead.

I have updated the keycert-demo.py command to allow re-encoding a file after loading and encrypting the re-encoded file.
The demo also has support for signing and validating the signature.

How to try and test the changes

reviewers: @dumol @lgheorghiu

There is a lot to test here :)

Version 2.0.0 was released on our private PyPI to help with chevah/server testing and Laura will test the server integration.

1.Load EC and ED keys

Try to generate ECDSA and ED keys using OpenSSH and PuttyGen and check that you can load them... with and without password. This is for public and private keys.

There is also support for loading ECDSA and ED keys from OpenSSL X509 PEM public keys.

Then there is also the PKCS#8 (public and private) and public PKCS#1 :)

# Load ECDSA and output as OpenSSH without password
./build-keycert/bin/python keycert-demo.py ssh-load-key --file ecdsa-521.ppk

# Load password protected ECDSA key and output as PuTTY format without password 
./build-keycert/bin/python keycert-demo.py ssh-load-key --file ecdsa-521.ppk --type putty --password chevah

2.Generate EC and ED keys

Check key generation help CLI as this also goes to chevah/server

$ ./build-keycert/bin/python keycert-demo.py ssh-gen-key --help

Generate the key in OpenSSH format and then check that you can load them / convert them with OpenSSH or Putty.

$ ./build-keycert/bin/python keycert-demo.py ssh-gen-key --key-file id_ecdsa --key-type ecdsa --key-size=384

Generate key in putty format with password protection

$ ./build-keycert/bin/python keycert-demo.py ssh-gen-key --key-type ed25519 --key-format putty --key-password chevah

3.Sign and verify data

# Generate the signature in base 64 using private key.
$ ./build-keycert/bin/python keycert-demo.py ssh-sign-data --file id_ecdsa --data test-value

# Validate the signature against the original data and public key.
$ ./build-keycert/bin/python keycert-demo.py ssh-verify-data --file id_ecdsa.pub --data test-value --signature AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAABpAAAAMQC2h6Akl/26ln+WxWqW8u38YQlUaR9rMt24E6/9Cl751UNQ0/e3xKzPMp75A6GZnIMAAAAwCCikhECXzFc2j6/+4nJ0VzBUJDi2WRS+OJiVozzlrNCMf3er2lM3lp4QLPP1ltLj

@adiroiban
Copy link
Member Author

needs-review

@adiroiban
Copy link
Member Author

adiroiban commented Jan 7, 2021

also added @lgheorghiu to needs-review for Windows testing

@dumol
Copy link
Contributor

dumol commented Jan 7, 2021

I've only tested the first scenario. Some things work, but I've already had some issues with deciphering private keys with non-empty passwords, no matter the format.

To decipher private ECDSA keys with non-empty passwords, I've tried both OpenSSH and Putty, for both I get: Passphrase must be provided for an encrypted key. even though I do append the password after --extra.

The commands to generate using the easy password interactively (also, the length is not an issue, I've also tried with a length of 256):

ssh-keygen -b 512 -t ecdsa
puttygen -t ecdsa -b 521

The commands to decipher:

$CHEVAH_BUILD/bin/python keycert-demo.py ssh-load-key --file ssh521_id_ecdsa --type openssh --extra hopasieu
$CHEVAH_BUILD/bin/python keycert-demo.py ssh-load-key --file putty521_id_ecdsa --type putty --extra hopasieu

The same for private ED25519 keys, generated with either OpenSSH or Putty. For the latter, I've also tried private-openssh and private-openssh-new output options, as I was confounded by the error you get when you specify the type for a private ED25519 key:

$CHEVAH_BUILD/bin/python keycert-demo.py ssh-load-key --file ssh_id_ed25519 --type openssh   
Traceback (most recent call last):
  File "keycert-demo.py", line 198, in <module>
    result = options.handler(options)
  File "keycert-demo.py", line 56, in ssh_load_key
    to_string = key.toString(output_format, passphrase=extra)
  File "/home/dumol/src/chevah-keycert/chevah/keycert/ssh.py", line 1405, in toString
    return method(comment=comment, passphrase=passphrase)
  File "/home/dumol/src/chevah-keycert/chevah/keycert/ssh.py", line 1521, in _toString_OPENSSH
    'cannot serialize Ed25519 key to OpenSSH PEM format; use v1 '
ValueError: cannot serialize Ed25519 key to OpenSSH PEM format; use v1 instead

If you don't specify the type for a private ED25519 key, it just works for both OpenSSH and Putty keys (with all its variants).

needs-changes

@adiroiban
Copy link
Member Author

private ED25519 key,

I don't think that OpenSSH supports ED25519 using the old format.
Can you generate an ED25519 private key using the old OpenSSH format?


I have updated the CLI to allow loading password protected keys.

@adiroiban
Copy link
Member Author

needs-review

@dumol
Copy link
Contributor

dumol commented Jan 8, 2021

private ED25519 key,

I don't think that OpenSSH supports ED25519 using the old format.
Can you generate an ED25519 private key using the old OpenSSH format?

This specific case is related to using Putty's puttygen, which has two options for the -O parameter: private-openssh and private-openssh-new. I've tried both in relation with the -t ed25519 parameter.

Maybe it doesn't make a difference in this case? --help shows:

           private-openssh     export OpenSSH private key
           private-openssh-new export OpenSSH private key (force new format)

Could be read as forcing new format where there's an option otherwise.

@dumol
Copy link
Contributor

dumol commented Jan 8, 2021

I've tried to automate scenario 1 this is what I've got so far.

It tests public and private keys in all formats supported by ssh-keygen and puttygen (including the non-native ones). For RSA and DSA keys too, while at it, but beware that large key sizes take lots of time to generate…

#!/usr/bin/env bash

set -euo pipefail

keycert_cmd="$CHEVAH_BUILD/bin/python keycert-demo.py"

# Empty file holding test errors.
> ssh_keys_tests_errors

# Files holding passwords.
> pass_file_empty
echo 'chevah' > pass_file_simple
echo 'V^#ev1uj#kq$N' > pass_file_complex
pass_types="empty simple complex"

openssh_keys="ed25519 ecdsa rsa dsa"
openssh_formats="RFC4716 PKCS8 PEM"

# rsa1 is not tested.
putty_keys="ed25519 ecdsa rsa dsa"
# private-sshcom doesn't work with ed25519 and ecdsa in puttygen 0.74.
putty_priv_outputs="private private-openssh private-openssh-new private-sshcom"
putty_pub_outputs="public public-openssh"

# First parameter is the private or public key file.
# Second (optional) parameter is the password.
keycert_load_key(){
    local keycert_opts="ssh-load-key --file $1"
    if [ "$#" = 2 ]; then
        local keycert_opts="$keycert_opts --password $2"
    fi
    set +e
    $keycert_cmd $keycert_opts
    if [ $? -ne 0 ]; then
        echo $1 >> ssh_keys_tests_errors 
    fi
    set -e
}

putty_gen_test_keys(){
    local bit_lengths_to_test="$1"

    for bits in $bit_lengths_to_test; do
        for pass_type in $pass_types; do
            echo "Testing $key key of type $priv_output and size $bits with $pass_type password:"
            priv_key_file="putty_${key}_${bits}_${priv_output}"
            pass_file="pass_file_${pass_type}"
            puttygen --new-passphrase $pass_file -t $key -O $priv_output -b $bits -o $priv_key_file
            # Test new private key.
            keycert_load_key $priv_key_file $(cat $pass_file)
            # Extract and test public key.
            for pub_output in $putty_pub_outputs; do
                echo "Testing $key key of type $pub_output and size $bits with $pass_type password:"
                pub_key_file="putty_${key}_${bits}_${pub_output}"
                puttygen --old-passphrase $pass_file -O $pub_output -o $pub_key_file $priv_key_file
                keycert_load_key $pub_key_file
            done
        done
    done
}

openssh_gen_test_keys(){
    local bit_lengths_to_test="$1"

    for bits in $bit_lengths_to_test; do
        for pass_type in $pass_types; do
            pass_file="pass_file_${pass_type}"
            # Default format.
            priv_key_file=openssh_${key}_${bits}_default_${pass_type}
            pub_key_file=$priv_key_file.pub
            if [ $pass_type = "empty" ]; then
                ssh-keygen -t $key -b $bits -N "" -f $priv_key_file
            else
                ssh-keygen -t $key -b $bits -N $(cat $pass_file) -f $priv_key_file
            fi
            keycert_load_key $priv_key_file $(cat $pass_file)
            keycert_load_key $pub_key_file
            for format in $openssh_formats; do
                # Other formats.
                priv_key_file=openssh_${key}_${bits}_${format}_${pass_type}
                pub_key_file=$priv_key_file.pub
                case $format in
                    "PKCS8")
                    if [ $pass_type = "empty" ]; then
                        if [ $key = "ecdsa" -o $key = "rsa" -o $key = "dsa" ]; then
                            # Minimum 5 characters required for these combinations.
                            (>&2 echo "Not testing $format with $key when pass is $pass_type")
                            break
                        fi
                    fi
                    ;;
                esac
                if [ $pass_type = "empty" ]; then
                    ssh-keygen -t $key -b $bits -N "" -m $format -f $priv_key_file
                else
                    ssh-keygen -t $key -b $bits -N $(cat $pass_file) -m $format -f $priv_key_file 
                fi
                keycert_load_key $priv_key_file $(cat $pass_file)
                keycert_load_key $pub_key_file
            done
        done
    done
}

# OpenSSH's ssh-keygen tests.
for key in $openssh_keys; do
    case $key in
        "ed25519")
            openssh_gen_test_keys "256"
            ;;
        "ecdsa")
            openssh_gen_test_keys "256 384 521"
            ;;
        "rsa")
            # An unusual prime size is also tested.
            openssh_gen_test_keys "1024 2048 2111 3072 4096 8192"
            ;;
        "dsa")
            openssh_gen_test_keys "1024"
            ;;
    esac
done

# Putty's puttygen tests.
for key in $putty_keys; do
    for priv_output in $putty_priv_outputs; do
        if [ $key = "ed25519" -a $priv_output = "private-openssh-new" ]; then
            # No need to force new OpenSSH format for ED25519 keys.
            break
        fi
        if [ $priv_output = "private-sshcom" ]; then
            if [ $key = "ed25519" -o $key = "ecdsa" ]; then
                # Not working in puttygen 0.74.
                break
            fi
        fi
        # Test specific numbers of bits per key type.
        case $key in
            "ed25519")
                putty_gen_test_keys "256"
                ;;
            "ecdsa")
                putty_gen_test_keys "256 384 521"
                ;;
            "rsa")
                # An unusual prime size is also tested.
                putty_gen_test_keys "1024 2048 2111 3072 4096 8192"
                ;;
            "dsa")
                putty_gen_test_keys "1024 2048 3072 4096"
                ;;
        esac
    done
done

echo "Combinations tested:"
ls -1 openssh_* putty_*

echo "Errors:"
cat ssh_keys_tests_errors

# Cleanup test files.
rm pass_file_* openssh_* putty_*

@dumol
Copy link
Contributor

dumol commented Jan 11, 2021

Updated the above script to also test public keys.

@dumol
Copy link
Contributor

dumol commented Jan 12, 2021

Updated the above script to also test private and public keys generated by OpenSSH's ssh-keygen for all supported formats (the extra ones are RFC4716, PKCS8, and PEM).

For the record, there is one combination that fails: private ECDSA keys in PKCS8 format using non-empty passwords. Commands to reproduce:

ssh-keygen -t ecdsa -m PKCS8 -N chevah -f new_key                                                                                            
$CHEVAH_BUILD/bin/python keycert-demo.py ssh-load-key --file new_key --password chevah

The error is:

Unsupported key found in the PKCS#8 private PEM file.

@adiroiban
Copy link
Member Author

Thanks @dumol for the review and tests :)

I will try to convert your tests into python test and include them in our GitHub ubuntu automated test run.

In theory we don't need python. we can just create separate "integration" workflow yaml file that will run your script with code from current branch.

needs-review

@dumol
Copy link
Contributor

dumol commented Jan 13, 2021

Thanks @dumol for the review and tests :)

Thanks for the fix, can confirm last commit fixes the above issue with private ECDSA keys in PKCS8 format using non-empty passwords.

However, at most I can say I've covered step 1 in the PR's testing scenario… (Haven't looked yet at testing OpenSSL X509 PEM public keys).

@dumol
Copy link
Contributor

dumol commented Jan 13, 2021

Have cleaned-up my script a bit and added comments for the generated keys with both ssh-keygen and puttygen.

For now, have also commented out generating RSA and DSA keys to keep the execution time under 1 minute.

Committed to chevah/keycert/tests/ssh_keys_tests.sh, not sure if it's the right place…

Next, I would add testing keys generated with Tectia's ssh-keygen-g3, which sports some more interesting combinations of formats and specific support for FIPS mode.

@dumol
Copy link
Contributor

dumol commented Jan 14, 2021

As discussed on #chevah today, there's an issue handling cryptography errors, e.g. when dealing with non-standard DSA key sizes.

Commands to reproduce:

>empty_file
puttygen -t dsa -b 2111 --new-passphrase empty_file -o dsa2111.ppk
./build-keycert/bin/python keycert-demo.py ssh-load-key --file dsa2111.ppk

Current output:

raceback (most recent call last):
  File "keycert-demo.py", line 204, in <module>
    result = options.handler(options)
  File "keycert-demo.py", line 57, in ssh_load_key
    key = Key.fromString(key_content, passphrase=password)
  File "/home/dumol/src/chevah-keycert/chevah/keycert/ssh.py", line 247, in fromString
    return method(data, passphrase)
  File "/home/dumol/src/chevah-keycert/chevah/keycert/ssh.py", line 2343, in _fromString_PRIVATE_PUTTY
    return cls._fromDSAComponents(y=y, g=g, p=p, q=q, x=x)
  File "/home/dumol/src/chevah-keycert/chevah/keycert/ssh.py", line 882, in _fromDSAComponents
    keyObject = privateNumbers.private_key(default_backend())
  File "/home/dumol/src/chevah-keycert/build-keycert/lib/python2.7/site-packages/cryptography/hazmat/primitives/asymmetric/dsa.py", line 250, in private_key
    return backend.load_dsa_private_numbers(self)
  File "/home/dumol/src/chevah-keycert/build-keycert/lib/python2.7/site-packages/cryptography/hazmat/backends/openssl/backend.py", line 852, in load_dsa_private_numbers
    dsa._check_dsa_private_numbers(numbers)
  File "/home/dumol/src/chevah-keycert/build-keycert/lib/python2.7/site-packages/cryptography/hazmat/primitives/asymmetric/dsa.py", line 147, in _check_dsa_private_numbers
    _check_dsa_parameters(parameters)
  File "/home/dumol/src/chevah-keycert/build-keycert/lib/python2.7/site-packages/cryptography/hazmat/primitives/asymmetric/dsa.py", line 136, in _check_dsa_parameters
    "p must be exactly 1024, 2048, 3072, or 4096 bits long"
ValueError: p must be exactly 1024, 2048, 3072, or 4096 bits long

@dumol
Copy link
Contributor

dumol commented Jan 14, 2021

Added support for testing with Tectia's ssh-keygen-g3 tool. This results in 10 times more keys to test, as this tool supports lots of formats and also takes various key hashes as an option for all types of output. Then there's also FIPS mode for some combinations of options.

However, lots of private keys generated by the Tectia tools are not loaded by the keycert demo. So far, I've seen two types of errors. Here's how to reproduces them (use the CentOS 8 VM at 172.20.4.137):

$ ssh-keygen-g3 -t ed25519 -p chevah `pwd`/tectia_ed25519_key

$ ./build-keycert/bin/python keycert-demo.py ssh-load-key --file tectia_ed25519_key --password chevah
unknown encryption type 'aes256-cbc'

$ cat tectia_ed25519_key
-----BEGIN OPENSSH PRIVATE KEY-----
b3BlbnNzaC1rZXktdjEAAAAACmFlczI1Ni1jYmMAAAAGYmNyeXB0AAAAGAAAABB4
k+ux2Po71qZkKV6SuhjFAAAAEAAAAAEAAAAzAAAAC3NzaC1lZDI1NTE5AAAAIDU7
PuP3ismiQabjWElVmfRCPr1G5nS6OWnwIM8/7UHpAAAA4IPvRrxUzPxQ27hnT+bj
Z7PcVOh09Y+0YUG4EV3T4IQG9JuNUPqmtQBUkwN/faaX+kgl66KJca4Z10wSyFEQ
0YaC9xjIfKXsO2a11jc7BmEyagOaEW1R/nTw8zNlKsjhqmo8FKr1o/6TL5AzqTP0
xAoz7Mnh5DCQw/6bbqloFC0e3R5XmjHGM3sBiTodp6ebhiF8WHdI4xeInscfodR+
5/RfWbAGMc5x+RohFOT++R5PdTDYFRnwbhmwr/H+/r5BMbgh+ze6fgJsPa9XeIXy
M2CPOxRjTYFfEDRevYYiGUdt
-----END OPENSSH PRIVATE KEY-----
$ ssh-keygen-g3 -t ecdsa -p chevah `pwd`/tectia_ecdsa_key

$ ./build-keycert/bin/python keycert-demo.py ssh-load-key --file tectia_ecdsa_key --password chevah
Unknown SSH.com key type ec-modp

$ cat tectia_ecdsa_key
---- BEGIN SSH2 ENCRYPTED PRIVATE KEY ----
Subject: chevah
Comment: "256-bit ecdsa, chevah@bs2a-lnx-centos8-x64-137, Thu Jan 14 2\
021 15:23:36 +0200"
P2/56wAAAFsAAAAHZWMtbW9kcAAAAAgzZGVzLWNiYwAAADhhndx4DBnaD2Sw4bJyzRJPTL
8wxe/N1fiqei4iy0RCJy/F6QTXJTgzWLIXmjAK5FnfLbn5VDyDhA==
---- END SSH2 ENCRYPTED PRIVATE KEY ----

@adiroiban
Copy link
Member Author

Thanks for the review. I have created #50 and #51 for unicode support.
There is also #49 for better error messages on key generation.

The IOError for path not found, is for the helper code from the demo. And that error is expected.
I have updated the demo script to be explicit about the raised errors.


Searching for errors is not easy with current script.
I see that putty_ecdsa_256_public is listed twice as failing, but if I don't know where to look for that error.

This is already a big PR,
I think that is best to look at improving the interop tests in a separate PR and I think that is better to write the scripts in Python.
I hope @danuker can help.

needs-review

Copy link
Contributor

@dumol dumol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IOError for path not found, is for the helper code from the demo. And that error is expected.
I have updated the demo script to be explicit about the raised errors.

OK, thanks. Have also updated the interop test script for loading keys to handle all current error codes.

Searching for errors is not easy with current script.
I see that putty_ecdsa_256_public is listed twice as failing, but if I don't know where to look for that error.

It was listed twice because public keys were generated for both private formats supported by Putty for Ed25519 keys: private and private-openssh.

As there was no point in testing both scenarios, I've updated the script to only generate and test public keys for Putty's own private keys. More so, only when the complex password is used to generate the private key, as the password type is not relevant here.

On top of that, I've added testing different comment types independently of the type of password.

Current failing results for Ed25519 keys would be:

Combinations with expected errors: 3
putty_ed25519_256_private_emptypass_complexcomm
putty_ed25519_256_private_simplepass_complexcomm
putty_ed25519_256_private_complexpass_complexcomm

Combinations with unexpected errors: 1
putty_ed25519_256_public_complexpass_unicodecomm

Is this clear enough? If not, there's always the option to use bash -x for quick debugging… ;-]

However, in my last commit I've disabled tests that fail for known issues and added relevant comments.

Therefore, all interop tests should be now green. :-]

changes-approved

@adiroiban
Copy link
Member Author

Is this clear enough? If not, there's always the option to use bash -x for quick debugging… ;-]

Thanks. Looks good :)

When migrating to Py3 I hope we can also migrate these tests to Python3.

The the current tests are excellent. Many thanks for your work.

We also need something like this for chevah/server to test with all Host Key, KEX, CIphers, HMAC combinations :)

There is still more work here, but I think we got a good start with ED and ECDSA.

I will merge this and we will improve the test during the Py3 migration.

I have to clean the current Python test so that they will also be green.

@adiroiban adiroiban merged commit 02bf197 into master Feb 11, 2021
@adiroiban adiroiban deleted the use-cryptography-take2 branch February 11, 2021 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for ED keys Add support for EC keys Support EC keys in SSH.com and Putty format
2 participants