feat(wire): add ecdsa authentication to wire #51
Conversation
Signed-off-by: Minh Huy Tran <[email protected]>
Signed-off-by: Minh Huy Tran <[email protected]>
| hash := PrefixedHash(msg) | ||
| sigCopy := make([]byte, SigLen) | ||
| copy(sigCopy, sig) | ||
| if len(sigCopy) == SigLen && (sigCopy[SigLen-1] >= sigVSubtract) { |
There was a problem hiding this comment.
Is it intentional that we don't compare sig but we compare the length after we have copied? I don't think it's a problem, but it might be better if we have an early length check and return an error immediately if sig is not of length 65. Could also result in a nicer error message if the length doesn't match.
I think this implementation allows 65 byte signatures with arbitrary bytes after it, not sure if that's a problem.
From the copy() docs: minimum of len(src) and len(dst) (https://pkg.go.dev/builtin#copy)
For example, the following would be seen as valid by this function:
- <65-bytes-valid-signature>
- <65-bytes-valid-signature>
As said: I don't think it's problematic, but I think it would be cleaner if this function would only allow signatures of exactly 65 bytes.
Signed-off-by: Minh Huy Tran <[email protected]>
Signed-off-by: Minh Huy Tran <[email protected]>
NhoxxKienn
force-pushed
the
draft/egoistic-funding
branch
from
c2a8f37
to
4c96443
Compare
NhoxxKienn
force-pushed
the
draft/egoistic-funding
branch
2 times, most recently
from
89961b9
to
5507b04
Compare
Signed-off-by: Minh Huy Tran <[email protected]>
NhoxxKienn
force-pushed
the
draft/egoistic-funding
branch
3 times, most recently
from
3f6ca74
to
e360848
Compare
Signed-off-by: Minh Huy Tran <[email protected]>
NhoxxKienn
force-pushed
the
draft/egoistic-funding
branch
from
e360848
to
81b5fc8
Compare
|
Add a test to create go-perun client with the new wire/net/bus in |
|
|
||
| hosts := make([]string, len(names)) | ||
| for i := 0; i < len(names); i++ { | ||
| port, err := findFreePort() |
There was a problem hiding this comment.
Auf lange Sicht wird es vermutlich besser sein eine Funktion zum listener hinzuzufügen (wenn er nicht schon eine hat) um den Port zu bekommen. Einen tcp listener aufzumachen nur um den Port zu bekommen erscheint mir unnötig.
Also effektiv erst die listener erdstellen und dann für jeden listener den Port abfragen (wie es in findFreePorts gemacht wird) + evtl. kleine Anpassung am perun code um diese Funktionalität bereitzustellen.
| if err != nil { | ||
| return nil, errors.Wrap(err, "SignHash") | ||
| } | ||
| sig[64] += 27 |
There was a problem hiding this comment.
wenn wir schon sigVSubtract als konstante haben sollten wir die hier auch verwenden. Evtl. ist sigVOffset ein passenderer Name.
| log.Print("Generated new account with address ", crypto.PubkeyToAddress(privateKey.PublicKey).Hex()) | ||
|
|
||
| addr := crypto.PubkeyToAddress(privateKey.PublicKey) |
There was a problem hiding this comment.
| log.Print("Generated new account with address ", crypto.PubkeyToAddress(privateKey.PublicKey).Hex()) | |
| addr := crypto.PubkeyToAddress(privateKey.PublicKey) | |
| addr := crypto.PubkeyToAddress(privateKey.PublicKey) | |
| log.Print("Generated new account with address ", addr.Hex()) |
| certPEMs := make([][]byte, numClients) | ||
| tlsCerts := make([]tls.Certificate, numClients) | ||
|
|
||
| for i := 0; i < numClients; i++ { |
There was a problem hiding this comment.
Soweit ich es sehe gibt es keinen Grund hier zwei loops zu haben. Stattdessen könnten wir die ganze Logik in eine funktion wie folgende packen, da wir bei der verarbeitung von [i] nichts von [x] x!=i brauchen. Die Funktion hier ruft die neue dann nur in einem loop auf.
// Returns (certPEM, tlsCert, err)
func GenerateSelfSignedCertConfig(commonName string, sans []string) ([]byte, *tls.Config, error)Das würde den code hier leichter lesbar machen weil man sich nicht mehr Gedanken machen muss ob die richtigen indicies verwendet werden.
Wire Identity Authentication
Description
This PR aims to add the ECDSA cryptographic authentication feature to the
wire's implementation of perun-eth-backend. The cryptographic key pair is ECDSA, with a signature length of 65, taking inspiration from the wallet implementation of perun-eth-backend. This implementation must be in sync with the wire interface of go-perun 1.11.Location:
wirepackage