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

Secure Communication Channels via TLS #75

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
22 changes: 16 additions & 6 deletions charts/ephemeral/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ provided while installing the chart. For example,
helm install --name my-release -f values.yaml ephemeral
```

<!-- markdownlint-disable MD013 -->

### Global Parameters

| Parameter | Description | Default |
Expand All @@ -94,19 +96,23 @@ helm install --name my-release -f values.yaml ephemeral
| `discovery.service.annotations` | Annotations that should be attached to the Discovery service | `[]` |
| `discovery.frontendUrl` | The external base URL of the VCP | \`\` |
| `discovery.master.port` | The port of the master discovery service instance | \`\` |
| `discovery.tls.enabled` | Determines whether the service communicates over TLS or plaintext | `false` |
| `discovery.tls.secret` | The name of the k8s secret that holds the TLS keys and certificates | \`\` |
| `discovery.isMaster` | Determines whether the service acts as master or slave | `true` |
| `discovery.slave.connectTimeout` | Timeout to establish the connection to the upstream master Discovery Service | `60s` |
| `discovery.stateTimeout` | Timeout in which the transition to the next state is expected | `60s` |
| `discovery.computationTimeout` | Timeout in which the result of a game's mpc computation is expected | `60s` |

### Network Controller

| Parameter | Description | Default |
| ------------------------------------ | ---------------------------------------------------------------- | ------------------------------------------- |
| `networkController.image.registry` | Image registry used to pull the Network Controller Service image | `ghcr.io` |
| `networkController.image.repository` | Network Controller Image name | `carbynestack/ephemeral/network-controller` |
| `networkController.image.tag` | Network Controller Image tag | `latest` |
| `networkController.image.pullPolicy` | Network Controller Image pull policy | `IfNotPresent` |
| Parameter | Description | Default |
| ------------------------------------ | ------------------------------------------------------------------- | ------------------------------------------- |
| `networkController.image.registry` | Image registry used to pull the Network Controller Service image | `ghcr.io` |
| `networkController.image.repository` | Network Controller Image name | `carbynestack/ephemeral/network-controller` |
| `networkController.image.tag` | Network Controller Image tag | `latest` |
| `networkController.image.pullPolicy` | Network Controller Image pull policy | `IfNotPresent` |
| `networkController.tls.enabled` | Determines whether the service communicates over TLS or plaintext | `false` |
| `networkController.tls.secret` | The name of the k8s secret that holds the TLS keys and certificates | \`\` |

### Ephemeral Service

Expand All @@ -133,6 +139,8 @@ helm install --name my-release -f values.yaml ephemeral
| `ephemeral.discovery.port` | The port of the discovery service | `8080` |
| `ephemeral.discovery.connectTimout` | Timeout to establish the connection to the discovery service | `60s` |
| `ephemeral.frontendUrl` | The external base URL of the VCP | \`\` |
| `ephemeral.tls.enabled` | Determines whether the service communicates over TLS or plaintext | `false` |
| `ephemeral.tls.secret` | The name of the k8s secret that holds the TLS keys and certificates | \`\` |
| `ephemeral.spdz.prime` | The prime used by SPDZ | \`\` |
| `ephemeral.spdz.rInv` | The rInv used by SPDZ | \`\` |
| `ephemeral.spdz.gfpMacKey` | The macKey for the prime protocol used by SPDZ | \`\` |
Expand All @@ -144,3 +152,5 @@ helm install --name my-release -f values.yaml ephemeral
| `ephemeral.networkEstablishTimeout` | Timeout to establish network connections | `1m` |
| `ephemeral.player.stateTimeout` | Timeout in which the transition to the next state is expected | `60s` |
| `ephemeral.player.computationTimeout` | Timeout in which the result of a game's mpc computation is expected | `60s` |

<!-- markdownlint-enable MD013 -->
20 changes: 20 additions & 0 deletions charts/ephemeral/templates/discovery.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Please update Copyright Header

Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,16 @@ spec:
volumeMounts:
- name: config-volume
mountPath: /etc/config
- name: tls-secret-volume
mountPath: /etc/tls
readOnly: true
volumes:
- name: config-volume
configMap:
name: discovery-config
- name: tls-secret-volume
secret:
secretName: {{ .Values.discovery.tls.secret }}
serviceAccountName: discovery
---
kind: Service
Expand Down Expand Up @@ -74,6 +80,7 @@ data:
"frontendURL": "{{ .Values.discovery.frontendUrl }}",
"masterHost": "{{ .Values.discovery.master.host }}",
"masterPort": "{{ .Values.discovery.master.port }}",
"tlsEnabled": {{ .Values.discovery.tls.enabled }},
"slave": {{ if .Values.discovery.isMaster }}false{{ else }}true{{ end }},
"playerCount": {{ .Values.playerCount }},
"stateTimeout": "{{ .Values.discovery.stateTimeout }}",
Expand All @@ -89,12 +96,25 @@ spec:
selector:
istio: ingressgateway # use istio default controller
servers:
{{- if .Values.discovery.tls.enabled }}
- port:
number: 31400
name: grpc-my
Copy link
Member

Choose a reason for hiding this comment

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

hmm I know it was named grpcy-my before, but eventually rename to sth like cs-grpc here and below

protocol: HTTPS
tls:
mode: MUTUAL # enables mTLS on the Gateway
credentialName: {{ .Values.discovery.tls.secret }} # the name of the Secret that holds the TLS certs and CA certificate
hosts:
- "*"
{{- else }}
- port:
number: 31400
name: grpc-my
protocol: GRPC
hosts:
- "*"
{{- end }}

---
apiVersion: networking.istio.io/v1beta1
kind: VirtualService
Expand Down
7 changes: 7 additions & 0 deletions charts/ephemeral/templates/ephemeral.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Please update Header

Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ spec:
volumeMounts:
- name: config-volume
mountPath: /etc/config
- name: tls-secret-volume
mountPath: /etc/tls
readOnly: true
{{- if or .Values.ephemeral.resources.requests.memory .Values.ephemeral.resources.requests.cpu .Values.ephemeral.resources.limits.memory .Values.ephemeral.resources.limits.cpu }}
resources:
{{- if or .Values.ephemeral.resources.requests.memory .Values.ephemeral.resources.requests.cpu }}
Expand All @@ -63,6 +66,9 @@ spec:
- name: config-volume
configMap:
name: {{ include "ephemeral.fullname" . }}-config1
- name: tls-secret-volume
secret:
secretName: {{ .Values.ephemeral.tls.secret }}
serviceAccountName: knative-serving
---
apiVersion: v1
Expand Down Expand Up @@ -93,6 +99,7 @@ data:
"tupleStock": {{ .Values.ephemeral.castor.tupleStock }}
},
"frontendURL": "{{ .Values.ephemeral.frontendUrl }}",
"tlsEnabled": {{ .Values.ephemeral.tls.enabled }},
"discoveryConfig": {
"host": "{{ .Values.ephemeral.discovery.host }}",
"port": "{{ .Values.ephemeral.discovery.port }}",
Expand Down
20 changes: 20 additions & 0 deletions charts/ephemeral/templates/network-controller.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Please update Copyright Header

Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,23 @@ spec:
fieldPath: metadata.name
- name: OPERATOR_NAME
value: "network-controller"
volumeMounts:
- name: config-volume
mountPath: /etc/config
volumes:
- name: config-volume
configMap:
name: {{ include "ephemeral.fullname" . }}-network-controller-config
serviceAccountName: network-controller

---
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ include "ephemeral.fullname" . }}-network-controller-config
data:
config.json: |-
{
"tlsEnabled": {{ .Values.networkController.tls.enabled }},
"tlsSecret": "{{ .Values.networkController.tls.secret }}"
}
9 changes: 9 additions & 0 deletions charts/ephemeral/values.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Please update Copyright Header

Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ discovery:
computationTimeout : "600s"
slave:
connectTimeout: "60s"
tls:
Copy link
Member

Choose a reason for hiding this comment

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

that's configured 3 times. Eventually make it "global" like playerCount or is there any scenario where 'secret' is different or only a subset of the services will use tls?

enabled: false
secret:

ephemeral:
service:
Expand Down Expand Up @@ -56,6 +59,9 @@ ephemeral:
path: "/"
tupleStock: 1000
frontendUrl:
tls:
enabled: false
secret:
discovery:
host: discovery.default.svc.cluster.local
port: 8080
Expand All @@ -81,3 +87,6 @@ networkController:
tag: latest
pullPolicy: "IfNotPresent"
pullSecrets: []
tls:
enabled: false
secret:
22 changes: 19 additions & 3 deletions cmd/discovery/main.go
Copy link
Member

Choose a reason for hiding this comment

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

Please update Copyright Header

Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ package main

import (
"context"
"crypto/tls"
"encoding/json"
"errors"
"fmt"
"time"

"github.com/carbynestack/ephemeral/pkg/discovery"
c "github.com/carbynestack/ephemeral/pkg/discovery/transport/client"
cl "github.com/carbynestack/ephemeral/pkg/discovery/transport/client"
Expand All @@ -21,7 +24,6 @@ import (
"github.com/carbynestack/ephemeral/pkg/utils"
mb "github.com/vardius/message-bus"
"go.uber.org/zap"
"time"
)

const (
Expand All @@ -32,6 +34,7 @@ const (
// DefaultPortRange is the range of ports used for MCP communication between the players.
DefaultPortRange = "30000:30100"
defaultConfigLocation = "/etc/config/config.json"
defaultTlsConfig = "/etc/tls"
)

func main() {
Expand All @@ -55,6 +58,16 @@ func main() {
if err != nil {
panic(err)
}

var tlsConfig *tls.Config
if config.TlsEnabled {
var err error
tlsConfig, err = utils.CreateTLSConfig(defaultTlsConfig)
if err != nil {
panic(err)
}
}

var upstreamConfig *DiscoveryClientTypedConfig
if config.Slave {
upstreamConfig = &DiscoveryClientTypedConfig{
Expand All @@ -63,7 +76,7 @@ func main() {
ConnectTimeout: config.ConnectTimeout,
}
}
client, mode, err := NewClient(upstreamConfig, logger, errCh)
client, mode, err := NewClient(upstreamConfig, tlsConfig, logger, errCh)
if err != nil {
panic(err)
}
Expand All @@ -86,7 +99,7 @@ func main() {
// NewClient returns a new client with parameters specific to the server mode. If upstreamClient is defined, the client
// will be configured to forward incoming events to an upstream master server. With upstreamClient set to nil, the
// service is considered to be the master service.
func NewClient(upstreamConfig *types.DiscoveryClientTypedConfig, logger *zap.SugaredLogger, errCh chan error) (*cl.Client, string, error) {
func NewClient(upstreamConfig *types.DiscoveryClientTypedConfig, tlsConfig *tls.Config, logger *zap.SugaredLogger, errCh chan error) (*cl.Client, string, error) {
logger.Debug("Creating new discovery client")
mode := ModeMaster
client := &cl.Client{}
Expand All @@ -105,6 +118,7 @@ func NewClient(upstreamConfig *types.DiscoveryClientTypedConfig, logger *zap.Sug
ConnectTimeout: upstreamConfig.ConnectTimeout,
Logger: logger,
Context: context.Background(),
TlsConfig: tlsConfig,
}
client, err = c.NewClient(grpcClientConf)
if err != nil {
Expand Down Expand Up @@ -181,10 +195,12 @@ func ParseConfig(path string) (*DiscoveryTypedConfig, error) {
if err != nil {
return nil, errors.New(fmt.Sprintf("invalid connection timeout format: %v", err))
}

return &DiscoveryTypedConfig{
FrontendURL: conf.FrontendURL,
MasterHost: conf.MasterHost,
MasterPort: conf.MasterPort,
TlsEnabled: conf.TlsEnabled,
Slave: conf.Slave,
StateTimeout: stateTimeout,
ComputationTimeout: computationTimeout,
Expand Down
2 changes: 1 addition & 1 deletion cmd/discovery/main_test.go
Copy link
Member

Choose a reason for hiding this comment

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

Please update Copyright Header

Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var _ = Describe("Main", func() {
}
logger := zap.NewNop().Sugar()
errCh := make(chan error)
cl, mode, err := NewClient(conf, logger, errCh)
cl, mode, err := NewClient(conf, nil, logger, errCh)
Copy link
Member

Choose a reason for hiding this comment

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

does it make sense to add a test case with a tls config given?

Expect(err).NotTo(HaveOccurred())
Expect(mode).To(Equal(ModeSlave))
Expect(cl).NotTo(BeNil())
Expand Down
21 changes: 18 additions & 3 deletions cmd/ephemeral/main.go
Copy link
Member

Choose a reason for hiding this comment

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

Please update Copyright Header

Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,30 @@
package main

import (
"crypto/tls"
"encoding/json"
"errors"

"github.com/carbynestack/ephemeral/pkg/amphora"
"github.com/carbynestack/ephemeral/pkg/castor"
. "github.com/carbynestack/ephemeral/pkg/ephemeral"
l "github.com/carbynestack/ephemeral/pkg/logger"
"github.com/carbynestack/ephemeral/pkg/utils"

. "github.com/carbynestack/ephemeral/pkg/types"
"math/big"
"net/http"
"net/url"
"time"

. "github.com/carbynestack/ephemeral/pkg/types"

"go.uber.org/zap"
)

const (
defaultConfig = "/etc/config/config.json"
defaultPort = "8080"
defaultConfig = "/etc/config/config.json"
defaultTlsConfig = "/etc/tls"
defaultPort = "8080"
)

func main() {
Expand Down Expand Up @@ -144,6 +148,15 @@ func InitTypedConfig(conf *SPDZEngineConfig) (*SPDZEngineTypedConfig, error) {
return nil, err
}

var tlsConfig *tls.Config
if conf.TlsEnabled {
var err error
tlsConfig, err = utils.CreateTLSConfig(defaultTlsConfig)
if err != nil {
return nil, err
}
}

return &SPDZEngineTypedConfig{
NetworkEstablishTimeout: networkEstablishTimeout,
RetrySleep: retrySleep,
Expand All @@ -168,5 +181,7 @@ func InitTypedConfig(conf *SPDZEngineConfig) (*SPDZEngineTypedConfig, error) {
},
StateTimeout: stateTimeout,
ComputationTimeout: computationTimeout,
TlsEnabled: conf.TlsEnabled,
TlsConfig: tlsConfig,
}, nil
}
Loading
Loading