Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions helm-chart/kuberay-operator/templates/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@ metadata:
labels:
{{- include "kuberay-operator.labels" . | nindent 4 }}
{{ include "role.consistentRules" (dict "batchSchedulerEnabled" .Values.batchScheduler.enabled "batchSchedulerName" .Values.batchScheduler.name) }}
- apiGroups:
- config.openshift.io
resources:
- apiservers
verbs:
- get
Comment thread
cursor[bot] marked this conversation as resolved.
{{- end }}
1 change: 1 addition & 0 deletions ray-operator/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ COPY main.go main.go
COPY apis/ apis/
COPY controllers/ controllers/
COPY pkg/features pkg/features
COPY pkg/tls pkg/tls
COPY pkg/utils pkg/utils
COPY pkg/webhooks pkg/webhooks
COPY rayjob-submitter/ rayjob-submitter/
Expand Down
6 changes: 6 additions & 0 deletions ray-operator/config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ rules:
- get
- list
- watch
- apiGroups:
- config.openshift.io
resources:
- apiservers
verbs:
- get
- apiGroups:
- coordination.k8s.io
resources:
Expand Down
1 change: 1 addition & 0 deletions ray-operator/controllers/ray/raycluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ type RayClusterReconcilerOptions struct {
// +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;create;update
// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingressclasses,verbs=get;list;watch
// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch;create;update;delete;patch
// +kubebuilder:rbac:groups=config.openshift.io,resources=apiservers,verbs=get;list;watch
// +kubebuilder:rbac:groups=route.openshift.io,resources=routes,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=extensions,resources=ingresses,verbs=get;list;watch;create;update;delete;patch
// +kubebuilder:rbac:groups=core,resources=serviceaccounts,verbs=get;list;watch;create;delete
Expand Down
13 changes: 13 additions & 0 deletions ray-operator/main.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"context"
"flag"
"fmt"
"os"
Expand All @@ -27,6 +28,7 @@ import (
k8szap "sigs.k8s.io/controller-runtime/pkg/log/zap"
ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
"sigs.k8s.io/controller-runtime/pkg/webhook"
gwv1 "sigs.k8s.io/gateway-api/apis/v1"

configapi "github.com/ray-project/kuberay/ray-operator/apis/config/v1alpha1"
Expand All @@ -37,6 +39,7 @@ import (
"github.com/ray-project/kuberay/ray-operator/controllers/ray/utils"
"github.com/ray-project/kuberay/ray-operator/internal/managercache"
"github.com/ray-project/kuberay/ray-operator/pkg/features"
pkgtls "github.com/ray-project/kuberay/ray-operator/pkg/tls"
webhooks "github.com/ray-project/kuberay/ray-operator/pkg/webhooks/v1"
)

Expand Down Expand Up @@ -269,6 +272,16 @@ func main() {
restConfig.QPS = float32(*config.QPS)
restConfig.Burst = *config.Burst

tlsResult, err := pkgtls.Resolve(context.Background(), restConfig)
Comment thread
cursor[bot] marked this conversation as resolved.
if err != nil {
setupLog.Error(err, "unable to resolve TLS configuration")
os.Exit(1)
}
options.Metrics.TLSOpts = tlsResult.TLSOpts
Comment thread
cursor[bot] marked this conversation as resolved.
options.WebhookServer = webhook.NewServer(webhook.Options{
TLSOpts: tlsResult.TLSOpts,
})

// Check if cert-manager API is available before registering the mTLS controller.
// If cert-manager is not installed, the controller's Certificate/Issuer cache would
// never sync and the manager would fail to start (e.g. in E2E environments without cert-manager).
Expand Down
157 changes: 157 additions & 0 deletions ray-operator/pkg/tls/tls.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package tls

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

configv1 "github.com/openshift/api/config/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
)

var log = ctrl.Log.WithName("tls")

var openSSLToGoCipher = map[string]uint16{
"TLS_AES_128_GCM_SHA256": tls.TLS_AES_128_GCM_SHA256,
"TLS_AES_256_GCM_SHA384": tls.TLS_AES_256_GCM_SHA384,
"TLS_CHACHA20_POLY1305_SHA256": tls.TLS_CHACHA20_POLY1305_SHA256,
"ECDHE-ECDSA-AES128-GCM-SHA256": tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
"ECDHE-RSA-AES128-GCM-SHA256": tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
"ECDHE-ECDSA-AES256-GCM-SHA384": tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
"ECDHE-RSA-AES256-GCM-SHA384": tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
"ECDHE-ECDSA-CHACHA20-POLY1305-SHA256": tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
"ECDHE-RSA-CHACHA20-POLY1305-SHA256": tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,
"ECDHE-ECDSA-CHACHA20-POLY1305": tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
"ECDHE-RSA-CHACHA20-POLY1305": tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Custom cipher map is incomplete

Medium Severity

openSSLToGoCipher only maps GCM and ChaCha20 ECDHE suites. Go-supported suites that still appear in OpenShift custom or Old profiles (for example CBC SHA and RSA GCM names) are dropped. A custom profile can therefore lose most of its intended ciphers, or fail startup if every listed cipher is treated as unsupported.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 79e5ef9. Configure here.


var intermediateCiphers = []uint16{
tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,
}

var tlsVersionMap = map[configv1.TLSProtocolVersion]uint16{
"VersionTLS10": tls.VersionTLS10,
"VersionTLS11": tls.VersionTLS11,
"VersionTLS12": tls.VersionTLS12,
"VersionTLS13": tls.VersionTLS13,
}

// Result holds the resolved TLS configuration.
type Result struct {
TLSOpts []func(*tls.Config)
}

// Resolve reads the cluster TLS profile from apiservers.config.openshift.io/cluster
// and returns TLS option functions for controller-runtime servers.
// On non-OpenShift clusters, it returns hardened Intermediate defaults.
// Returns an error only on unexpected failures that should prevent startup.
func Resolve(ctx context.Context, cfg *rest.Config) (Result, error) {
var result Result

scheme := runtime.NewScheme()
if err := configv1.Install(scheme); err != nil {
return result, fmt.Errorf("installing OpenShift config scheme: %w", err)
}

k8sClient, err := client.New(cfg, client.Options{Scheme: scheme})
if err != nil {
return result, fmt.Errorf("creating bootstrap client for TLS profile: %w", err)
}

apiServer := &configv1.APIServer{}
if err := k8sClient.Get(ctx, client.ObjectKey{Name: "cluster"}, apiServer); err != nil {
switch {
case meta.IsNoMatchError(err):
log.Info("TLS profile not available, using hardened defaults (non-OpenShift cluster)")
case apierrors.IsNotFound(err):
log.Info("APIServer resource not found, using hardened defaults")
case apierrors.IsServiceUnavailable(err),
apierrors.IsTimeout(err),
apierrors.IsServerTimeout(err),
apierrors.IsTooManyRequests(err),
errors.Is(err, context.DeadlineExceeded):
log.Info("Transient API error, using Intermediate defaults", "error", err)
default:
return result, fmt.Errorf("reading APIServer TLS profile: %w", err)
}
result.TLSOpts = append(result.TLSOpts, intermediateWithALPN)
return result, nil
}

minVersion, ciphers := parseProfile(apiServer.Spec.TLSSecurityProfile)
if ciphers != nil && len(ciphers) == 0 {
return result, fmt.Errorf("custom TLS profile specified ciphers but none are supported by Go")
}

result.TLSOpts = append(result.TLSOpts, func(c *tls.Config) {
c.MinVersion = minVersion
if len(ciphers) > 0 {
c.CipherSuites = ciphers
}
c.NextProtos = []string{"h2", "http/1.1"}
})
return result, nil
}

func intermediateWithALPN(c *tls.Config) {
c.MinVersion = tls.VersionTLS12
c.CipherSuites = intermediateCiphers
c.NextProtos = []string{"h2", "http/1.1"}
}

func parseProfile(profile *configv1.TLSSecurityProfile) (uint16, []uint16) {
if profile == nil {
return tls.VersionTLS12, intermediateCiphers
}

switch profile.Type {
case configv1.TLSProfileIntermediateType, "":
return tls.VersionTLS12, intermediateCiphers
case configv1.TLSProfileModernType:
return tls.VersionTLS13, nil
case configv1.TLSProfileOldType:
return tls.VersionTLS10, nil
case configv1.TLSProfileCustomType:
if profile.Custom == nil {
log.Info("Custom TLS profile type specified but custom block is nil, falling back to Intermediate")
return tls.VersionTLS12, intermediateCiphers
}
return parseCustomProfile(profile.Custom)
default:
log.Info("Unknown TLS profile type, falling back to Intermediate", "type", profile.Type)
return tls.VersionTLS12, intermediateCiphers
}
}

func parseCustomProfile(custom *configv1.CustomTLSProfile) (uint16, []uint16) {
minVersion, ok := tlsVersionMap[custom.MinTLSVersion]
if !ok {
log.Info("Unknown minTLSVersion in custom profile, defaulting to TLS 1.2", "minTLSVersion", custom.MinTLSVersion)
minVersion = tls.VersionTLS12
}

if len(custom.Ciphers) == 0 {
return minVersion, nil
}

ciphers := make([]uint16, 0, len(custom.Ciphers))
for _, name := range custom.Ciphers {
if id, ok := openSSLToGoCipher[name]; ok {
ciphers = append(ciphers, id)
} else {
log.Info("Dropping unsupported cipher from custom TLS profile", "cipher", name)
}
}
return minVersion, ciphers
}
134 changes: 134 additions & 0 deletions ray-operator/pkg/tls/tls_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package tls

import (
"crypto/tls"
"testing"

configv1 "github.com/openshift/api/config/v1"
)

func TestParseProfile(t *testing.T) {
tests := []struct {
profile *configv1.TLSSecurityProfile
name string
wantCiphers []uint16
wantMinVersion uint16
}{
{
name: "nil profile returns Intermediate defaults",
profile: nil,
wantMinVersion: tls.VersionTLS12,
wantCiphers: intermediateCiphers,
},
{
name: "empty profile returns Intermediate defaults",
profile: &configv1.TLSSecurityProfile{},
wantMinVersion: tls.VersionTLS12,
wantCiphers: intermediateCiphers,
},
{
name: "Intermediate type",
profile: &configv1.TLSSecurityProfile{
Type: configv1.TLSProfileIntermediateType,
},
wantMinVersion: tls.VersionTLS12,
wantCiphers: intermediateCiphers,
},
{
name: "Modern returns TLS 1.3 with nil ciphers",
profile: &configv1.TLSSecurityProfile{
Type: configv1.TLSProfileModernType,
},
wantMinVersion: tls.VersionTLS13,
wantCiphers: nil,
},
{
name: "Old returns TLS 1.0 with nil ciphers",
profile: &configv1.TLSSecurityProfile{
Type: configv1.TLSProfileOldType,
},
wantMinVersion: tls.VersionTLS10,
wantCiphers: nil,
},
{
name: "Custom with valid ciphers",
profile: &configv1.TLSSecurityProfile{
Type: configv1.TLSProfileCustomType,
Custom: &configv1.CustomTLSProfile{
TLSProfileSpec: configv1.TLSProfileSpec{
MinTLSVersion: "VersionTLS12",
Ciphers: []string{"ECDHE-ECDSA-AES128-GCM-SHA256", "ECDHE-RSA-AES256-GCM-SHA384"},
},
},
},
wantMinVersion: tls.VersionTLS12,
wantCiphers: []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384},
},
{
name: "Custom with unsupported cipher skips it",
profile: &configv1.TLSSecurityProfile{
Type: configv1.TLSProfileCustomType,
Custom: &configv1.CustomTLSProfile{
TLSProfileSpec: configv1.TLSProfileSpec{
MinTLSVersion: "VersionTLS12",
Ciphers: []string{"ECDHE-ECDSA-AES128-GCM-SHA256", "UNSUPPORTED-CIPHER"},
},
},
},
wantMinVersion: tls.VersionTLS12,
wantCiphers: []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
},
{
name: "Custom with all unsupported ciphers returns empty slice",
profile: &configv1.TLSSecurityProfile{
Type: configv1.TLSProfileCustomType,
Custom: &configv1.CustomTLSProfile{
TLSProfileSpec: configv1.TLSProfileSpec{
MinTLSVersion: "VersionTLS12",
Ciphers: []string{"DHE-RSA-AES128-GCM-SHA256"},
},
},
},
wantMinVersion: tls.VersionTLS12,
wantCiphers: []uint16{},
},
{
name: "Unknown type falls back to Intermediate",
profile: &configv1.TLSSecurityProfile{
Type: "SuperSecure",
},
wantMinVersion: tls.VersionTLS12,
wantCiphers: intermediateCiphers,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotMinVersion, gotCiphers := parseProfile(tt.profile)

if gotMinVersion != tt.wantMinVersion {
t.Errorf("parseProfile() minVersion = %d, want %d", gotMinVersion, tt.wantMinVersion)
}

if tt.wantCiphers == nil {
if gotCiphers != nil {
t.Errorf("parseProfile() ciphers = %v, want nil", gotCiphers)
}
return
}

if gotCiphers == nil {
t.Fatal("expected non-nil empty slice, got nil")
}
if len(gotCiphers) != len(tt.wantCiphers) {
t.Errorf("parseProfile() ciphers length = %d, want %d", len(gotCiphers), len(tt.wantCiphers))
return
}
for i, c := range gotCiphers {
if c != tt.wantCiphers[i] {
t.Errorf("parseProfile() ciphers[%d] = %d, want %d", i, c, tt.wantCiphers[i])
}
}
})
}
}
Loading