Add a config to control if authgear verify the signature of saml elements from SP

docs/specs/saml.md

@@ -59,7 +59,7 @@ sequenceDiagram
 
 - The login endpoint `https://example.authgear.cloud/saml2/login/EXAMPLE_ID` is from the metadata.
 - The Assertion Consumer Service (ACS) URL `https://example.com/acs` is from `<AuthnRequest>` if provided, else it is the first item in `acs_urls` of the SP's configuration.
-- `Destination`, `Audience` and `Recipient` of the `<Response>` and SAML assertions will be set to the  Assertion Consumer Service URL by default, but these fields can be customized in the configuration of each SP.
+- `Destination`, `Audience` and `Recipient` of the `<Response>` and SAML assertions will be set to the Assertion Consumer Service URL by default, but these fields can be customized in the configuration of each SP.
 - In step [4], Authgear creates an IdP Session with the User Agent using cookies. `SessionIndex` in the `<Response>` of step [5] is composed by the ID of the IdP Session.
 - In step [5], the SP should memorize the authentication state. For example, by creating a session using cookies.
 
@@ -175,10 +175,10 @@ saml:
 
 ### <a id="1_5"></a> Support Pre-entering login id by SAML Subject in Auth UI
 
-
-
 - In the oidc flow, new login_hint type `login_id` is supported for prefilling login ids:
+
   - Supported login ids:
+
     - Email: https://authgear.com/login_hint?type=login_id&[email protected]
     - Phone: https://authgear.com/login_hint?type=login_id&phone=+123456
     - Username: https://authgear.com/login_hint?type=login_id&username=test
@@ -188,6 +188,7 @@ saml:
     - If `false` or other invalid value, the UI will not prevent the user to login to another account which is different than the one specified by the login_hint.
 
 - If a `<Subject>` element exist in the `<AuthnRequest>`, the `<NameID>` inside the subject will be used to prefill the login id in Auth UI. It will be mapped to a login_hint by the following rules:
+
   - If `nameid_format` (See the [NameID section](#3)) of the service provider is `urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified`, the login_hint will be generated according to `nameid_attribute_pointer`.
     - When `nameid_attribute_pointer` is `/email`, a login_hint of email `https://authgear.com/login_hint?type=login_id&[email protected]` will be generated.
     - When `nameid_attribute_pointer` is `/phone_number`, a login_hint of phone `https://authgear.com/login_hint?type=login_id&phone=+123456` will be generated.
@@ -199,7 +200,6 @@ saml:
 
   - If `IsPassive` is `true` in `<AuthnRequest>`, and `<Subject>` also exist, the above rules will also be applied. And if the current logged in user is not the user specified by the login_hint, `urn:oasis:names:tc:SAML:2.0:status:NoPassive` error will be returned.
 
-
 ### <a id="1_6"></a> The Login Endpoint
 
 - The URL is https://example.authgear.cloud/saml2/login/EXAMPLE_ID. Where `EXAMPLE_ID` is the `id` of the service provider as specified in the config `saml.service_providers[index].id`. As a result, each service provider will have a independent login endpoint.
@@ -539,11 +539,13 @@ We have no plan to support other bindings not mentioned at the moment.
   - The signatures are created according to [SAMLCore](https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf) Section 5.
   - Each project will have independent signing certs. New certs can be generated by the portal. To facilitate cert rotations, a maximum of two certs is allowed. However, only one single cert can be used in signing. The active signing cert is controlled by the config `saml.signing.key_id`.
   - Signing Algorithm ([SignatureMethod](https://www.w3.org/TR/xmldsig-core1/#sec-SignatureMethod)) is configurable with config `saml.signing.signature_method`. Please see below for supported values.
-- If Service Providers has signing certs uploaded, all requests and responses from the Service Provider will be validated against the provided certs.
+- If Service Providers has at least one signing certs uploaded, and the `signature_verification_enabled` config of that SP is set to `true`, all requests and responses from the Service Provider will be validated against the provided certs.
   - To facilitate cert rotations, two certs are allowed to be uploaded. When there are two certs, the requests and responses will be considered valid if its signature can be validated by any one of the uploaded certs.
 
 ### <a id="6_1"></a> Configs
 
+Configuring IdP signing:
+
 ```yaml
 saml:
   signing:
@@ -555,6 +557,17 @@ saml:
   - `key_id`: Required. A string which points to a signing key secret inside the `saml.idp.signing` secret. See the below [Secrets] section for details.
   - `signature_method`: Optional. An enum which specifies the signing algorithm used to generate the signature. The only supported value is `http://www.w3.org/2001/04/xmldsig-more#rsa-sha256`. If not provided, it defaults to `http://www.w3.org/2001/04/xmldsig-more#rsa-sha256`. For details, see [xmldsig](https://www.w3.org/TR/xmldsig-core1/#sec-SignatureAlg).
 
+Configuring SP signature verification:
+
+```yaml
+saml:
+  service_providers:
+    - client_id: EXAMPLE_SP_ID
+      signature_verification_enabled: true
+```
+
+- `saml.service_providers[index].signature_verification_enabled`: Optional. Default `false`. If `true`, all SAML requests and responses from this service provider must be signed. And authgear will always verify the signature with the uploaded certificates.
+
 ### <a id="6_2"></a> Secrets
 
 Two secrets are defined for SAML signing:
@@ -602,8 +615,8 @@ Two secrets are defined for SAML signing:
   - `key`: Must be `saml.service_providers.signing`.
   - `data`: Required. A list of objects which represents the certs of one service provider. The object inside the list MUST contains the following fields:
     - `service_provider_id`: Required. The ID of the service provider which owns the certs in `certificates` below.
-    - `certificates`: Requried. A list of objects representing X.509 certs. The minimum number of items in the list is 0. The maximum number of items in the list is 2. When there is at least one item in the list, all requests or responses from the service provider which specified by the above `service_provider_id` will be rejected if it is not signed with one of the cert in the list. The objects inside the list MUST contains the following fields:
-        - `pem`: Required. The X.509 cert in pem format.
+    - `certificates`: Requried. A list of objects representing X.509 certs. The minimum number of items in the list is 0 if `signature_verification_enabled` of the pointed service provider is `false`, else the minimum number of items is 1. The maximum number of items in the list is 2. . The objects inside the list MUST contains the following fields:
+      - `pem`: Required. The X.509 cert in pem format.
 
 ## <a id="7"></a> Metadata
 
@@ -677,7 +690,7 @@ saml:
       audience: https://example.com
       destination: https://example.com/acs
       recipient: https://example.com/acs
-      acs_urls: 
+      acs_urls:
         - https://example.com/acs
       assertion_valid_duration: 1h
       slo_callback_url: https://app1.example.com/logout
@@ -704,7 +717,9 @@ saml:
 ## <a id="11"></a> Service Provider Support
 
 - Google Workspace
+
   - To support Web Browser SSO, we need to meet the SAML requirement here: https://support.google.com/a/answer/6330801?hl=en
+
     - Example config:
       ```yaml
       saml:

pkg/lib/config/saml.go

@@ -39,7 +39,8 @@ var _ = Schema.Add("SAMLServiceProviderConfig", `
 		"assertion_valid_duration":  { "$ref": "#/$defs/DurationString" },
 		"slo_enabled": { "type": "boolean" },
 		"slo_callback_url": { "type": "string", "format": "uri" },
-		"slo_binding": { "$ref": "#/$defs/SAMLSLOBinding" }
+		"slo_binding": { "$ref": "#/$defs/SAMLSLOBinding" },
+		"signature_verification_enabled": { "type": "boolean" }
 	},
 	"required": ["client_id", "acs_urls"],
 	"allOf": [
@@ -118,17 +119,18 @@ func (p SAMLNameIDAttributePointer) MustGetJSONPointer() jsonpointer.T {
 }
 
 type SAMLServiceProviderConfig struct {
-	ClientID               string                        `json:"client_id,omitempty"`
-	NameIDFormat           samlprotocol.SAMLNameIDFormat `json:"nameid_format,omitempty"`
-	NameIDAttributePointer SAMLNameIDAttributePointer    `json:"nameid_attribute_pointer,omitempty"`
-	AcsURLs                []string                      `json:"acs_urls,omitempty"`
-	Destination            string                        `json:"destination,omitempty"`
-	Recipient              string                        `json:"recipient,omitempty"`
-	Audience               string                        `json:"audience,omitempty"`
-	AssertionValidDuration DurationString                `json:"assertion_valid_duration,omitempty"`
-	SLOEnabled             bool                          `json:"slo_enabled,omitempty"`
-	SLOCallbackURL         string                        `json:"slo_callback_url,omitempty"`
-	SLOBinding             samlprotocol.SAMLBinding      `json:"slo_binding,omitempty"`
+	ClientID                     string                        `json:"client_id,omitempty"`
+	NameIDFormat                 samlprotocol.SAMLNameIDFormat `json:"nameid_format,omitempty"`
+	NameIDAttributePointer       SAMLNameIDAttributePointer    `json:"nameid_attribute_pointer,omitempty"`
+	AcsURLs                      []string                      `json:"acs_urls,omitempty"`
+	Destination                  string                        `json:"destination,omitempty"`
+	Recipient                    string                        `json:"recipient,omitempty"`
+	Audience                     string                        `json:"audience,omitempty"`
+	AssertionValidDuration       DurationString                `json:"assertion_valid_duration,omitempty"`
+	SLOEnabled                   bool                          `json:"slo_enabled,omitempty"`
+	SLOCallbackURL               string                        `json:"slo_callback_url,omitempty"`
+	SLOBinding                   samlprotocol.SAMLBinding      `json:"slo_binding,omitempty"`
+	SignatureVerificationEnabled bool                          `json:"signature_verification_enabled,omitempty"`
 }
 
 func (c *SAMLServiceProviderConfig) SetDefaults() {

pkg/lib/config/secret.go

@@ -250,6 +250,15 @@ func (c *SecretConfig) validateSAMLSigningKey(ctx *validation.Context, keyID str
 	ctx.EmitErrorMessage(fmt.Sprintf("saml idp signing key '%s' does not exist", keyID))
 }
 
+func (c *SecretConfig) validateSAMLServiceProviderCerts(ctx *validation.Context, sp *SAMLServiceProviderConfig) {
+	_, data, _ := c.LookupDataWithIndex(SAMLSpSigningMaterialsKey)
+	signingMaterials, _ := data.(*SAMLSpSigningMaterials)
+	certs, ok := signingMaterials.Resolve(sp)
+	if !ok || len(certs.Certificates) < 1 {
+		ctx.EmitErrorMessage(fmt.Sprintf("certificates of saml sp '%s' is not configured", sp.GetID()))
+	}
+}
+
 func (c *SecretConfig) Validate(appConfig *AppConfig) error {
 	ctx := &validation.Context{}
 
@@ -290,6 +299,11 @@ func (c *SecretConfig) Validate(appConfig *AppConfig) error {
 
 	if len(appConfig.SAML.ServiceProviders) > 0 {
 		c.validateRequire(ctx, SAMLIdpSigningMaterialsKey, "saml idp signing key materials")
+		for _, sp := range appConfig.SAML.ServiceProviders {
+			if sp.SignatureVerificationEnabled {
+				c.validateSAMLServiceProviderCerts(ctx, sp)
+			}
+		}
 	}
 	if appConfig.SAML.Signing.KeyID != "" {
 		c.validateSAMLSigningKey(ctx, appConfig.SAML.Signing.KeyID)

pkg/lib/config/testdata/config_tests.yaml

@@ -1201,6 +1201,7 @@ config:
         slo_enabled: true
         slo_callback_url: https://authgear.cloud/slo
         slo_binding: urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect
+        signature_verification_enabled: true
 ---
 name: saml-signing
 error: null

pkg/lib/saml/service.go

@@ -563,10 +563,13 @@ func (s *Service) IssueLogoutResponse(
 func (s *Service) VerifyEmbeddedSignature(
 	sp *config.SAMLServiceProviderConfig,
 	samlElementXML string) error {
+	if !sp.SignatureVerificationEnabled {
+		return nil
+	}
 	certs, ok := s.SAMLSpSigningMaterials.Resolve(sp)
 	if !ok || len(certs.Certificates) == 0 {
-		// Signing cert not configured, nothing to verify
-		return nil
+		// This should be prevented by config validation. Therefore it is a programming error.
+		panic(fmt.Errorf("SP certificates not configured but signature verification is required"))
 	}
 	certificateStore := &dsig.MemoryX509CertificateStore{
 		Roots: slice.Map(certs.Certificates, func(c config.X509Certificate) *x509.Certificate {
@@ -601,10 +604,13 @@ func (s *Service) VerifyExternalSignature(
 	sigAlg string,
 	relayState string,
 	signature string) error {
+	if !sp.SignatureVerificationEnabled {
+		return nil
+	}
 	certs, ok := s.SAMLSpSigningMaterials.Resolve(sp)
 	if !ok || len(certs.Certificates) == 0 {
-		// Signing cert not configured, nothing to verify
-		return nil
+		// This should be prevented by config validation. Therefore it is a programming error.
+		panic(fmt.Errorf("SP certificates not configured but signature verification is required"))
 	}
 
 	q := url.Values{}
@@ -613,7 +619,7 @@ func (s *Service) VerifyExternalSignature(
 	} else if el.SAMLResponse != "" {
 		q.Set("SAMLResponse", el.SAMLResponse)
 	} else {
-		panic("no signed element")
+		panic(fmt.Errorf("no signed element"))
 	}
 	q.Set("RelayState", relayState)
 	q.Set("SigAlg", sigAlg)
@@ -672,7 +678,7 @@ func (s *Service) ConstructSignedQueryParameters(
 	} else if el.SAMLRequest != "" {
 		q.Set("SAMLRequest", el.SAMLRequest)
 	} else {
-		panic("nothing to sign: SAMLResponse and SAMLRequest are both empty")
+		panic(fmt.Errorf("nothing to sign: SAMLResponse and SAMLRequest are both empty"))
 	}
 	q.Set("RelayState", relayState)
 	q.Set("SigAlg", string(s.SAMLConfig.Signing.SignatureMethod))
@@ -836,7 +842,7 @@ func (s *Service) idpSigningContext() (*dsig.SigningContext, error) {
 	// Create a cert chain based off of the IDP cert and its intermediates.
 	activeCert, ok := s.SAMLIdpSigningMaterials.FindSigningCert(s.SAMLConfig.Signing.KeyID)
 	if !ok {
-		panic("unexpected: cannot find the corresponding idp key by id")
+		panic(fmt.Errorf("unexpected: cannot find the corresponding idp key by id"))
 	}
 
 	var signingContext *dsig.SigningContext
@@ -883,7 +889,7 @@ func (s Service) recordSessionParticipant(
 			return err
 		}
 	default:
-		panic("unexpected session type")
+		panic(fmt.Errorf("unexpected session type"))
 	}
 	return nil
 }

pkg/lib/saml/service_test.go

@@ -63,14 +63,15 @@ func TestSAMLService(t *testing.T) {
 	spID := "testsp"
 	loginEndpoint, _ := url.Parse("http://idp.local/login")
 	endpoints.EXPECT().SAMLLoginURL(spID).AnyTimes().Return(loginEndpoint)
-	sp := &config.SAMLServiceProviderConfig{
-		ClientID:     spID,
-		NameIDFormat: samlprotocol.SAMLNameIDFormatEmailAddress,
-		AcsURLs: []string{
-			"http://localhost/saml-test",
-		},
-	}
+
 	createService := func() *saml.Service {
+		sp := &config.SAMLServiceProviderConfig{
+			ClientID:     spID,
+			NameIDFormat: samlprotocol.SAMLNameIDFormatEmailAddress,
+			AcsURLs: []string{
+				"http://localhost/saml-test",
+			},
+		}
 		return &saml.Service{
 			Clock: clk,
 			AppID: config.AppID("test"),
@@ -217,6 +218,8 @@ func TestSAMLService(t *testing.T) {
 					},
 				},
 			}
+			sp := svc.SAMLConfig.ServiceProviders[0]
+			sp.SignatureVerificationEnabled = true
 
 			err := svc.VerifyEmbeddedSignature(sp, requestXml)
 			So(err, ShouldBeNil)
@@ -234,6 +237,8 @@ func TestSAMLService(t *testing.T) {
 					},
 				},
 			}
+			sp := svc.SAMLConfig.ServiceProviders[0]
+			sp.SignatureVerificationEnabled = true
 
 			err := svc.VerifyEmbeddedSignature(sp, requestXml)
 			expectedErr := &samlprotocol.InvalidSignatureError{}
@@ -271,6 +276,8 @@ func TestSAMLService(t *testing.T) {
 					},
 				},
 			}
+			sp := svc.SAMLConfig.ServiceProviders[0]
+			sp.SignatureVerificationEnabled = true
 			err := svc.VerifyExternalSignature(
 				sp,
 				&saml.SAMLElementSigned{
@@ -294,6 +301,8 @@ func TestSAMLService(t *testing.T) {
 					},
 				},
 			}
+			sp := svc.SAMLConfig.ServiceProviders[0]
+			sp.SignatureVerificationEnabled = true
 
 			err := svc.VerifyExternalSignature(
 				sp,