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

Use only ed25519 key by default #1240

Merged
merged 2 commits into from
Sep 1, 2020
Merged

Use only ed25519 key by default #1240

merged 2 commits into from
Sep 1, 2020

Conversation

jagerman
Copy link
Member

We currently generate (for service nodes) both a /key and /key_ed25519
file: the former is a SN's primary monero privkey (i.e. the private key
point of an Ed25519 key, but with no seed so that it can't do full
Ed25519 signatures), the latter is a sodium standard seed+pubkey
concatenated file.

This commit makes lokid stop generate a new /key file: instead, if it
doesn't already exist, we calculate the private key point from the
ed25519 key and set that as the monero-style private key. (The pubkeys
will be the same).

When a /key file exists, we continue to use it for the primary SN
keypair so that existing SN keys won't be affected.

We currently generate (for service nodes) both a /key and /key_ed25519
file: the former is a SN's primary monero privkey (i.e. the private key
point of an Ed25519 key, but with no seed so that it can't do full
Ed25519 signatures), the latter is a sodium standard seed+pubkey
concatenated file.

This commit makes lokid stop generate a new /key file: instead, if it
doesn't already exist, we calculate the private key point from the
ed25519 key and set that as the monero-style private key.  (The pubkeys
will be the same).

When a /key file exists, we continue to use it for the primary SN
keypair so that existing SN keys won't be affected.
jagerman added a commit to jagerman/loki that referenced this pull request Aug 29, 2020
This allows inspecting, generating, and restoring both Ed25519 and
legacy (naming in line with PR oxen-io#1240 now avoiding the non-Ed25519 keys
by default) secret key files as needed by lokid:

    $ ./loki-sn-keys generate key_ed25519
    Generated SN Ed25519 secret key in key_ed25519
    Public key:      8f94f6bbc1d5876484309fc7fd41396c6bd66c4631a34f04895887547cb374dd
    X25519 pubkey:   f3e42d1aff8db8232433ec86e3f56cdda44d4c510144f9b6786520175b08c97f
    Lokinet address: t6kxpq6b4sdsjbbou9d94oj3pti7c5ngggtw6brjmndie9fuquqo.snode

    $ ./loki-sn-keys legacy key
    Generated SN legacy private key in key
    Public key: f3a513ddfbe946c79c62c27cca2260b80606a2da1b4f75ba16c793a5750a8510

    $ ./loki-sn-keys show key
    key (legacy SN keypair)
    ==========
    Private key: c8c17cc296c3e8bb603098b5d2535ad86662cdb24cd4c785c0485d0a490a2356
    Public key:  f3a513ddfbe946c79c62c27cca2260b80606a2da1b4f75ba16c793a5750a8510

    $ ./loki-sn-keys show key_ed25519
    key_ed25519 (Ed25519 SN keypair)
    ==========
    Secret key:      3f078900bac5f3e97d5fe451a6e332826d29aae2e13fec895c1b527af88093c3
    Public key:      8f94f6bbc1d5876484309fc7fd41396c6bd66c4631a34f04895887547cb374dd
    X25519 pubkey:   f3e42d1aff8db8232433ec86e3f56cdda44d4c510144f9b6786520175b08c97f
    Lokinet address: t6kxpq6b4sdsjbbou9d94oj3pti7c5ngggtw6brjmndie9fuquqo.snode

    $ ./loki-sn-keys restore key_ed25519-2
    Enter the Ed25519 secret key:
    3f078900bac5f3e97d5fe451a6e332826d29aae2e13fec895c1b527af88093c3

    Public key:      8f94f6bbc1d5876484309fc7fd41396c6bd66c4631a34f04895887547cb374dd
    X25519 pubkey:   f3e42d1aff8db8232433ec86e3f56cdda44d4c510144f9b6786520175b08c97f
    Lokinet address: t6kxpq6b4sdsjbbou9d94oj3pti7c5ngggtw6brjmndie9fuquqo.snode

    Is this correct?  Press Enter to continue, Ctrl-C to cancel.

    Saved secret key to key_ed25519-2

    $ ./loki-sn-keys restore-legacy key-2
    Enter the legacy SN private key:
    c8c17cc296c3e8bb603098b5d2535ad86662cdb24cd4c785c0485d0a490a2356

    Public key:      f3a513ddfbe946c79c62c27cca2260b80606a2da1b4f75ba16c793a5750a8510

    Is this correct?  Press Enter to continue, Ctrl-C to cancel.

    Saved secret key to key-2
Copy link
Collaborator

@Doy-lee Doy-lee left a comment

Choose a reason for hiding this comment

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

service_node_keys description in service_node_list should also be updated now to reflect the changes here.

if (m_service_node) {
if (!epee::file_io_utils::is_file_exist(m_config_folder + "/key")) {
epee::wipeable_string privkey_signhash;
privkey_signhash.resize(64);
Copy link
Collaborator

Choose a reason for hiding this comment

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

crypto_hash_sha512_BYTES from src/libsodium/include/sodium/crypto_hash_sha512.h

The 32 passed into crypto_hash_sha512 looked like it only needed 32 bytes for both buffers.

Comment on lines 983 to 985
pk_sh_data[0] &= 248;
pk_sh_data[31] &= 63;
pk_sh_data[31] |= 64;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need some reference of where this was pulled from. In Sodium this is where it's done

https://github.com/jedisct1/libsodium/blob/master/src/libsodium/crypto_sign/ed25519/ref10/keypair.c#L70

But they do

    crypto_hash_sha512(h, ed25519_sk, 32);
    h[0] &= 248;
    h[31] &= 127;
    h[31] |= 64;

I've found it in our code-base in the reference crypto implementations, would be good to just drop a comment to that.

Though, its slightly confusing in that we amalgamate the reference implementation into crypto-ops which doesn't include this signing code. Has it been verified that these changes are sufficient for a legacy key owner to validate a signature from a post v8.x SN and vice versa?

Copy link
Member Author

Choose a reason for hiding this comment

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

Those two bitwise operations are identical; the only difference is whether or not it clears the 7th bit or not before it explicitly sets it.

Since all this is doing is changing the private key value (to prevent small subgroup attacks), and Monero keys are unrestrained values, this will always be a valid Monero private key point, but many Monero key points won't be possible with this generation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah yeah nice, they do work out to be the same.

Doy-lee pushed a commit that referenced this pull request Sep 1, 2020
This allows inspecting, generating, and restoring both Ed25519 and
legacy (naming in line with PR #1240 now avoiding the non-Ed25519 keys
by default) secret key files as needed by lokid:

    $ ./loki-sn-keys generate key_ed25519
    Generated SN Ed25519 secret key in key_ed25519
    Public key:      8f94f6bbc1d5876484309fc7fd41396c6bd66c4631a34f04895887547cb374dd
    X25519 pubkey:   f3e42d1aff8db8232433ec86e3f56cdda44d4c510144f9b6786520175b08c97f
    Lokinet address: t6kxpq6b4sdsjbbou9d94oj3pti7c5ngggtw6brjmndie9fuquqo.snode

    $ ./loki-sn-keys legacy key
    Generated SN legacy private key in key
    Public key: f3a513ddfbe946c79c62c27cca2260b80606a2da1b4f75ba16c793a5750a8510

    $ ./loki-sn-keys show key
    key (legacy SN keypair)
    ==========
    Private key: c8c17cc296c3e8bb603098b5d2535ad86662cdb24cd4c785c0485d0a490a2356
    Public key:  f3a513ddfbe946c79c62c27cca2260b80606a2da1b4f75ba16c793a5750a8510

    $ ./loki-sn-keys show key_ed25519
    key_ed25519 (Ed25519 SN keypair)
    ==========
    Secret key:      3f078900bac5f3e97d5fe451a6e332826d29aae2e13fec895c1b527af88093c3
    Public key:      8f94f6bbc1d5876484309fc7fd41396c6bd66c4631a34f04895887547cb374dd
    X25519 pubkey:   f3e42d1aff8db8232433ec86e3f56cdda44d4c510144f9b6786520175b08c97f
    Lokinet address: t6kxpq6b4sdsjbbou9d94oj3pti7c5ngggtw6brjmndie9fuquqo.snode

    $ ./loki-sn-keys restore key_ed25519-2
    Enter the Ed25519 secret key:
    3f078900bac5f3e97d5fe451a6e332826d29aae2e13fec895c1b527af88093c3

    Public key:      8f94f6bbc1d5876484309fc7fd41396c6bd66c4631a34f04895887547cb374dd
    X25519 pubkey:   f3e42d1aff8db8232433ec86e3f56cdda44d4c510144f9b6786520175b08c97f
    Lokinet address: t6kxpq6b4sdsjbbou9d94oj3pti7c5ngggtw6brjmndie9fuquqo.snode

    Is this correct?  Press Enter to continue, Ctrl-C to cancel.

    Saved secret key to key_ed25519-2

    $ ./loki-sn-keys restore-legacy key-2
    Enter the legacy SN private key:
    c8c17cc296c3e8bb603098b5d2535ad86662cdb24cd4c785c0485d0a490a2356

    Public key:      f3a513ddfbe946c79c62c27cca2260b80606a2da1b4f75ba16c793a5750a8510

    Is this correct?  Press Enter to continue, Ctrl-C to cancel.

    Saved secret key to key-2
@Doy-lee Doy-lee merged commit b0f8ffa into oxen-io:dev Sep 1, 2020
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.

2 participants