Use proper CSRSigner interface rather than just CSRSignerFunc

micromdm · Sep 24, 2020 · adab923 · adab923
1 parent 57e5ba0
commit adab923
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 14 deletions.
diff --git a/challenge/challenge.go b/challenge/challenge.go
@@ -15,7 +15,7 @@ type Store interface {
 	HasChallenge(pw string) (bool, error)
 }
 
-func csrSignerMiddleWare(store Store, next scepserver.CSRSignerFunc) scepserver.CSRSignerFunc {
+func csrSignerMiddleWare(store Store, next scepserver.CSRSigner) scepserver.CSRSignerFunc {
 	return func(m *scep.CSRReqMessage) (*x509.Certificate, error) {
 		// TODO: this was only verified in the old version if our MessageType was PKCSReq
 		valid, err := store.HasChallenge(m.ChallengePassword)
@@ -30,8 +30,8 @@ func csrSignerMiddleWare(store Store, next scepserver.CSRSignerFunc) scepserver.
 }
 
 // NewCSRSignerMiddleware creates a new middleware adaptor
-func NewCSRSignerMiddleware(store Store) func(scepserver.CSRSignerFunc) scepserver.CSRSignerFunc {
-	return func(f scepserver.CSRSignerFunc) scepserver.CSRSignerFunc {
+func NewCSRSignerMiddleware(store Store) func(scepserver.CSRSigner) scepserver.CSRSigner {
+	return func(f scepserver.CSRSigner) scepserver.CSRSigner {
 		return csrSignerMiddleWare(store, f)
 	}
 }
diff --git a/challenge/challenge_bolt_test.go b/challenge/challenge_bolt_test.go
@@ -9,6 +9,7 @@ import (
 	"github.com/boltdb/bolt"
 	challengestore "github.com/micromdm/scep/challenge/bolt"
 	"github.com/micromdm/scep/scep"
+	scepserver "github.com/micromdm/scep/server"
 )
 
 func TestDynamicChallenge(t *testing.T) {
@@ -62,9 +63,9 @@ func TestDynamicChallenge(t *testing.T) {
 	}
 
 	// test CSRSigner middleware
-	nullSigner := func(*scep.CSRReqMessage) (*x509.Certificate, error) {
+	nullSigner := scepserver.CSRSignerFunc(func(*scep.CSRReqMessage) (*x509.Certificate, error) {
 		return nil, nil
-	}
+	})
 	mw := NewCSRSignerMiddleware(depot)
 	signer := mw(nullSigner)
 

diff --git a/csrverifier/csrverifier.go b/csrverifier/csrverifier.go
@@ -14,7 +14,7 @@ type CSRVerifier interface {
 	Verify(data []byte) (bool, error)
 }
 
-func csrSignerMiddleWare(verifier CSRVerifier, next scepserver.CSRSignerFunc) scepserver.CSRSignerFunc {
+func csrSignerMiddleWare(verifier CSRVerifier, next scepserver.CSRSigner) scepserver.CSRSignerFunc {
 	return func(m *scep.CSRReqMessage) (*x509.Certificate, error) {
 		result, err := verifier.Verify(m.RawDecrypted)
 		if err != nil {
@@ -28,8 +28,8 @@ func csrSignerMiddleWare(verifier CSRVerifier, next scepserver.CSRSignerFunc) sc
 }
 
 // NewCSRSignerMiddleware creates a new middleware adaptor
-func NewCSRSignerMiddleware(verifier CSRVerifier) func(scepserver.CSRSignerFunc) scepserver.CSRSignerFunc {
-	return func(f scepserver.CSRSignerFunc) scepserver.CSRSignerFunc {
+func NewCSRSignerMiddleware(verifier CSRVerifier) func(scepserver.CSRSigner) scepserver.CSRSigner {
+	return func(f scepserver.CSRSigner) scepserver.CSRSigner {
 		return csrSignerMiddleWare(verifier, f)
 	}
 }
diff --git a/server/service.go b/server/service.go
@@ -31,6 +31,14 @@ type Service interface {
 	GetNextCACert(ctx context.Context) ([]byte, error)
 }
 
+// CSRSigner is a handler for CSR signing by the CA/RA
+//
+// SignCSR should take the CSR in the CSRReqMessage and return a
+// Certificate signed by the CA.
+type CSRSigner interface {
+	SignCSR(*scep.CSRReqMessage) (*x509.Certificate, error)
+}
+
 // CSRSignerFunc is an adapter for CSR signing by the CA/RA
 type CSRSignerFunc func(*scep.CSRReqMessage) (*x509.Certificate, error)
 
@@ -53,7 +61,7 @@ type service struct {
 	// The (chainable) CSR signing function. Intended to handle all
 	// SCEP request functionality such as CSR & challenge checking, CA
 	// issuance, RA proxying, etc.
-	signer CSRSignerFunc
+	signer CSRSigner
 
 	/// info logging is implemented in the service middleware layer.
 	debugLogger log.Logger
@@ -124,7 +132,7 @@ func WithAddlCA(ca *x509.Certificate) ServiceOption {
 	}
 }
 
-func staticChallengePasswordCSRSignerMiddleware(pw string, next CSRSignerFunc) CSRSignerFunc {
+func staticChallengePasswordCSRSignerMiddleware(pw string, next CSRSigner) CSRSignerFunc {
 	return func(m *scep.CSRReqMessage) (*x509.Certificate, error) {
 		// TODO: this was only verified in the old version if our MessageType was PKCSReq
 		if pw != m.ChallengePassword {
@@ -144,15 +152,15 @@ func WithStaticChallengePassword(pw string) ServiceOption {
 }
 
 // WithCSRSignerMiddleware wraps the service
-func WithCSRSignerMiddleware(f func(CSRSignerFunc) CSRSignerFunc) ServiceOption {
+func WithCSRSignerMiddleware(f func(CSRSigner) CSRSigner) ServiceOption {
 	return func(s *service) error {
 		s.signer = f(s.signer)
 		return nil
 	}
 }
 
 // NewService creates a new scep service
-func NewService(crt *x509.Certificate, key *rsa.PrivateKey, signer CSRSignerFunc, opts ...ServiceOption) (Service, error) {
+func NewService(crt *x509.Certificate, key *rsa.PrivateKey, signer CSRSigner, opts ...ServiceOption) (Service, error) {
 	s := &service{
 		crt:         crt,
 		key:         key,

diff --git a/server/transport_test.go b/server/transport_test.go
@@ -100,9 +100,9 @@ func newServer(t *testing.T, opts ...scepserver.ServiceOption) (*httptest.Server
 		depot = &noopDepot{depot}
 	}
 	crt, key, err := depot.CA([]byte{})
-	nullSigner := func(*scep.CSRReqMessage) (*x509.Certificate, error) {
+	nullSigner := scepserver.CSRSignerFunc(func(*scep.CSRReqMessage) (*x509.Certificate, error) {
 		return nil, nil
-	}
+	})
 	var svc scepserver.Service // scep service
 	{
 		svc, err = scepserver.NewService(crt[0], key, nullSigner)