Skip to content

Commit eb90b39

Browse files
committed
Add webhook validation for PTP secrets and context-based merge logic
Add validateSecretExists() to verify ptpSecretName references exist Add validateSppInSecret() to validate SPP numbers against secret data Implement context-based merge to distinguish controller sources PtpConfig controller: use security volumes from updated (supports deletion) PtpOperatorConfig controller: preserve security volumes from current Update merge logic for volumes, annotations, and volume mounts Add controller source constants to apply package Update all tests to pass context parameter Add PTP authentication testing support with conditional enablement Add configure-switch-ptp-security.sh script to configure external switch Add ptp-security.yaml template with security associations (SPP 0 and 1) Add GetPtp4lConfigWithAuth() to conditionally inject auth settings Add PtpSecretName field conditionally based on PTP_AUTH_ENABLED Add negative test for SPP mismatch validation (2-minute check) Update run-ci-github.sh to configure switch and run auth-enabled tests Apply auth settings to all config creation functions (GM, slave, BC, etc) Add PTP authentication security tests for attack scenarios Add rogue client injection test: verifies unauthenticated clients are blocked Add MITM protection test: verifies tampered messages with wrong keys are dropped Add replay attack test: verifies seqid_window is configured for anti-replay Implement robust cleanup with delete and recreate for test-slave1 Add pod stabilization waits between tests to prevent race conditions Include optional log validation for authentication failure messages
1 parent 392b39a commit eb90b39

20 files changed

+1828
-64
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ ENVTEST ?= $(LOCALBIN)/setup-envtest
155155

156156
## Tool Versions
157157
KUSTOMIZE_VERSION ?= v4.5.7
158-
CONTROLLER_TOOLS_VERSION ?= v0.15.0
158+
CONTROLLER_TOOLS_VERSION ?= v0.19.0
159159

160160
KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh"
161161
.PHONY: kustomize

api/v1/ptpconfig_types.go

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,18 +62,19 @@ type PtpConfigList struct {
6262
}
6363

6464
type PtpProfile struct {
65-
Name *string `json:"name"`
66-
Interface *string `json:"interface,omitempty"`
67-
Ptp4lOpts *string `json:"ptp4lOpts,omitempty"`
68-
Phc2sysOpts *string `json:"phc2sysOpts,omitempty"`
69-
Ts2PhcOpts *string `json:"ts2phcOpts,omitempty"`
70-
Synce4lOpts *string `json:"synce4lOpts,omitempty"`
71-
ChronydOpts *string `json:"chronydOpts,omitempty"`
72-
Ptp4lConf *string `json:"ptp4lConf,omitempty"`
73-
Phc2sysConf *string `json:"phc2sysConf,omitempty"`
74-
Ts2PhcConf *string `json:"ts2phcConf,omitempty"`
75-
Synce4lConf *string `json:"synce4lConf,omitempty"`
76-
ChronydConf *string `json:"chronydConf,omitempty"`
65+
Name *string `json:"name"`
66+
Interface *string `json:"interface,omitempty"`
67+
Ptp4lOpts *string `json:"ptp4lOpts,omitempty"`
68+
Phc2sysOpts *string `json:"phc2sysOpts,omitempty"`
69+
Ts2PhcOpts *string `json:"ts2phcOpts,omitempty"`
70+
Synce4lOpts *string `json:"synce4lOpts,omitempty"`
71+
ChronydOpts *string `json:"chronydOpts,omitempty"`
72+
Ptp4lConf *string `json:"ptp4lConf,omitempty"`
73+
Phc2sysConf *string `json:"phc2sysConf,omitempty"`
74+
Ts2PhcConf *string `json:"ts2phcConf,omitempty"`
75+
Synce4lConf *string `json:"synce4lConf,omitempty"`
76+
ChronydConf *string `json:"chronydConf,omitempty"`
77+
PtpSecretName *string `json:"ptpSecretName,omitempty"`
7778
// +kubebuilder:validation:Enum=SCHED_OTHER;SCHED_FIFO;
7879
PtpSchedulingPolicy *string `json:"ptpSchedulingPolicy,omitempty"`
7980
// +kubebuilder:validation:Minimum=1

api/v1/ptpconfig_webhook.go

Lines changed: 280 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package v1
1818

1919
import (
20+
"context"
2021
"errors"
2122
"fmt"
2223
"regexp"
@@ -26,8 +27,12 @@ import (
2627
"time"
2728

2829
"github.com/sirupsen/logrus"
30+
corev1 "k8s.io/api/core/v1"
31+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2932
"k8s.io/apimachinery/pkg/runtime"
33+
"k8s.io/apimachinery/pkg/types"
3034
ctrl "sigs.k8s.io/controller-runtime"
35+
"sigs.k8s.io/controller-runtime/pkg/client"
3136
logf "sigs.k8s.io/controller-runtime/pkg/log"
3237
"sigs.k8s.io/controller-runtime/pkg/webhook"
3338
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -45,7 +50,12 @@ var ptpconfiglog = logf.Log.WithName("ptpconfig-resource")
4550
var profileRegEx = regexp.MustCompile(`^([\w\-_]+)(,\s*([\w\-_]+))*$`)
4651
var clockTypes = []string{"T-GM", "T-BC"}
4752

53+
// webhookClient is used by the webhook to query existing PtpConfigs
54+
var webhookClient client.Client
55+
4856
func (r *PtpConfig) SetupWebhookWithManager(mgr ctrl.Manager) error {
57+
// Store the client for use in validation
58+
webhookClient = mgr.GetClient()
4959
return ctrl.NewWebhookManagedBy(mgr).
5060
For(r).
5161
Complete()
@@ -61,6 +71,26 @@ type ptp4lConf struct {
6171
sections map[string]ptp4lConfSection
6272
}
6373

74+
// Ptp4lConf is a public wrapper for ptp4lConf
75+
type Ptp4lConf struct {
76+
conf ptp4lConf
77+
}
78+
79+
// PopulatePtp4lConf parses the ptp4l configuration
80+
func (p *Ptp4lConf) PopulatePtp4lConf(config *string, ptp4lopts *string) error {
81+
return p.conf.populatePtp4lConf(config, ptp4lopts)
82+
}
83+
84+
// GetOption retrieves an option value from a specific section
85+
func (p *Ptp4lConf) GetOption(section, key string) string {
86+
if sec, ok := p.conf.sections[section]; ok {
87+
if val, ok := sec.options[key]; ok {
88+
return val
89+
}
90+
}
91+
return ""
92+
}
93+
6494
func (output *ptp4lConf) populatePtp4lConf(config *string, ptp4lopts *string) error {
6595
var string_config string
6696
if config != nil {
@@ -103,6 +133,7 @@ func (output *ptp4lConf) populatePtp4lConf(config *string, ptp4lopts *string) er
103133

104134
func (r *PtpConfig) validate() error {
105135
profiles := r.Spec.Profile
136+
106137
for _, profile := range profiles {
107138
conf := &ptp4lConf{}
108139
conf.populatePtp4lConf(profile.Ptp4lConf, profile.Ptp4lOpts)
@@ -194,6 +225,235 @@ func (r *PtpConfig) validate() error {
194225
return nil
195226
}
196227

228+
// validateSecretConflicts checks if this PtpConfig's sa_file + secret combination
229+
// conflicts with any existing PtpConfigs in the openshift-ptp namespace
230+
func (r *PtpConfig) validateSecretConflicts(ctx context.Context) error {
231+
if webhookClient == nil {
232+
ptpconfiglog.Info("webhook client not initialized, skipping cross-PtpConfig validation")
233+
return nil
234+
}
235+
236+
// Build map of sa_file -> secret for THIS PtpConfig
237+
currentSaFileToSecret := make(map[string]string)
238+
for _, profile := range r.Spec.Profile {
239+
if profile.PtpSecretName == nil || *profile.PtpSecretName == "" {
240+
continue
241+
}
242+
if profile.Ptp4lConf == nil {
243+
continue
244+
}
245+
246+
conf := &ptp4lConf{}
247+
if err := conf.populatePtp4lConf(profile.Ptp4lConf, profile.Ptp4lOpts); err != nil {
248+
continue
249+
}
250+
251+
if globalSection, exists := conf.sections["[global]"]; exists {
252+
if saFile, exists := globalSection.options["sa_file"]; exists && saFile != "" {
253+
currentSaFileToSecret[saFile] = *profile.PtpSecretName
254+
}
255+
}
256+
}
257+
258+
// If this PtpConfig doesn't use any secrets, no conflict possible
259+
if len(currentSaFileToSecret) == 0 {
260+
return nil
261+
}
262+
263+
// List all existing PtpConfigs in openshift-ptp namespace
264+
ptpConfigList := &PtpConfigList{}
265+
if err := webhookClient.List(ctx, ptpConfigList, &client.ListOptions{
266+
Namespace: "openshift-ptp",
267+
}); err != nil {
268+
ptpconfiglog.Error(err, "failed to list PtpConfigs for validation")
269+
// Don't block creation if we can't list - fail open
270+
return nil
271+
}
272+
273+
// Check each existing PtpConfig
274+
for _, existingConfig := range ptpConfigList.Items {
275+
// Skip checking against ourselves (for updates)
276+
if existingConfig.Name == r.Name && existingConfig.Namespace == r.Namespace {
277+
continue
278+
}
279+
280+
// Check each profile in the existing config
281+
for _, profile := range existingConfig.Spec.Profile {
282+
if profile.PtpSecretName == nil || *profile.PtpSecretName == "" {
283+
continue
284+
}
285+
if profile.Ptp4lConf == nil {
286+
continue
287+
}
288+
289+
conf := &ptp4lConf{}
290+
if err := conf.populatePtp4lConf(profile.Ptp4lConf, profile.Ptp4lOpts); err != nil {
291+
continue
292+
}
293+
294+
if globalSection, exists := conf.sections["[global]"]; exists {
295+
if saFile, exists := globalSection.options["sa_file"]; exists && saFile != "" {
296+
// Check if THIS PtpConfig uses the same sa_file
297+
if currentSecret, found := currentSaFileToSecret[saFile]; found {
298+
// Conflict: same sa_file but different secret
299+
if currentSecret != *profile.PtpSecretName {
300+
return fmt.Errorf("sa_file '%s' conflict: PtpConfig '%s' already uses secret '%s' with this sa_file path, but this PtpConfig tries to use secret '%s'. All PtpConfigs using the same sa_file must reference the same secret",
301+
saFile, existingConfig.Name, *profile.PtpSecretName, currentSecret)
302+
}
303+
}
304+
}
305+
}
306+
}
307+
}
308+
309+
return nil
310+
}
311+
312+
// validateSecretExists checks that ptpSecretName references an existing secret
313+
func (r *PtpConfig) validateSecretExists(ctx context.Context) error {
314+
if webhookClient == nil {
315+
ptpconfiglog.Info("webhook client not initialized, skipping secret existence validation")
316+
return nil
317+
}
318+
319+
for _, profile := range r.Spec.Profile {
320+
// Skip if no secret specified
321+
if profile.PtpSecretName == nil || *profile.PtpSecretName == "" {
322+
continue
323+
}
324+
325+
secretName := *profile.PtpSecretName
326+
profileName := "unknown"
327+
if profile.Name != nil {
328+
profileName = *profile.Name
329+
}
330+
331+
// Try to get the secret from openshift-ptp namespace
332+
secret := &corev1.Secret{}
333+
err := webhookClient.Get(ctx, types.NamespacedName{
334+
Namespace: "openshift-ptp",
335+
Name: secretName,
336+
}, secret)
337+
338+
if err != nil {
339+
if apierrors.IsNotFound(err) {
340+
return fmt.Errorf("secret '%s' referenced by profile '%s' does not exist in namespace 'openshift-ptp'. Please create the secret before referencing it in PtpConfig",
341+
secretName, profileName)
342+
}
343+
// For other errors (like permission issues), log but don't block
344+
ptpconfiglog.Error(err, "failed to verify secret existence", "secret", secretName, "profile", profileName)
345+
// Fail open - don't block if we can't verify
346+
return nil
347+
}
348+
349+
ptpconfiglog.Info("validated secret exists", "secret", secretName, "profile", profileName)
350+
351+
// Validate SPP (Security Parameter Profile) if specified
352+
if err := r.validateSppInSecret(profile, secret); err != nil {
353+
return err
354+
}
355+
}
356+
357+
return nil
358+
}
359+
360+
// validateSppInSecret checks that the spp value in ptp4lConf exists in the referenced secret
361+
func (r *PtpConfig) validateSppInSecret(profile PtpProfile, secret *corev1.Secret) error {
362+
// Skip if no ptp4lConf
363+
if profile.Ptp4lConf == nil {
364+
return nil
365+
}
366+
367+
// Parse ptp4lConf to get spp value from [global] section
368+
conf := &ptp4lConf{}
369+
if err := conf.populatePtp4lConf(profile.Ptp4lConf, profile.Ptp4lOpts); err != nil {
370+
// If we can't parse the config, skip validation (other validations will catch this)
371+
return nil
372+
}
373+
374+
globalSection, exists := conf.sections["[global]"]
375+
if !exists {
376+
return nil
377+
}
378+
379+
sppValue, exists := globalSection.options["spp"]
380+
if !exists || sppValue == "" {
381+
// No spp specified, nothing to validate
382+
return nil
383+
}
384+
385+
profileName := "unknown"
386+
if profile.Name != nil {
387+
profileName = *profile.Name
388+
}
389+
390+
// Parse the secret data to find valid SPP values
391+
validSpps, err := parseValidSppsFromSecret(secret)
392+
if err != nil {
393+
ptpconfiglog.Error(err, "failed to parse SPPs from secret", "secret", secret.Name, "profile", profileName)
394+
// Fail open - don't block if we can't parse
395+
return nil
396+
}
397+
398+
// Check if the specified SPP exists in the secret
399+
if !contains(validSpps, sppValue) {
400+
return fmt.Errorf("spp '%s' in profile '%s' is not defined in secret '%s'. Valid SPPs in secret: %v",
401+
sppValue, profileName, secret.Name, validSpps)
402+
}
403+
404+
ptpconfiglog.Info("validated spp exists in secret", "spp", sppValue, "secret", secret.Name, "profile", profileName)
405+
return nil
406+
}
407+
408+
// parseValidSppsFromSecret extracts all valid SPP numbers from a PTP security secret
409+
// It looks for any line starting with "spp <number>" regardless of structure or sections
410+
func parseValidSppsFromSecret(secret *corev1.Secret) ([]string, error) {
411+
var validSpps []string
412+
413+
// Iterate through all keys in the secret
414+
for key, value := range secret.Data {
415+
// Parse the security configuration
416+
content := string(value)
417+
lines := strings.Split(content, "\n")
418+
419+
// Look for any line starting with "spp <number>"
420+
for _, line := range lines {
421+
line = strings.TrimSpace(line)
422+
423+
// Skip empty lines and comments
424+
if line == "" || strings.HasPrefix(line, "#") {
425+
continue
426+
}
427+
428+
// Check if line starts with "spp " (case-insensitive)
429+
if strings.HasPrefix(strings.ToLower(line), "spp ") {
430+
parts := strings.Fields(line)
431+
if len(parts) >= 2 {
432+
sppNumber := parts[1]
433+
// Add if not already in list
434+
if !contains(validSpps, sppNumber) {
435+
validSpps = append(validSpps, sppNumber)
436+
}
437+
}
438+
}
439+
}
440+
441+
ptpconfiglog.Info("parsed SPPs from secret", "key", key, "spps", validSpps)
442+
}
443+
444+
return validSpps, nil
445+
}
446+
447+
// contains checks if a string slice contains a specific string
448+
func contains(slice []string, item string) bool {
449+
for _, s := range slice {
450+
if s == item {
451+
return true
452+
}
453+
}
454+
return false
455+
}
456+
197457
var _ webhook.Validator = &PtpConfig{}
198458

199459
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
@@ -203,6 +463,16 @@ func (r *PtpConfig) ValidateCreate() (admission.Warnings, error) {
203463
return admission.Warnings{}, err
204464
}
205465

466+
// Check that referenced secrets exist
467+
if err := r.validateSecretExists(context.Background()); err != nil {
468+
return admission.Warnings{}, err
469+
}
470+
471+
// Check for cross-PtpConfig secret conflicts
472+
if err := r.validateSecretConflicts(context.Background()); err != nil {
473+
return admission.Warnings{}, err
474+
}
475+
206476
return admission.Warnings{}, nil
207477
}
208478

@@ -213,6 +483,16 @@ func (r *PtpConfig) ValidateUpdate(old runtime.Object) (admission.Warnings, erro
213483
return admission.Warnings{}, err
214484
}
215485

486+
// Check that referenced secrets exist
487+
if err := r.validateSecretExists(context.Background()); err != nil {
488+
return admission.Warnings{}, err
489+
}
490+
491+
// Check for cross-PtpConfig secret conflicts
492+
if err := r.validateSecretConflicts(context.Background()); err != nil {
493+
return admission.Warnings{}, err
494+
}
495+
216496
return admission.Warnings{}, nil
217497
}
218498

api/v1/zz_generated.deepcopy.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)