diff --git a/csrverifier/csrverifier.go b/csrverifier/csrverifier.go index f4eba5c..db47336 100644 --- a/csrverifier/csrverifier.go +++ b/csrverifier/csrverifier.go @@ -1,7 +1,14 @@ // Package csrverifier defines an interface for CSR verification. package csrverifier +import "context" + // Verify the raw decrypted CSR. type CSRVerifier interface { Verify(data []byte) (bool, error) } + +// Verify the CSR and Challenge together +type CombinedVerifier interface { + Validate(ctx context.Context, data []byte, challenge string) (bool, error) +} diff --git a/server/service.go b/server/service.go index 627ea59..8e2f535 100644 --- a/server/service.go +++ b/server/service.go @@ -5,6 +5,7 @@ import ( "crypto" "crypto/rsa" "crypto/sha1" + "crypto/subtle" "crypto/x509" "encoding/asn1" "errors" @@ -40,17 +41,17 @@ type Service interface { } type service struct { - depot depot.Depot - ca []*x509.Certificate // CA cert or chain - caKey *rsa.PrivateKey - caKeyPassword []byte - csrTemplate *x509.Certificate - challengePassword string - supportDynamciChallenge bool - dynamicChallengeStore challenge.Store - csrVerifier csrverifier.CSRVerifier - allowRenewal int // days before expiry, 0 to disable - clientValidity int // client cert validity in days + depot depot.Depot + ca []*x509.Certificate // CA cert or chain + caKey *rsa.PrivateKey + caKeyPassword []byte + csrTemplate *x509.Certificate + challengePassword string + dynamicChallengeStore challenge.Store + csrVerifier csrverifier.CSRVerifier + combinedVerifier csrverifier.CombinedVerifier + allowRenewal int // days before expiry, 0 to disable + clientValidity int // client cert validity in days /// info logging is implemented in the service middleware layer. debugLogger log.Logger @@ -58,11 +59,10 @@ type service struct { // SCEPChallenge returns a brand new, random dynamic challenge. func (svc *service) SCEPChallenge() (string, error) { - if !svc.supportDynamciChallenge { - return svc.challengePassword, nil + if svc.dynamicChallengeStore != nil { + return svc.dynamicChallengeStore.SCEPChallenge() } - - return svc.dynamicChallengeStore.SCEPChallenge() + return svc.challengePassword, nil } func (svc *service) GetCACaps(ctx context.Context) ([]byte, error) { @@ -81,6 +81,30 @@ func (svc *service) GetCACert(ctx context.Context) ([]byte, int, error) { return data, len(svc.ca), err } +func (svc *service) verify(ctx context.Context, rawCSR []byte, challengePassword string) (bool, error) { + // Consider changing this so verifiers stack, instead of first one wins. + + if svc.combinedVerifier != nil { + return svc.combinedVerifier.Validate(ctx, rawCSR, challengePassword) + } + + if svc.csrVerifier != nil { + return svc.csrVerifier.Verify(rawCSR) + } + + if svc.dynamicChallengeStore != nil { + return svc.dynamicChallengeStore.HasChallenge(challengePassword) + } + + // No required password, don't validate + if svc.challengePassword == "" { + return true, nil + } + + match := subtle.ConstantTimeCompare([]byte(svc.challengePassword), []byte(challengePassword)) == 1 + return match, nil +} + func (svc *service) PKIOperation(ctx context.Context, data []byte) ([]byte, error) { msg, err := scep.ParsePKIMessage(data, scep.WithLogger(svc.debugLogger)) if err != nil { @@ -91,27 +115,15 @@ func (svc *service) PKIOperation(ctx context.Context, data []byte) ([]byte, erro return nil, err } - // validate challenge passwords + // validate csr & challenge password if msg.MessageType == scep.PKCSReq { - CSRIsValid := false - - if svc.csrVerifier != nil { - result, err := svc.csrVerifier.Verify(msg.CSRReqMessage.RawDecrypted) - if err != nil { - return nil, err - } - CSRIsValid = result - if !CSRIsValid { - svc.debugLogger.Log("err", "CSR is not valid") - } - } else { - CSRIsValid = svc.challengePasswordMatch(msg.CSRReqMessage.ChallengePassword) - if !CSRIsValid { - svc.debugLogger.Log("err", "scep challenge password does not match") - } + isValid, err := svc.verify(ctx, msg.CSRReqMessage.RawDecrypted, msg.CSRReqMessage.ChallengePassword) + if err != nil { + svc.debugLogger.Log("err", err) + return nil, err } - if !CSRIsValid { + if !isValid { certRep, err := msg.Fail(ca, svc.caKey, scep.BadRequest) if err != nil { return nil, err @@ -182,27 +194,6 @@ func (svc *service) GetNextCACert(ctx context.Context) ([]byte, error) { panic("not implemented") } -func (svc *service) challengePasswordMatch(pw string) bool { - if svc.challengePassword == "" && !svc.supportDynamciChallenge { - // empty password, don't validate - return true - } - if !svc.supportDynamciChallenge && svc.challengePassword == pw { - return true - } - - if svc.supportDynamciChallenge { - valid, err := svc.dynamicChallengeStore.HasChallenge(pw) - if err != nil { - svc.debugLogger.Log(err) - return false - } - return valid - } - - return false -} - // ServiceOption is a server configuration option type ServiceOption func(*service) error @@ -215,6 +206,13 @@ func WithCSRVerifier(csrVerifier csrverifier.CSRVerifier) ServiceOption { } } +func WithCombinedVerifier(combinedVerifier csrverifier.CombinedVerifier) ServiceOption { + return func(s *service) error { + s.combinedVerifier = combinedVerifier + return nil + } +} + // ChallengePassword is an optional argument to NewService // which allows setting a preshared key for SCEP. func ChallengePassword(pw string) ServiceOption { @@ -260,7 +258,6 @@ func WithLogger(logger log.Logger) ServiceOption { func WithDynamicChallenges(cache challenge.Store) ServiceOption { return func(s *service) error { - s.supportDynamciChallenge = true s.dynamicChallengeStore = cache return nil } diff --git a/server/service_bolt_test.go b/server/service_bolt_test.go index 74f6572..6c29ca8 100644 --- a/server/service_bolt_test.go +++ b/server/service_bolt_test.go @@ -51,11 +51,23 @@ func TestDynamicChallenge(t *testing.T) { } impl := svc.(*service) - if !impl.challengePasswordMatch(challenge) { - t.Errorf("challenge password does not match") + { + result, err := impl.verify(context.TODO(), nil, challenge) + if err != nil { + t.Fatal(err) + } + if !result { + t.Errorf("challenge password does not match") + } } - if impl.challengePasswordMatch(challenge) { - t.Errorf("challenge password matched but should only be used once") + { + result, err := impl.verify(context.TODO(), nil, challenge) + if err != nil { + t.Fatal(err) + } + if result { + t.Errorf("challenge password matched but should only be used once") + } } }