Add support for ECDSA in Service Providers (crewjam#602)

Rebased and refactored from crewjam#586 from @floren

Co-authored-by: John Floren <[email protected]>

Author: crewjam

samlsp/new.go

@@ -2,11 +2,15 @@
 package samlsp
 
 import (
+	"crypto"
+	"crypto/ecdsa"
 	"crypto/rsa"
 	"crypto/x509"
+	"fmt"
 	"net/http"
 	"net/url"
 
+	"github.com/golang-jwt/jwt/v4"
 	dsig "github.com/russellhaering/goxmldsig"
 
 	"github.com/crewjam/saml"
@@ -16,7 +20,7 @@ import (
 type Options struct {
 	EntityID              string
 	URL                   url.URL
-	Key                   *rsa.PrivateKey
+	Key                   crypto.Signer
 	Certificate           *x509.Certificate
 	Intermediates         []*x509.Certificate
 	HTTPClient            *http.Client
@@ -33,11 +37,23 @@ type Options struct {
 	LogoutBindings        []string
 }
 
+func getDefaultSigningMethod(signer crypto.Signer) jwt.SigningMethod {
+	if signer != nil {
+		switch signer.Public().(type) {
+		case *ecdsa.PublicKey:
+			return jwt.SigningMethodES256
+		case *rsa.PublicKey:
+			return jwt.SigningMethodRS256
+		}
+	}
+	return jwt.SigningMethodRS256
+}
+
 // DefaultSessionCodec returns the default SessionCodec for the provided options,
 // a JWTSessionCodec configured to issue signed tokens.
 func DefaultSessionCodec(opts Options) JWTSessionCodec {
 	return JWTSessionCodec{
-		SigningMethod: defaultJWTSigningMethod,
+		SigningMethod: getDefaultSigningMethod(opts.Key),
 		Audience:      opts.URL.String(),
 		Issuer:        opts.URL.String(),
 		MaxAge:        defaultSessionMaxAge,
@@ -67,7 +83,7 @@ func DefaultSessionProvider(opts Options) CookieSessionProvider {
 // options, a JWTTrackedRequestCodec that uses a JWT to encode TrackedRequests.
 func DefaultTrackedRequestCodec(opts Options) JWTTrackedRequestCodec {
 	return JWTTrackedRequestCodec{
-		SigningMethod: defaultJWTSigningMethod,
+		SigningMethod: getDefaultSigningMethod(opts.Key),
 		Audience:      opts.URL.String(),
 		Issuer:        opts.URL.String(),
 		MaxAge:        saml.MaxIssueDelay,
@@ -99,7 +115,8 @@ func DefaultServiceProvider(opts Options) saml.ServiceProvider {
 	if opts.ForceAuthn {
 		forceAuthn = &opts.ForceAuthn
 	}
-	signatureMethod := dsig.RSASHA1SignatureMethod
+
+	signatureMethod := defaultSigningMethodForKey(opts.Key)
 	if !opts.SignRequest {
 		signatureMethod = ""
 	}
@@ -131,6 +148,19 @@ func DefaultServiceProvider(opts Options) saml.ServiceProvider {
 	}
 }
 
+func defaultSigningMethodForKey(key crypto.Signer) string {
+	switch key.(type) {
+	case *rsa.PrivateKey:
+		return dsig.RSASHA1SignatureMethod
+	case *ecdsa.PrivateKey:
+		return dsig.ECDSASHA256SignatureMethod
+	case nil:
+		return ""
+	default:
+		panic(fmt.Sprintf("programming error: unsupported key type %T", key))
+	}
+}
+
 // DefaultAssertionHandler returns the default AssertionHandler for the provided options,
 // a NopAssertionHandler configured to do nothing.
 func DefaultAssertionHandler(_ Options) NopAssertionHandler {

samlsp/request_tracker_jwt.go

@@ -1,7 +1,7 @@
 package samlsp
 
 import (
-	"crypto/rsa"
+	"crypto"
 	"fmt"
 	"time"
 
@@ -10,15 +10,13 @@ import (
 	"github.com/crewjam/saml"
 )
 
-var defaultJWTSigningMethod = jwt.SigningMethodRS256
-
 // JWTTrackedRequestCodec encodes TrackedRequests as signed JWTs
 type JWTTrackedRequestCodec struct {
 	SigningMethod jwt.SigningMethod
 	Audience      string
 	Issuer        string
 	MaxAge        time.Duration
-	Key           *rsa.PrivateKey
+	Key           crypto.Signer
 }
 
 var _ TrackedRequestCodec = JWTTrackedRequestCodec{}

samlsp/session_jwt.go

@@ -1,7 +1,7 @@
 package samlsp
 
 import (
-	"crypto/rsa"
+	"crypto"
 	"errors"
 	"fmt"
 	"time"
@@ -23,7 +23,7 @@ type JWTSessionCodec struct {
 	Audience      string
 	Issuer        string
 	MaxAge        time.Duration
-	Key           *rsa.PrivateKey
+	Key           crypto.Signer
 }
 
 var _ SessionCodec = JWTSessionCodec{}

service_provider.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"compress/flate"
 	"context"
+	"crypto"
+	"crypto/ecdsa"
 	"crypto/rsa"
 	"crypto/sha256"
 	"crypto/sha512"
@@ -68,8 +70,9 @@ type ServiceProvider struct {
 	// Entity ID is optional - if not specified then MetadataURL will be used
 	EntityID string
 
-	// Key is the RSA private key we use to sign requests.
-	Key *rsa.PrivateKey
+	// Key is private key we use to sign requests. It must be either an
+	// *rsa.PrivateKey or an *ecdsa.PrivateKey.
+	Key crypto.Signer
 
 	// Certificate is the RSA public part of Key.
 	Certificate   *x509.Certificate
@@ -131,7 +134,17 @@ type ServiceProvider struct {
 	// to verify signatures.
 	SignatureVerifier SignatureVerifier
 
-	// SignatureMethod, if non-empty, authentication requests will be signed
+	// SignatureMethod, if non-empty, authentication requests will be signed.
+	//
+	// The method specified here must be consistent with the type of Key.
+	//
+	// If Key is *rsa.PrivateKey, then this must be one of dsig.RSASHA1SignatureMethod,
+	// dsig.RSASHA256SignatureMethod, dsig.RSASHA384SignatureMethod, or
+	// dsig.RSASHA512SignatureMethod:
+	//
+	// If Key is *ecdsa.PrivateKey, then this must be one of dsig.ECDSASHA1SignatureMethod,
+	// dsig.ECDSASHA256SignatureMethod, dsig.ECDSASHA384SignatureMethod, or
+	// dsig.ECDSASHA512SignatureMethod.
 	SignatureMethod string
 
 	// LogoutBindings specify the bindings available for SLO endpoint. If empty,
@@ -548,17 +561,38 @@ func GetSigningContext(sp *ServiceProvider) (*dsig.SigningContext, error) {
 	// for _, cert := range sp.Intermediates {
 	// 	keyPair.Certificate = append(keyPair.Certificate, cert.Raw)
 	// }
-	keyStore := dsig.TLSCertKeyStore(keyPair)
 
-	if sp.SignatureMethod != dsig.RSASHA1SignatureMethod &&
-		sp.SignatureMethod != dsig.RSASHA256SignatureMethod &&
-		sp.SignatureMethod != dsig.RSASHA512SignatureMethod {
+	switch sp.SignatureMethod {
+	case dsig.RSASHA1SignatureMethod,
+		dsig.RSASHA256SignatureMethod,
+		dsig.RSASHA384SignatureMethod,
+		dsig.RSASHA512SignatureMethod:
+		if _, ok := sp.Key.(*rsa.PrivateKey); !ok {
+			return nil, fmt.Errorf("signature method %s requires a key of type rsa.PrivateKey, not %T", sp.SignatureMethod, sp.Key)
+		}
+
+	case dsig.ECDSASHA1SignatureMethod,
+		dsig.ECDSASHA256SignatureMethod,
+		dsig.ECDSASHA384SignatureMethod,
+		dsig.ECDSASHA512SignatureMethod:
+		if _, ok := sp.Key.(*ecdsa.PrivateKey); !ok {
+			return nil, fmt.Errorf("signature method %s requires a key of type ecdsa.PrivateKey, not %T", sp.SignatureMethod, sp.Key)
+		}
+	default:
 		return nil, fmt.Errorf("invalid signing method %s", sp.SignatureMethod)
 	}
-	signatureMethod := sp.SignatureMethod
-	signingContext := dsig.NewDefaultSigningContext(keyStore)
+
+	keyStore := dsig.TLSCertKeyStore(keyPair)
+	chain, err := keyStore.GetChain()
+	if err != nil {
+		return nil, err
+	}
+	signingContext, err := dsig.NewSigningContext(sp.Key, chain)
+	if err != nil {
+		return nil, err
+	}
 	signingContext.Canonicalizer = dsig.MakeC14N10ExclusiveCanonicalizerWithPrefixList(canonicalizerPrefixList)
-	if err := signingContext.SetSignatureMethod(signatureMethod); err != nil {
+	if err := signingContext.SetSignatureMethod(sp.SignatureMethod); err != nil {
 		return nil, err
 	}
 
@@ -1307,31 +1341,12 @@ func (sp *ServiceProvider) validateSignature(el *etree.Element) error {
 
 // SignLogoutRequest adds the `Signature` element to the `LogoutRequest`.
 func (sp *ServiceProvider) SignLogoutRequest(req *LogoutRequest) error {
-	keyPair := tls.Certificate{
-		Certificate: [][]byte{sp.Certificate.Raw},
-		PrivateKey:  sp.Key,
-		Leaf:        sp.Certificate,
-	}
-	// TODO: add intermediates for SP
-	// for _, cert := range sp.Intermediates {
-	// 	keyPair.Certificate = append(keyPair.Certificate, cert.Raw)
-	// }
-	keyStore := dsig.TLSCertKeyStore(keyPair)
-
-	if sp.SignatureMethod != dsig.RSASHA1SignatureMethod &&
-		sp.SignatureMethod != dsig.RSASHA256SignatureMethod &&
-		sp.SignatureMethod != dsig.RSASHA512SignatureMethod {
-		return fmt.Errorf("invalid signing method %s", sp.SignatureMethod)
-	}
-	signatureMethod := sp.SignatureMethod
-	signingContext := dsig.NewDefaultSigningContext(keyStore)
-	signingContext.Canonicalizer = dsig.MakeC14N10ExclusiveCanonicalizerWithPrefixList(canonicalizerPrefixList)
-	if err := signingContext.SetSignatureMethod(signatureMethod); err != nil {
+	signingContext, err := GetSigningContext(sp)
+	if err != nil {
 		return err
 	}
 
 	assertionEl := req.Element()
-
 	signedRequestEl, err := signingContext.SignEnveloped(assertionEl)
 	if err != nil {
 		return err
@@ -1361,7 +1376,7 @@ func (sp *ServiceProvider) MakeLogoutRequest(idpURL, nameID string) (*LogoutRequ
 			SPNameQualifier: sp.Metadata().EntityID,
 		},
 	}
-	if len(sp.SignatureMethod) > 0 {
+	if sp.SignatureMethod != "" {
 		if err := sp.SignLogoutRequest(&req); err != nil {
 			return nil, err
 		}
@@ -1475,7 +1490,7 @@ func (sp *ServiceProvider) MakeLogoutResponse(idpURL, logoutRequestID string) (*
 		},
 	}
 
-	if len(sp.SignatureMethod) > 0 {
+	if sp.SignatureMethod != "" {
 		if err := sp.SignLogoutResponse(&response); err != nil {
 			return nil, err
 		}
@@ -1572,31 +1587,12 @@ func (r *LogoutResponse) Post(relayState string) []byte {
 
 // SignLogoutResponse adds the `Signature` element to the `LogoutResponse`.
 func (sp *ServiceProvider) SignLogoutResponse(resp *LogoutResponse) error {
-	keyPair := tls.Certificate{
-		Certificate: [][]byte{sp.Certificate.Raw},
-		PrivateKey:  sp.Key,
-		Leaf:        sp.Certificate,
-	}
-	// TODO: add intermediates for SP
-	// for _, cert := range sp.Intermediates {
-	// 	keyPair.Certificate = append(keyPair.Certificate, cert.Raw)
-	// }
-	keyStore := dsig.TLSCertKeyStore(keyPair)
-
-	if sp.SignatureMethod != dsig.RSASHA1SignatureMethod &&
-		sp.SignatureMethod != dsig.RSASHA256SignatureMethod &&
-		sp.SignatureMethod != dsig.RSASHA512SignatureMethod {
-		return fmt.Errorf("invalid signing method %s", sp.SignatureMethod)
-	}
-	signatureMethod := sp.SignatureMethod
-	signingContext := dsig.NewDefaultSigningContext(keyStore)
-	signingContext.Canonicalizer = dsig.MakeC14N10ExclusiveCanonicalizerWithPrefixList(canonicalizerPrefixList)
-	if err := signingContext.SetSignatureMethod(signatureMethod); err != nil {
+	signingContext, err := GetSigningContext(sp)
+	if err != nil {
 		return err
 	}
 
 	assertionEl := resp.Element()
-
 	signedRequestEl, err := signingContext.SignEnveloped(assertionEl)
 	if err != nil {
 		return err

service_provider_test.go

@@ -267,39 +267,53 @@ func TestSPCanProducePostRequest(t *testing.T) {
 }
 
 func TestSPCanProduceSignedRequestRedirectBinding(t *testing.T) {
-	test := NewServiceProviderTest(t)
-	TimeNow = func() time.Time {
-		rv, _ := time.Parse("Mon Jan 2 15:04:05.999999999 UTC 2006", "Mon Dec 1 01:31:21.123456789 UTC 2015")
-		return rv
-	}
-	Clock = dsig.NewFakeClockAt(TimeNow())
-	s := ServiceProvider{
-		Key:             test.Key,
-		Certificate:     test.Certificate,
-		MetadataURL:     mustParseURL("https://15661444.ngrok.io/saml2/metadata"),
-		AcsURL:          mustParseURL("https://15661444.ngrok.io/saml2/acs"),
-		IDPMetadata:     &EntityDescriptor{},
-		SignatureMethod: dsig.RSASHA1SignatureMethod,
+	for _, alg := range []string{
+		dsig.RSASHA1SignatureMethod,
+		dsig.RSASHA256SignatureMethod,
+		dsig.RSASHA384SignatureMethod,
+		dsig.RSASHA512SignatureMethod,
+		dsig.ECDSASHA1SignatureMethod,
+		dsig.ECDSASHA256SignatureMethod,
+		dsig.ECDSASHA384SignatureMethod,
+		dsig.ECDSASHA512SignatureMethod,
+	} {
+		testName := strings.Split(alg, "#")[1]
+		t.Run(testName, func(t *testing.T) {
+			test := NewServiceProviderTest(t)
+			TimeNow = func() time.Time {
+				rv, _ := time.Parse("Mon Jan 2 15:04:05.999999999 UTC 2006", "Mon Dec 1 01:31:21.123456789 UTC 2015")
+				return rv
+			}
+			Clock = dsig.NewFakeClockAt(TimeNow())
+			s := ServiceProvider{
+				Key:             test.Key,
+				Certificate:     test.Certificate,
+				MetadataURL:     mustParseURL("https://15661444.ngrok.io/saml2/metadata"),
+				AcsURL:          mustParseURL("https://15661444.ngrok.io/saml2/acs"),
+				IDPMetadata:     &EntityDescriptor{},
+				SignatureMethod: dsig.RSASHA1SignatureMethod,
+			}
+			err := xml.Unmarshal(test.IDPMetadata, &s.IDPMetadata)
+			assert.Check(t, err)
+
+			redirectURL, err := s.MakeRedirectAuthenticationRequest("relayState")
+			assert.Assert(t, err)
+			// Signature we check against in the query string was validated with
+			// https://www.samltool.com/validate_authn_req.php . Once we add
+			// support for validating signed AuthN requests in the IDP implementation
+			// we can switch to testing using that.
+			golden.Assert(t, redirectURL.RawQuery, t.Name()+"_queryString")
+
+			decodedRequest, err := testsaml.ParseRedirectRequest(redirectURL)
+			assert.Check(t, err)
+			assert.Check(t, is.Equal("idp.testshib.org",
+				redirectURL.Host))
+			assert.Check(t, is.Equal("/idp/profile/SAML2/Redirect/SSO",
+				redirectURL.Path))
+			// Contains no enveloped signature
+			golden.Assert(t, string(decodedRequest), t.Name()+"_decodedRequest")
+		})
 	}
-	err := xml.Unmarshal(test.IDPMetadata, &s.IDPMetadata)
-	assert.Check(t, err)
-
-	redirectURL, err := s.MakeRedirectAuthenticationRequest("relayState")
-	assert.Check(t, err)
-	// Signature we check against in the query string was validated with
-	// https://www.samltool.com/validate_authn_req.php . Once we add
-	// support for validating signed AuthN requests in the IDP implementation
-	// we can switch to testing using that.
-	golden.Assert(t, redirectURL.RawQuery, t.Name()+"_queryString")
-
-	decodedRequest, err := testsaml.ParseRedirectRequest(redirectURL)
-	assert.Check(t, err)
-	assert.Check(t, is.Equal("idp.testshib.org",
-		redirectURL.Host))
-	assert.Check(t, is.Equal("/idp/profile/SAML2/Redirect/SSO",
-		redirectURL.Path))
-	// Contains no enveloped signature
-	golden.Assert(t, string(decodedRequest), t.Name()+"_decodedRequest")
 }
 
 func TestSPCanProduceSignedRequestPostBinding(t *testing.T) {

testdata/TestSPCanProduceSignedRequestRedirectBinding_decodedRequest renamed to testdata/TestSPCanProduceSignedRequestRedirectBinding/ecdsa-sha1_decodedRequest

@@ -0,0 +1 @@
+<samlp:AuthnRequest xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="id-00020406080a0c0e10121416181a1c1e20222426" Version="2.0" IssueInstant="2015-12-01T01:31:21.123Z" Destination="https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO" AssertionConsumerServiceURL="https://15661444.ngrok.io/saml2/acs" ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"><saml:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">https://15661444.ngrok.io/saml2/metadata</saml:Issuer><samlp:NameIDPolicy Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient" AllowCreate="true"/></samlp:AuthnRequest>

testdata/TestSPCanProduceSignedRequestRedirectBinding_queryString renamed to testdata/TestSPCanProduceSignedRequestRedirectBinding/ecdsa-sha1_queryString

@@ -0,0 +1 @@
+SAMLRequest=nFJNb9swDP0rhu62RNU1CqE2kDUYFqBbgzjbYTfVZhNituSJ9Lb%2B%2B8Fph2WXDOiV4uP70LtlPw6TW81yDDv8PiNL9mscArvloVZzCi56JnbBj8hOOteuPt47WxjnmTEJxaDOINNlzJSixC4OKtusa0V9boyxpjSVuTHedAbBgIUSKrgBDx2gNdba0lYq%2B4KJKYZa2cKobMM84yaw%2BCC1sgauc7C5gb0BdwXOQgH26qvK1shCwcsJeRSZ2GlN%2FVQIsvCRHouYDstATyk%2B0YB60Wr1DntK2Ilu2weVrf5YvYuB5xFTi%2BkHdfh5d%2F%2F3KlxXFZRlWYRDit8KinoJxGrfscq2r8bfUegpHC6n9PiyxO7Dfr%2FNtw%2FtXjWnn3In2yl7H9Po5fKRZUJ9%2FnRadRiE5Fk1%2FxM7ovjei7%2FVZ3zNa00%2B%2BRE3620cqHt%2BgwZJPjBhEJWthiH%2BvEvoBWslaUalmxfKf8vY%2FA4AAP%2F%2F&RelayState=relayState&SigAlg=http%3A%2F%2Fwww.w3.org%2F2000%2F09%2Fxmldsig%23rsa-sha1&Signature=WqMc7vKRJVNXwNHJmTemdfw5OML2XkLntYw%2FzwKoLMfavV%2FYy6fBP0GeGYlJVMweZBvbpjwoe%2BgpRkUCHKDUgixCG7hPi41p6MpQC%2Fp7ExTW5plvlS97iVAOvaF5V1MjvQCgBNKYnKNnvwAuxK%2Bu3N4rZjwGM%2F4JGgjJ5pannFQ%3D

testdata/TestSPCanProduceSignedRequestRedirectBinding/ecdsa-sha256_decodedRequest

@@ -0,0 +1 @@
+<samlp:AuthnRequest xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="id-00020406080a0c0e10121416181a1c1e20222426" Version="2.0" IssueInstant="2015-12-01T01:31:21.123Z" Destination="https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO" AssertionConsumerServiceURL="https://15661444.ngrok.io/saml2/acs" ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"><saml:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">https://15661444.ngrok.io/saml2/metadata</saml:Issuer><samlp:NameIDPolicy Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient" AllowCreate="true"/></samlp:AuthnRequest>