Merge pull request #1795 from smallstep/herman/fix-scep-failinfo-oid

Prevent exposing any internal details in SCEP failure message
pull/1765/merge
Herman Slatman 2 months ago committed by GitHub
commit 397877a7b6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -387,9 +387,10 @@ func PKIOperation(ctx context.Context, req request) (Response, error) {
if msg.MessageType == smallscep.PKCSReq || msg.MessageType == smallscep.RenewalReq { if msg.MessageType == smallscep.PKCSReq || msg.MessageType == smallscep.RenewalReq {
if err := auth.ValidateChallenge(ctx, csr, challengePassword, transactionID); err != nil { if err := auth.ValidateChallenge(ctx, csr, challengePassword, transactionID); err != nil {
if errors.Is(err, provisioner.ErrSCEPChallengeInvalid) { 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 // TODO(hs): ignore this error case? It's not critical if the notification fails; but logging it might be good
_ = notifyErr _ = 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 { 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) 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) 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 { if err != nil {
return Response{}, err return Response{}, err
} }

@ -308,7 +308,7 @@ func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, m
certChain, err := a.signAuth.SignWithContext(ctx, csr, opts, signOps...) certChain, err := a.signAuth.SignWithContext(ctx, csr, opts, signOps...)
if err != nil { 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 // take the issued certificate (only); https://tools.ietf.org/html/rfc8894#section-3.3.2

Loading…
Cancel
Save