TLS x509 for Websocket #145
Conversation
go.mod
Outdated
There was a problem hiding this comment.
Can we run go mod tidy and include go.mod and go.sum afterwards?
| case libp2pcrypto.Ed25519: | ||
| return ed25519.PrivateKey(rawKey), nil | ||
| default: | ||
| return nil, fmt.Errorf("unsupported key type for X.509 conversion") |
There was a problem hiding this comment.
Can be useful to know which key it was:
| return nil, fmt.Errorf("unsupported key type for X.509 conversion") | |
| return nil, fmt.Errorf("unsupported key type for X.509 conversion: %s", privKey.Type()) |
| func publicKey(priv crypto.PrivateKey) crypto.PublicKey { | ||
| switch key := priv.(type) { | ||
| case *rsa.PrivateKey: | ||
| return &key.PublicKey | ||
| case *ecdsa.PrivateKey: | ||
| return &key.PublicKey | ||
| case ed25519.PrivateKey: | ||
| return key.Public().(ed25519.PublicKey) | ||
| default: | ||
| panic("unsupported key type") | ||
| } |
There was a problem hiding this comment.
I would prefer to have this return crypto.PublicKey and an error than have this panic.
Instead in default I'd do return nil, fmt.Errorf("unsupported key type: %T", key)
| Organization: []string{"b7s"}, | ||
| }, | ||
| NotBefore: time.Now(), | ||
| NotAfter: time.Now().Add(365 * 24 * time.Hour), // 1 year validity |
There was a problem hiding this comment.
Perhaps for visibility we define this in params.go:
var (
DefaultCertificateDuration = 365 * 24 * time.Hour // 1 year
)Then here we do:
| NotAfter: time.Now().Add(365 * 24 * time.Hour), // 1 year validity | |
| NotAfter: time.Now().Add(DefaultCertificateDuration), |
I'd do the same for Organization.
| // Create the certificate | ||
| derBytes, err := x509.CreateCertificate(rand.Reader, template, template, pubKey, privKey) | ||
| if err != nil { | ||
| return tls.Certificate{}, err |
There was a problem hiding this comment.
Can we wrap the error here? Especially if we change the above to return public key and an error - we'd have two paths that return an error - it's useful to have context
| libp2pcrypto "github.com/libp2p/go-libp2p/core/crypto" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
There was a problem hiding this comment.
I'd change these asserts to requires. That way it's consistent with the codebase and it'll rarely be useful to continue the test after the first error.
There was a problem hiding this comment.
Would be good to have tests for other types of keys too - but I can add that in a separate PR.
| addresses = append(addresses, wsAddr) | ||
| } | ||
| // define a subset of the default transports, so that we can offer a x509 certificate for the websocket transport | ||
| DefaultTransports := libp2p.ChainOptions( |
There was a problem hiding this comment.
| DefaultTransports := libp2p.ChainOptions( | |
| transports := libp2p.ChainOptions( |
| } | ||
|
|
||
| // Convert libp2p private key to crypto.PrivateKey | ||
| cryptoPrivKey, err := convertLibp2pPrivKeyToCryptoPrivKey(key) |
There was a problem hiding this comment.
This here line relies on us having a key. However the key is optional - we could generate a libp2p identity on the fly.
If we start the node with node --websocket true (without --private-key) - this will blow up.
What I'm thinking:
- We change how private key config works for the host. Instead of this:
// WithPrivateKey specifies the private key for the Host.
func WithPrivateKey(filepath string) func(*Config) we do:
// WithPrivateKey specifies the private key for the Host.
func WithPrivateKey(key crypto.PrivKey) func(*Config)- We make the key a hard requirement -
host.Newfails if it does not have a key. - In the
main()function of the node, we take care of a) reading the key from disk - if the private key is specified, or b) we generate an in-memory key that we will use for the current invocation. In either case we pass the key to thehost. Then we can rely on having the key.
What do you think?
Signed-off-by: Derek Anderson <[email protected]> add x509 generation Signed-off-by: Derek Anderson <[email protected]> add tests Signed-off-by: Derek Anderson <[email protected]> add tls config with cert Signed-off-by: Derek Anderson <[email protected]> x509 generation from libp2p cert, TLS upgrade for WS transport. Signed-off-by: Derek Anderson <[email protected]> cleanup Signed-off-by: Derek Anderson <[email protected]> put back in trap comment Signed-off-by: Derek Anderson <[email protected]> load the private key, and then generate the x509 Signed-off-by: Derek Anderson <[email protected]> update makefile for platform detection Signed-off-by: Derek Anderson <[email protected]> update make file to run node Signed-off-by: Derek Anderson <[email protected]> update certificate creation for ed25519, update makefile Signed-off-by: Derek Anderson <[email protected]> configure org name Signed-off-by: Derek Anderson <[email protected]>
Signed-off-by: Derek Anderson <[email protected]>
Signed-off-by: Derek Anderson <[email protected]>
This draft introduces converting an IPFS private key into a valid x509, and then using that to secure the WebSocket Connection with a TLS 1.2+ upgrade.