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

secret_connection: fix bogus identity key handling #164

Merged
merged 1 commit into from
Oct 13, 2020

Commits on Oct 13, 2020

  1. secret_connection: fix bogus identity key handling

    The `SecretConnection { remote_pubkey }` field is intended to store the
    remote peer's static Ed25519 identity key.
    
    However, during the AKE verification process, it was previously
    temporarily initialized to the remote peer's ephemeral X25519 key.
    This only worked because previously it's using Signatory's
    `ed25519::PublicKey` type, which does not apply point decompression
    (as that involves solving the curve equation, which requires a basic
    curve arithmetic backend).
    
    However, this did cause sporadic failures in the Secret Connection AKE
    after attempting to switch this key type out for ed25519-dalek's
    `PublicKey` type, which does decompress points (#162). This caused
    sporadic failures whever the X25519 ephemeral key failed to decompress
    as an Ed25519 public key (which is mere coincidence, as attempting to do
    so makes no sense mathematically).
    
    This commit changes `remote_pubkey` to an `Option` and doesn't attempt
    to store any key until validated. This is perhaps needlessly fallible,
    but at least addresses the immediate problem.
    
    There doesn't appear to be any immediate security impact from this:
    while in combination with other bugs it could potentially be used to
    forge an identity key, the keys are cryptographically validated by
    subsequent steps, and any failure to validate them aborts the handshake.
    Upon success, the identity key is always overwritten with the correct
    one. A security vulnerability would require some way to successfully
    complete the handshake in spite of the cryptographic errors and without
    the key being overwritten.
    tony-iqlusion committed Oct 13, 2020
    Configuration menu
    Copy the full SHA
    06579a4 View commit details
    Browse the repository at this point in the history