Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

Invalid Ed25519 Go Interop #175

Closed
carsonfarmer opened this issue May 27, 2020 · 4 comments · Fixed by #178
Closed

Invalid Ed25519 Go Interop #175

carsonfarmer opened this issue May 27, 2020 · 4 comments · Fixed by #178
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked

Comments

@carsonfarmer
Copy link
Contributor

The private key marshaling of Ed25519 keys in Go no longer includes the redundant public key bytes (libp2p/go-libp2p-crypto#54). Unfortunately, the go-interop tests in this library are based on the old marshaling with the redundant public key bytes, and as such unmarshalEd25519PrivateKey throws when it shouldn't (when the redundant bytes are missing).

The 'fix' is now here in Go: https://github.com/libp2p/go-libp2p-core/blob/master/crypto/ed25519.go#L131.

@carsonfarmer
Copy link
Contributor Author

carsonfarmer commented May 27, 2020

To test this, one can simply re-generate the go-key-ed25519.js test fixtures, based on the instructions in the comments (here's an example):

{
    privateKey: Buffer.from([8, 1, 18, 64, 155, 239, 222, 118, 236, 90, 110, 88, 231, 141, 170, 117, 219, 216, 115, 244, 80, 92, 16, 6, 157, 157, 98, 66, 170, 177, 118, 240, 205, 191, 49, 82, 158, 27, 235, 18, 133, 243, 1, 53, 246, 180, 14, 66, 126, 27, 227, 227, 23, 97, 244, 26, 80, 224, 222, 221, 17, 195, 6, 224, 217, 170, 231, 139]),
    publicKey: Buffer.from([8, 1, 18, 32, 158, 27, 235, 18, 133, 243, 1, 53, 246, 180, 14, 66, 126, 27, 227, 227, 23, 97, 244, 26, 80, 224, 222, 221, 17, 195, 6, 224, 217, 170, 231, 139]),
    data: Buffer.from([104, 101, 108, 108, 111, 33, 32, 97, 110, 100, 32, 119, 101, 108, 99, 111, 109, 101, 32, 116, 111, 32, 115, 111, 109, 101, 32, 97, 119, 101, 115, 111, 109, 101, 32, 99, 114, 121, 112, 116, 111, 32, 112, 114, 105, 109, 105, 116, 105, 118, 101, 115]),
    signature: Buffer.from([19, 82, 7, 49, 108, 87, 8, 98, 51, 59, 87, 233, 67, 123, 130, 208, 219, 216, 117, 183, 252, 7, 132, 136, 94, 234, 82, 226, 103, 183, 133, 12, 212, 111, 184, 156, 83, 107, 172, 3, 124, 19, 198, 155, 40, 108, 201, 127, 18, 34, 108, 213, 239, 52, 97, 19, 166, 186, 55, 95, 29, 72, 164, 5])
}

and run the tests. You should end up with:

  148 passing (2s)
  1 pending
  1 failing

  1) ed25519
       go interop
         "before all" hook for "verifies with data from go":
     Error: Key must be a Uint8Array or Buffer of length 96
      at ensureKey (src/keys/ed25519-class.js:118:19)
      at Object.unmarshalEd25519PrivateKey (src/keys/ed25519-class.js:92:11)
      at Object.exports.unmarshalPrivateKey (src/keys/index.js:93:36)
      at Context.<anonymous> (test/keys/ed25519.spec.js:140:37)
      at processImmediate (internal/timers.js:456:21)

@carsonfarmer
Copy link
Contributor Author

Here's a simple Go program to generate new values in case its useful:

package main

import (
	"crypto/rand"
	"fmt"
	"strings"

	"github.com/libp2p/go-libp2p-core/crypto"
)

func main() {
	priv, pub, _ := crypto.GenerateEd25519Key(rand.Reader)
	pubkeyBytes, _ := pub.Bytes()
	privkeyBytes, _ := priv.Bytes()
	data := []byte("hello! and welcome to some awesome crypto primitives")
	sig, _ := priv.Sign(data)
	fmt.Println("{\n  publicKey: Buffer.from(", strings.Replace(fmt.Sprint(pubkeyBytes), " ", ",", -1), "),")
	fmt.Println("  privateKey: Buffer.from(", strings.Replace(fmt.Sprint(privkeyBytes), " ", ",", -1), "),")
	fmt.Println("  data: Buffer.from(", strings.Replace(fmt.Sprint(data), " ", ",", -1), "),")
	fmt.Println("  signature: Buffer.from(", strings.Replace(fmt.Sprint(sig), " ", ",", -1), ")\n}")
}

@jacobheun jacobheun added kind/bug A bug in existing code (including security flaws) exp/novice Someone with a little familiarity can pick up P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked P2 Medium: Good to have, but can wait until someone steps up and removed P1 High: Likely tackled by core team if no one steps up labels May 27, 2020
jacobheun added a commit that referenced this issue Jul 18, 2020
jacobheun added a commit that referenced this issue Jul 20, 2020
@jacobheun
Copy link
Contributor

This is fixed in 0.17.8. We're still marshaling with the redundant key by default to avoid a breaking change. We'll track updating that in a future, breaking release.

achingbrain added a commit to ipfs/js-ipfs that referenced this issue Jul 21, 2020
@jacobheun
Copy link
Contributor

This is fixed in 0.17.8. We're still marshaling with the redundant key by default to avoid a breaking change. We'll track updating that in a future, breaking release.

The default has been changed in 0.18.0, so the redundant key is no longer included when js marshals the key.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants