From b226b6eb4c993b430bd1b8c50edd5ad27df6ad10 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Wed, 10 Apr 2024 01:58:00 +0200 Subject: [PATCH] Prevent exposing any internal details in SCEP failure message To be on the safe side, block errors from signing operations from being returned to the client. We should revisit, and make it return a more informative error, but with high assurance that no sensitive information is added to the message. --- scep/api/api.go | 11 ++++++----- scep/authority.go | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/scep/api/api.go b/scep/api/api.go index bab60302..fd3c61ea 100644 --- a/scep/api/api.go +++ b/scep/api/api.go @@ -387,9 +387,10 @@ func PKIOperation(ctx context.Context, req request) (Response, error) { if msg.MessageType == smallscep.PKCSReq || msg.MessageType == smallscep.RenewalReq { if err := auth.ValidateChallenge(ctx, csr, challengePassword, transactionID); err != nil { if errors.Is(err, provisioner.ErrSCEPChallengeInvalid) { - return createFailureResponse(ctx, csr, msg, smallscep.BadRequest, err) + return createFailureResponse(ctx, csr, msg, smallscep.BadRequest, err.Error(), err) } - return createFailureResponse(ctx, csr, msg, smallscep.BadRequest, errors.New("failed validating challenge password")) + scepErr := errors.New("failed validating challenge password") + return createFailureResponse(ctx, csr, msg, smallscep.BadRequest, scepErr.Error(), scepErr) } } @@ -407,7 +408,7 @@ func PKIOperation(ctx context.Context, req request) (Response, error) { // TODO(hs): ignore this error case? It's not critical if the notification fails; but logging it might be good _ = notifyErr } - return createFailureResponse(ctx, csr, msg, smallscep.BadRequest, fmt.Errorf("error when signing new certificate: %w", err)) + return createFailureResponse(ctx, csr, msg, smallscep.BadRequest, "internal server error; please see the certificate authority logs for more info", fmt.Errorf("error when signing new certificate: %w", err)) } if notifyErr := auth.NotifySuccess(ctx, csr, certRep.Certificate, transactionID); notifyErr != nil { @@ -448,9 +449,9 @@ func fail(w http.ResponseWriter, err error) { http.Error(w, err.Error(), http.StatusInternalServerError) } -func createFailureResponse(ctx context.Context, csr *x509.CertificateRequest, msg *scep.PKIMessage, info smallscep.FailInfo, failError error) (Response, error) { +func createFailureResponse(ctx context.Context, csr *x509.CertificateRequest, msg *scep.PKIMessage, info smallscep.FailInfo, infoText string, failError error) (Response, error) { auth := scep.MustFromContext(ctx) - certRepMsg, err := auth.CreateFailureResponse(ctx, csr, msg, scep.FailInfoName(info), failError.Error()) + certRepMsg, err := auth.CreateFailureResponse(ctx, csr, msg, scep.FailInfoName(info), infoText) if err != nil { return Response{}, err } diff --git a/scep/authority.go b/scep/authority.go index 8ed065fb..00c58d8d 100644 --- a/scep/authority.go +++ b/scep/authority.go @@ -308,7 +308,7 @@ func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, m certChain, err := a.signAuth.SignWithContext(ctx, csr, opts, signOps...) if err != nil { - return nil, fmt.Errorf("error generating certificate for order: %w", err) + return nil, fmt.Errorf("error generating certificate: %w", err) } // take the issued certificate (only); https://tools.ietf.org/html/rfc8894#section-3.3.2