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

Jump to bottom

crypto/ecdh: document, that a key-derivation function is necessary #55977

Open
ostcar opened this issue Sep 30, 2022 · 5 comments
Open

crypto/ecdh: document, that a key-derivation function is necessary #55977

ostcar opened this issue Sep 30, 2022 · 5 comments
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@ostcar
Copy link

ostcar commented Sep 30, 2022

I am excited for the new package crypto/ecdh that will be added to go 1.20.

In the current documentation it says, that Curve.ECHD() returns a shared secret. Later it says, that it performs ECDH as specified in RFC 7748, Section 6.1.

In RFC 7748 in Section 6.1 there is the sentence:

Alice and Bob can then use a key-derivation function that includes K, K_A, and K_B to derive a symmetric key.

The current code of X25519().ECHD does not do the key-derivation but returns the shared secret. This is the documented behavior.

I have two questions about this:

  1. Is it really necessary to use a key-derivation function or could the shared secret be used directly as a symmetric key if the algorithms has the correct key-length?
  2. If it is necessary to use a key-derivation function, then it might be important to add a warning in the documentation of the package, that the returned value should not be used as a key. It is quite easy to forget the key-derivation function. In this case you have code that works, but it not secure. For example:
key, err := X25519().ECDH(privateLocalKey, publicRemoteKey)
if err != nil {
  ...
}
block, err := aes.NewCipher(key)
@dmitshur
Copy link
Contributor

CC @golang/security.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 30, 2022
@dmitshur dmitshur added this to the Backlog milestone Sep 30, 2022
@q0jt
Copy link

q0jt commented Oct 1, 2022

Is it really necessary to use a key-derivation function or could the shared secret be used directly as a symmetric key if the algorithms has the correct key-length?

The shared secret key length is fixed at 32 bytes, so it can be used as a 256-bit key when used with AES.

If it is necessary to use a key-derivation function, then it might be important to add a warning in the documentation of the package, that the returned value should not be used as a key. It is quite easy to forget the key-derivation function. In this case you have code that works, but it not secure. For example:

In terms of security risks, the GCM mode sets an upper limit 2^32 on the number of encryptions for the same key.
Therefore, using the key directly may exceed the upper limit.
To prevent the above security risk, a key-derivation function can be used, but this is beyond the scope of this document.

@ericlagergren
Copy link
Contributor

A warning is a good idea. You should always (at least) hash the Diffie-Hellman value.

@FiloSottile
Copy link
Contributor

crypto/ecdh and ECDH in general are not high-level constructs, so the answer is: "generally, yeah, according to the higher-level protocol you are implementing". You probably want to do key derivation for

  1. avoiding related key attacks
  2. guaranteeing contributory behavior (although how we defined the crypto/ecdh APIs should provide that regardless)
  3. allowing domain separation and producing different lengths of keys

We can add a comment about it, but we should not be too prescriptive given there is a lot of different specifications doing this in different ways.

@mitar
Copy link
Contributor

mitar commented Dec 4, 2023

I think there should be somewhere documentation (even better: a constant) about the size of the shared secret from ECDH.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
7 participants
@mitar @ostcar @FiloSottile @dmitshur @ericlagergren @gopherbot @q0jt