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.

Sign up for GitHub

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

initial POC for grpc tls #3917

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

initial POC for grpc tls #3917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions nodebuilder/core/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ var MetricsEnabled bool

// Config combines all configuration fields for managing the relationship with a Core node.
type Config struct {
IP string
RPCPort string
GRPCPort string
IP string
RPCPort string
GRPCPort string
EnableTLS bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's avoid growing the node's config. We have an env variable that signals that TLS should be enabled.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wanted to create a POC. My recommended approach would be to have a RPC url and gRPC url.

}

// DefaultConfig returns default configuration for managing the
Expand Down
17 changes: 14 additions & 3 deletions nodebuilder/core/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import (
)

var (
coreFlag = "core.ip"
coreRPCFlag = "core.rpc.port"
coreGRPCFlag = "core.grpc.port"
coreFlag = "core.ip"
coreRPCFlag = "core.rpc.port"
coreGRPCFlag = "core.grpc.port"
coreEnableTLSFlag = "core.grpc.tls"
)

// Flags gives a set of hardcoded Core flags.
Expand All @@ -34,6 +35,11 @@ func Flags() *flag.FlagSet {
DefaultGRPCPort,
"Set a custom gRPC port for the core node connection. The --core.ip flag must also be provided.",
)
flags.Bool(
coreEnableTLSFlag,
false,
"Enables grpc TLS. The --core.ip flag must also be provided.",
)
return flags
}

Expand All @@ -60,6 +66,11 @@ func ParseFlags(
cfg.GRPCPort = grpc
}

if cmd.Flag(coreEnableTLSFlag).Changed {
enabled := cmd.Flag(coreEnableTLSFlag).Value.String() == "true"
cfg.EnableTLS = enabled
}

cfg.IP = coreIP
return cfg.Validate()
}
3 changes: 1 addition & 2 deletions nodebuilder/state/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ func coreAccessor(
*modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader],
error,
) {
ca, err := state.NewCoreAccessor(keyring, string(keyname), sync, corecfg.IP, corecfg.GRPCPort,
network.String(), opts...)
ca, err := state.NewCoreAccessor(keyring, string(keyname), sync, corecfg.IP, corecfg.GRPCPort, corecfg.EnableTLS, network.String(), opts...)

sBreaker := &modfraud.ServiceBreaker[*state.CoreAccessor, *header.ExtendedHeader]{
Service: ca,
Expand Down
43 changes: 22 additions & 21 deletions state/core_access.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,18 @@

import (
"context"
"crypto/tls"
"errors"
"fmt"
"sync"
"time"

"github.com/celestiaorg/celestia-app/v3/app"
"github.com/celestiaorg/celestia-app/v3/app/encoding"
apperrors "github.com/celestiaorg/celestia-app/v3/app/errors"
"github.com/celestiaorg/celestia-app/v3/pkg/user"
libhead "github.com/celestiaorg/go-header"
libshare "github.com/celestiaorg/go-square/v2/share"
nodeservice "github.com/cosmos/cosmos-sdk/client/grpc/node"
"github.com/cosmos/cosmos-sdk/client/grpc/tmservice"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
Expand All @@ -20,15 +27,9 @@
"github.com/tendermint/tendermint/proto/tendermint/crypto"
"google.golang.org/grpc"
"google.golang.org/grpc/connectivity"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"

"github.com/celestiaorg/celestia-app/v3/app"
"github.com/celestiaorg/celestia-app/v3/app/encoding"
apperrors "github.com/celestiaorg/celestia-app/v3/app/errors"
"github.com/celestiaorg/celestia-app/v3/pkg/user"
libhead "github.com/celestiaorg/go-header"
libshare "github.com/celestiaorg/go-square/v2/share"

"github.com/celestiaorg/celestia-node/header"
)

Expand Down Expand Up @@ -67,10 +68,11 @@

prt *merkle.ProofRuntime

coreConn *grpc.ClientConn
coreIP string
grpcPort string
network string
coreConn *grpc.ClientConn
coreIP string
grpcPort string
enableTLS bool
network string

// these fields are mutatable and thus need to be protected by a mutex
lock sync.Mutex
Expand All @@ -86,15 +88,7 @@
// NewCoreAccessor dials the given celestia-core endpoint and
// constructs and returns a new CoreAccessor (state service) with the active
// connection.
func NewCoreAccessor(
keyring keyring.Keyring,
keyname string,
getter libhead.Head[*header.ExtendedHeader],
coreIP,
grpcPort string,
network string,
options ...Option,
) (*CoreAccessor, error) {
func NewCoreAccessor(keyring keyring.Keyring, keyname string, getter libhead.Head[*header.ExtendedHeader], coreIP, grpcPort string, enableTLS bool, network string, options ...Option) (*CoreAccessor, error) {
// create verifier
prt := merkle.DefaultProofRuntime()
prt.RegisterOpDecoder(storetypes.ProofOpIAVLCommitment, storetypes.CommitmentOpDecoder)
Expand All @@ -106,6 +100,7 @@
getter: getter,
coreIP: coreIP,
grpcPort: grpcPort,
enableTLS: enableTLS,
prt: prt,
network: network,
}
Expand All @@ -124,9 +119,15 @@

// dial given celestia-core endpoint
endpoint := fmt.Sprintf("%s:%s", ca.coreIP, ca.grpcPort)
grpcOpts := make([]grpc.DialOption, 0)
if ca.enableTLS {
grpcOpts = append(grpcOpts, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{})))

Check failure on line 124 in state/core_access.go

View workflow job for this annotation

GitHub Actions / go-ci / Lint 

G402: TLS MinVersion too low. (gosec)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without specifying valid certificates in the tls.Config object will not secure gRPC connection. By default, an empty tls.Config{} means that no certificate or private key is provided, which leads to an insecure setup.

Node already has TLS certificates handling logic here for web socket, so you can reuse the certificate for the grpc connection.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a proof of concept to get the Light node working against a Quicknode celestia endpoint. QN doesn't support client certs. gRPC auth for a QN endpoint occurs over the x-token header

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more point here is the solution is not generic. You may not support the certificates but if someone does, the connection won't be established. Also, it is recommended to manually specify the MinTLSVersion(linter confirmed it)

} else {
grpcOpts = append(grpcOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
client, err := grpc.NewClient(
endpoint,
grpc.WithTransportCredentials(insecure.NewCredentials()),
grpcOpts...,
)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion state/core_access_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ func buildAccessor(t *testing.T) (*CoreAccessor, []string) {
WithAppCreator(appCreator) // needed until https://github.com/celestiaorg/celestia-app/pull/3680 merges
cctx, _, grpcAddr := testnode.NewNetwork(t, config)

ca, err := NewCoreAccessor(cctx.Keyring, accounts[0].Name, nil, "127.0.0.1", extractPort(grpcAddr), chainID)
ca, err := NewCoreAccessor(cctx.Keyring, accounts[0].Name, nil, "127.0.0.1", extractPort(grpcAddr), false, chainID)
require.NoError(t, err)
return ca, getNames(accounts)
}
Expand Down
2 changes: 1 addition & 1 deletion state/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
s.Require().Greater(len(s.accounts), 0)
accountName := s.accounts[0].Name

accessor, err := NewCoreAccessor(s.cctx.Keyring, accountName, localHeader{s.cctx.Client}, "", "", "")
accessor, err := NewCoreAccessor(s.cctx.Keyring, accountName, localHeader{s.cctx.Client}, "", "", false, "")
require.NoError(s.T(), err)
setClients(accessor, s.cctx.GRPCClient)
s.accessor = accessor
Expand Down
Loading