Skip to content

Commit 756aaf6

Browse files
committed
fix: code review cleanup from SCEP implementation
- Export radius.UpsertSecret and use it in loadOrGenerateSCEPRACert; fixes create-first ordering that masked non-AlreadyExists errors - Precompute GetCACert degenerate response in NewHandler instead of re-encoding three certs on every request - Bound pkiOperation body read to 64 KiB with io.LimitReader - Hoist radiusHost split above switch in GenerateProfileHandler
1 parent 065082e commit 756aaf6

4 files changed

Lines changed: 17 additions & 19 deletions

File tree

cmd/pint/main.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -471,10 +471,8 @@ func loadOrGenerateSCEPRACert(ctx context.Context, log *zap.Logger, k8sClient ku
471471
ObjectMeta: metav1.ObjectMeta{Name: cfg.SCEPRACertSecret, Namespace: cfg.Namespace},
472472
Data: map[string][]byte{"tls.crt": certPEM, "tls.key": keyPEM},
473473
}
474-
if _, createErr := k8sClient.CoreV1().Secrets(cfg.Namespace).Create(ctx, raSecret, metav1.CreateOptions{}); createErr != nil {
475-
if _, updateErr := k8sClient.CoreV1().Secrets(cfg.Namespace).Update(ctx, raSecret, metav1.UpdateOptions{}); updateErr != nil {
476-
return nil, nil, nil, fmt.Errorf("store SCEP RA cert: %w", updateErr)
477-
}
474+
if err := radius.UpsertSecret(ctx, k8sClient, raSecret); err != nil {
475+
return nil, nil, nil, fmt.Errorf("store SCEP RA cert: %w", err)
478476
}
479477
log.Info("generated new SCEP RA cert")
480478
return cert, key, cert.Raw, nil

internal/handlers/profile.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ func GenerateProfileHandler(log *zap.Logger, ipaClient *freeipa.Client, cfg *con
4444
return
4545
}
4646

47+
radiusHost := strings.Split(cfg.RadiusServer, ":")[0]
4748
switch platform {
4849
case "windows":
49-
radiusHost := strings.Split(cfg.RadiusServer, ":")[0]
5050
wlan, err := profile.BuildWLANProfile(profile.WLANProfileParams{
5151
SSID: cfg.WiFiSSID,
5252
RadiusHost: radiusHost,
@@ -68,7 +68,6 @@ func GenerateProfileHandler(log *zap.Logger, ipaClient *freeipa.Client, cfg *con
6868
c.JSON(http.StatusInternalServerError, gin.H{"error": "challenge generation failed"})
6969
return
7070
}
71-
radiusHost := strings.Split(cfg.RadiusServer, ":")[0]
7271
mc, err := profile.BuildMobileconfig(profile.MobileconfigParams{
7372
SSID: cfg.WiFiSSID,
7473
RadiusHost: radiusHost,

internal/radius/reload.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func WriteRadSecTLS(ctx context.Context, k8s kubernetes.Interface, namespace, se
4545
// - ca.pem: RadSec CA chain; verifies connecting router client certificates
4646
// - wifi-ca.pem: WiFi CA cert; verifies EAP-TLS user certificates
4747
func WriteRadSecServerCert(ctx context.Context, k8s kubernetes.Interface, namespace, secretName string, certPEM, keyPEM, caPEM, wifiCAPEM []byte) error {
48-
return upsertSecret(ctx, k8s, &corev1.Secret{
48+
return UpsertSecret(ctx, k8s, &corev1.Secret{
4949
ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: namespace},
5050
Data: map[string][]byte{
5151
"tls.crt": certPEM,
@@ -85,8 +85,8 @@ func patchSecretKey(ctx context.Context, k8s kubernetes.Interface, namespace, se
8585
return err
8686
}
8787

88-
// upsertSecret creates or updates a Kubernetes Secret.
89-
func upsertSecret(ctx context.Context, k8s kubernetes.Interface, secret *corev1.Secret) error {
88+
// UpsertSecret creates or updates a Kubernetes Secret.
89+
func UpsertSecret(ctx context.Context, k8s kubernetes.Interface, secret *corev1.Secret) error {
9090
ns := secret.Namespace
9191
_, err := k8s.CoreV1().Secrets(ns).Update(ctx, secret, metav1.UpdateOptions{})
9292
if errors.IsNotFound(err) {

internal/scep/server.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ type Handler struct {
3030
raKey *rsa.PrivateKey
3131
// RA cert + wireless CA + root CA, ordered for GetCACert response.
3232
// RA cert must be first so clients use it for envelope encryption.
33-
caCerts []*x509.Certificate
33+
caCerts []*x509.Certificate
34+
degCACerts []byte // precomputed GetCACert response body
3435
}
3536

3637
func NewHandler(log *zap.Logger, store *ChallengeStore, ipaClient *freeipa.Client, caName, certProfile string, raCert *x509.Certificate, raKey *rsa.PrivateKey, caDER, rootCACertDER []byte) (*Handler, error) {
@@ -42,6 +43,11 @@ func NewHandler(log *zap.Logger, store *ChallengeStore, ipaClient *freeipa.Clien
4243
if err != nil {
4344
return nil, fmt.Errorf("parse root CA: %w", err)
4445
}
46+
caCerts := []*x509.Certificate{raCert, wifiCA, rootCA}
47+
deg, err := sceppkg.DegenerateCertificates(caCerts)
48+
if err != nil {
49+
return nil, fmt.Errorf("build CA cert response: %w", err)
50+
}
4551
return &Handler{
4652
log: log,
4753
store: store,
@@ -50,7 +56,8 @@ func NewHandler(log *zap.Logger, store *ChallengeStore, ipaClient *freeipa.Clien
5056
certProfile: certProfile,
5157
raCert: raCert,
5258
raKey: raKey,
53-
caCerts: []*x509.Certificate{raCert, wifiCA, rootCA},
59+
caCerts: caCerts,
60+
degCACerts: deg,
5461
}, nil
5562
}
5663

@@ -75,13 +82,7 @@ func (h *Handler) handle(c *gin.Context) {
7582
}
7683

7784
func (h *Handler) getCACert(c *gin.Context) {
78-
deg, err := sceppkg.DegenerateCertificates(h.caCerts)
79-
if err != nil {
80-
h.log.Error("scep: GetCACert failed", zap.Error(err))
81-
c.Status(http.StatusInternalServerError)
82-
return
83-
}
84-
c.Data(http.StatusOK, "application/x-x509-ca-ra-cert", deg)
85+
c.Data(http.StatusOK, "application/x-x509-ca-ra-cert", h.degCACerts)
8586
}
8687

8788
func (h *Handler) pkiOperation(c *gin.Context) {
@@ -91,7 +92,7 @@ func (h *Handler) pkiOperation(c *gin.Context) {
9192
// SCEP GET fallback: message is base64-encoded in the query param.
9293
body, err = base64.StdEncoding.DecodeString(c.Query("message"))
9394
} else {
94-
body, err = io.ReadAll(c.Request.Body)
95+
body, err = io.ReadAll(io.LimitReader(c.Request.Body, 64*1024))
9596
}
9697
if err != nil || len(body) == 0 {
9798
c.Status(http.StatusBadRequest)

0 commit comments

Comments
 (0)