Skip to content

Commit

Permalink
recording: Remove state from the webhook, use a random nonce instead
Browse files Browse the repository at this point in the history 
Instead of keeping track of the replicas in the webhook, let's use a
random nonce to distinguish between different container replicas. When
the profile is recorded for a replicated container, we already know the
pod name, so let's use the hash of the pod to append to the container.
This would also make it easier to cross-reference the pods and the
containers.

In order to parse the recording annotations easily, we also switch from
using dashes in the recording annotations to underscores. Because
underscores can't be used in k8s object names, they make it easy to
split the annotations along them and are allowed in annotations.
  • Loading branch information
@jhrozek
jhrozek committed Sep 6, 2022
1 parent eb43c8a commit 7786f81
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 163 deletions.
51 changes: 20 additions & 31 deletions api/profilerecording/v1alpha1/profilerecording_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilrand "k8s.io/apimachinery/pkg/util/rand"

"sigs.k8s.io/security-profiles-operator/internal/pkg/config"
)
Expand Down Expand Up @@ -79,17 +80,12 @@ type ProfileRecording struct {
Status ProfileRecordingStatus `json:"status,omitempty"`
}

func (pr *ProfileRecording) CtrAnnotation(replica, ctrName string) (key, value string, err error) {
ctrReplicaName := ctrName
if replica != "" {
ctrReplicaName += replica
}

func (pr *ProfileRecording) CtrAnnotation(ctrName string) (key, value string, err error) {
switch pr.Spec.Kind {
case ProfileRecordingKindSeccompProfile:
return pr.ctrAnnotationSeccomp(ctrReplicaName, ctrName)
return pr.ctrAnnotationSeccomp(ctrName)
case ProfileRecordingKindSelinuxProfile:
return pr.ctrAnnotationSelinux(ctrReplicaName, ctrName)
return pr.ctrAnnotationSelinux(ctrName)
}

return "", "", fmt.Errorf(
Expand All @@ -105,58 +101,51 @@ func (pr *ProfileRecording) IsKindSupported() bool {
return false
}

func (pr *ProfileRecording) ctrAnnotationSeccomp(ctrReplicaName, ctrName string) (key, value string, err error) {
func (pr *ProfileRecording) ctrAnnotationValue(ctrName string) string {
const nonceSize = 5

return fmt.Sprintf(
"%s_%s_%s_%d",
pr.GetName(),
ctrName,
utilrand.String(nonceSize),
time.Now().Unix(),
)
}

func (pr *ProfileRecording) ctrAnnotationSeccomp(ctrName string) (key, value string, err error) {
var annotationPrefix string

switch pr.Spec.Recorder {
case ProfileRecorderLogs:
annotationPrefix = config.SeccompProfileRecordLogsAnnotationKey
value = fmt.Sprintf(
"%s-%s-%d",
pr.GetName(),
ctrReplicaName,
time.Now().Unix(),
)

case ProfileRecorderBpf:
annotationPrefix = config.SeccompProfileRecordBpfAnnotationKey
value = fmt.Sprintf(
"%s-%s-%d",
pr.GetName(),
ctrReplicaName,
time.Now().Unix(),
)

default:
return "", "", fmt.Errorf(
"invalid recorder: %s", pr.Spec.Recorder,
)
}

key = annotationPrefix + ctrName
value = pr.ctrAnnotationValue(ctrName)
return key, value, err
}

func (pr *ProfileRecording) ctrAnnotationSelinux(ctrReplicaName, ctrName string) (key, value string, err error) {
func (pr *ProfileRecording) ctrAnnotationSelinux(ctrName string) (key, value string, err error) {
var annotationPrefix string

switch pr.Spec.Recorder {
case ProfileRecorderLogs:
annotationPrefix = config.SelinuxProfileRecordLogsAnnotationKey
value = fmt.Sprintf(
"%s-%s-%d",
pr.GetName(),
ctrReplicaName,
time.Now().Unix(),
)

case ProfileRecorderBpf:
default:
return "", "", fmt.Errorf(
"invalid recorder: %s, only %s is supported", pr.Spec.Recorder, ProfileRecorderLogs,
)
}

value = pr.ctrAnnotationValue(ctrName)
key = annotationPrefix + ctrName
return
}
Expand Down
1 change: 1 addition & 0 deletions internal/pkg/daemon/profilerecorder/impl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package profilerecorder

import (
"context"

"github.com/containers/common/pkg/seccomp"
"github.com/crossplane/crossplane-runtime/pkg/resource"
"google.golang.org/grpc"
Expand Down
118 changes: 84 additions & 34 deletions internal/pkg/daemon/profilerecorder/profilerecorder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ type profileToCollect struct {
}

type podToWatch struct {
baseName types.NamespacedName
recorder profilerecording1alpha1.ProfileRecorder
profiles []profileToCollect
}
Expand Down Expand Up @@ -257,9 +258,16 @@ func (r *RecorderReconciler) Reconcile(_ context.Context, req reconcile.Request)
logger.Info("Recording profile", "kind", prf.kind, "name", prf.name, "pod", req.NamespacedName.String())
}

// for pods managed by a replicated controller, let's store the replicated
// name so that we can later strip the suffix from the fully-generated pod name
baseName := req.NamespacedName
if pod.GenerateName != "" {
baseName.Name = pod.GenerateName
}

r.podsToWatch.Store(
req.NamespacedName.String(),
podToWatch{recorder, profiles},
podToWatch{baseName, recorder, profiles},
)
r.record.Event(pod, event.Normal(reasonProfileRecording, "Recording profiles"))
}
Expand Down Expand Up @@ -322,9 +330,9 @@ func (r *RecorderReconciler) stopBpfRecorder() error {
}

func (r *RecorderReconciler) collectProfile(
ctx context.Context, name types.NamespacedName,
ctx context.Context, podName types.NamespacedName,
) error {
n := name.String()
n := podName.String()

value, ok := r.podsToWatch.Load(n)
if !ok {
Expand All @@ -336,17 +344,23 @@ func (r *RecorderReconciler) collectProfile(
return errors.New("type assert pod to watch")
}

replicaSuffix := ""
if podToWatch.baseName.Name != podName.Name && strings.HasPrefix(podName.Name, podToWatch.baseName.Name) {
// this is a replica, we need to strip the suffix from the pod name
replicaSuffix = strings.TrimPrefix(podName.Name, podToWatch.baseName.Name)
}

if podToWatch.recorder == profilerecording1alpha1.ProfileRecorderLogs {
if err := r.collectLogProfiles(
ctx, name, podToWatch.profiles,
ctx, replicaSuffix, podName, podToWatch.profiles,
); err != nil {
return fmt.Errorf("collect log profile: %w", err)
}
}

if podToWatch.recorder == profilerecording1alpha1.ProfileRecorderBpf {
if err := r.collectBpfProfiles(
ctx, name, podToWatch.profiles,
ctx, replicaSuffix, podName, podToWatch.profiles,
); err != nil {
return fmt.Errorf("collect bpf profile: %w", err)
}
Expand All @@ -358,7 +372,8 @@ func (r *RecorderReconciler) collectProfile(

func (r *RecorderReconciler) collectLogProfiles(
ctx context.Context,
name types.NamespacedName,
replicaSuffix string,
podName types.NamespacedName,
profiles []profileToCollect,
) error {
r.log.Info("Checking checking if enricher is enabled")
Expand All @@ -381,19 +396,18 @@ func (r *RecorderReconciler) collectLogProfiles(
enricherClient := enricherapi.NewEnricherClient(conn)

for _, prf := range profiles {
// Remove the timestamp
profileName, err := extractProfileName(prf.name)
profileNamespacedName, err := profileIDToName(replicaSuffix, podName.Namespace, prf.name)
if err != nil {
return fmt.Errorf("extract profile name: %w", err)
return fmt.Errorf("converting profile id to name: %w", err)
}

r.log.Info("Collecting profile", "name", profileName, "kind", prf.kind)
r.log.Info("Collecting profile", "name", profileNamespacedName, "kind", prf.kind)

switch prf.kind {
case profilerecording1alpha1.ProfileRecordingKindSeccompProfile:
err = r.collectLogSeccompProfile(ctx, enricherClient, profileName, name.Namespace, prf.name)
err = r.collectLogSeccompProfile(ctx, enricherClient, profileNamespacedName, prf.name)
case profilerecording1alpha1.ProfileRecordingKindSelinuxProfile:
err = r.collectLogSelinuxProfile(ctx, enricherClient, profileName, name.Namespace, prf.name)
err = r.collectLogSelinuxProfile(ctx, enricherClient, profileNamespacedName, prf.name)
default:
err = fmt.Errorf("unrecognized kind %s", prf.kind)
}
Expand All @@ -409,8 +423,7 @@ func (r *RecorderReconciler) collectLogProfiles(
func (r *RecorderReconciler) collectLogSeccompProfile(
ctx context.Context,
enricherClient enricherapi.EnricherClient,
profileName string,
namespace string,
profileNamespacedName types.NamespacedName,
profileID string,
) error {
// Retrieve the syscalls for the recording
Expand All @@ -436,8 +449,8 @@ func (r *RecorderReconciler) collectLogSeccompProfile(

profile := &seccompprofileapi.SeccompProfile{
ObjectMeta: metav1.ObjectMeta{
Name: profileName,
Namespace: namespace,
Name: profileNamespacedName.Name,
Namespace: profileNamespacedName.Namespace,
},
Spec: profileSpec,
}
Expand All @@ -454,7 +467,7 @@ func (r *RecorderReconciler) collectLogSeccompProfile(
return fmt.Errorf("create seccompProfile resource: %w", err)
}

r.log.Info("Created/updated profile", "action", res, "name", profileName)
r.log.Info("Created/updated profile", "action", res, "name", profileNamespacedName.Name)
r.record.Event(
profile,
event.Normal(reasonProfileCreated, "seccomp profile created"),
Expand All @@ -471,8 +484,7 @@ func (r *RecorderReconciler) collectLogSeccompProfile(
func (r *RecorderReconciler) collectLogSelinuxProfile(
ctx context.Context,
enricherClient enricherapi.EnricherClient,
profileName string,
namespace string,
profileNamespacedName types.NamespacedName,
profileID string,
) error {
// Retrieve the syscalls for the recording
Expand All @@ -493,8 +505,8 @@ func (r *RecorderReconciler) collectLogSelinuxProfile(

profile := &selxv1alpha2.SelinuxProfile{
ObjectMeta: metav1.ObjectMeta{
Name: profileName,
Namespace: namespace,
Name: profileNamespacedName.Name,
Namespace: profileNamespacedName.Namespace,
},
Spec: selinuxProfileSpec,
}
Expand All @@ -518,15 +530,15 @@ func (r *RecorderReconciler) collectLogSelinuxProfile(
r.record.Event(profile, event.Warning(reasonProfileCreationFailed, err))
return fmt.Errorf("create selinuxprofile resource: %w", err)
}
r.log.Info("Created/updated selinux profile", "action", res, "name", profileName)
r.log.Info("Created/updated selinux profile", "action", res, "name", profileNamespacedName)
r.record.Event(
profile,
event.Normal(reasonProfileCreated, "selinuxprofile profile created"),
)

// Reset the selinuxprofile for further recordings
if err := r.ResetAvcs(ctx, enricherClient, request); err != nil {
return fmt.Errorf("reset selinuxprofile for profile %s: %w", profileName, err)
return fmt.Errorf("reset selinuxprofile for profile %s: %w", profileNamespacedName, err)
}

return nil
Expand All @@ -552,7 +564,8 @@ func (r *RecorderReconciler) formatSelinuxProfile(

func (r *RecorderReconciler) collectBpfProfiles(
ctx context.Context,
name types.NamespacedName,
replicaSuffix string,
podName types.NamespacedName,
profiles []profileToCollect,
) error {
recorderClient, cancel, err := r.getBpfRecorderClient()
Expand Down Expand Up @@ -593,15 +606,15 @@ func (r *RecorderReconciler) collectBpfProfiles(
}

// Remove the timestamp
profileName, err := extractProfileName(profile.name)
profileNamespacedName, err := profileIDToName(replicaSuffix, podName.Namespace, profile.name)
if err != nil {
return fmt.Errorf("extract profile name: %w", err)
return fmt.Errorf("converting profile id to name: %w", err)
}

profile := &seccompprofileapi.SeccompProfile{
ObjectMeta: metav1.ObjectMeta{
Name: profileName,
Namespace: name.Namespace,
Name: profileNamespacedName.Name,
Namespace: profileNamespacedName.Namespace,
},
Spec: profileSpec,
}
Expand All @@ -618,7 +631,7 @@ func (r *RecorderReconciler) collectBpfProfiles(
return fmt.Errorf("create seccompProfile resource: %w", err)
}

r.log.Info("Created/updated profile", "action", res, "name", profileName)
r.log.Info("Created/updated profile", "action", res, "name", profileNamespacedName)
r.record.Event(
profile,
event.Normal(reasonProfileCreated, "seccomp profile created"),
Expand All @@ -632,12 +645,49 @@ func (r *RecorderReconciler) collectBpfProfiles(
return nil
}

func extractProfileName(s string) (string, error) {
lastIndex := strings.LastIndex(s, "-")
if lastIndex == -1 {
return "", fmt.Errorf("malformed profile path: %s", s)
type parsedAnnotation struct {
profileName string
cntName string
nonce string
timestamp string
}

func parseProfileAnnotation(annotation string) (*parsedAnnotation, error) {
const expectedParts = 4

parts := strings.Split(annotation, "_")
if len(parts) != expectedParts {
return nil,
fmt.Errorf("invalid annotation: %s, expected %d parts got %d", annotation, expectedParts, len(parts))
}
return s[:lastIndex], nil

return &parsedAnnotation{
profileName: parts[0],
cntName: parts[1],
nonce: parts[2], // unused for now, but we might need it in the future
timestamp: parts[3], // unused for now, but let's keep it for future use
}, nil
}

func createProfileName(cntName, replicaSuffix, namespace, profileName string) types.NamespacedName {
name := fmt.Sprintf("%s-%s", profileName, cntName)
if replicaSuffix != "" {
name = fmt.Sprintf("%s-%s", name, replicaSuffix)
}

return types.NamespacedName{
Name: name,
Namespace: namespace,
}
}

func profileIDToName(replicaSuffix, namespace, profileID string) (types.NamespacedName, error) {
parsedProfileName, err := parseProfileAnnotation(profileID)
if err != nil {
return types.NamespacedName{}, fmt.Errorf("parse profile raw annotation: %w", err)
}

return createProfileName(parsedProfileName.cntName, replicaSuffix, namespace, parsedProfileName.profileName), nil
}

// parseLogAnnotations parses the provided annotations and extracts the
Expand Down
Loading

0 comments on commit 7786f81

Please sign in to comment.