Improve Microsoft endpoint validation (#38180)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #13698

If some of the following don't apply, delete the relevant line.

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.

- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] If paths of existing endpoints are modified without backwards
compatibility, checked the frontend/CLI for any necessary changes

- [x] Added/updated automated tests
- [x] Where appropriate, [automated tests simulate multiple hosts and
test for host
isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing)
(updates to one hosts's records do not affect another)

- [x] QA'd all new/changed functionality manually

Author: JordanMontgomery

changes/13698-windows-mdm

@@ -0,0 +1 @@
+* Improved SOAP message validation on Windows MDM endpoints

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/DATA-DOG/go-sqlmock v1.5.0
 	github.com/Masterminds/semver v1.5.0
 	github.com/Masterminds/semver/v3 v3.3.1
+	github.com/MicahParks/jwkset v0.11.0
 	github.com/RobotsAndPencils/buford v0.14.0
 	github.com/VividCortex/mysqlerr v0.0.0-20170204212430-6c6b55f8796f
 	github.com/WatchBeam/clock v0.0.0-20170901150240-b08e6b4da7ea

go.sum

@@ -48,6 +48,8 @@ github.com/Masterminds/semver/v3 v3.3.1 h1:QtNSWtVZ3nBfk8mAOu/B6v7FMJ+NHTIgUPi7r
 github.com/Masterminds/semver/v3 v3.3.1/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM=
 github.com/Masterminds/sprig v2.22.0+incompatible h1:z4yfnGrZ7netVz+0EDJ0Wi+5VZCSYp4Z0m2dk6cEM60=
 github.com/Masterminds/sprig v2.22.0+incompatible/go.mod h1:y6hNFY5UBTIWBxnzTeuNhlNS5hqE0NB0E6fgfo2Br3o=
+github.com/MicahParks/jwkset v0.11.0 h1:yc0zG+jCvZpWgFDFmvs8/8jqqVBG9oyIbmBtmjOhoyQ=
+github.com/MicahParks/jwkset v0.11.0/go.mod h1:U2oRhRaLgDCLjtpGL2GseNKGmZtLs/3O7p+OZaL5vo0=
 github.com/Microsoft/go-winio v0.5.2/go.mod h1:WpS1mjBmmwHBEWmogvA2mj8546UReBk4v8QkMxJ6pZY=
 github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
 github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=

pkg/mdm/mdmtest/windows.go

@@ -2,13 +2,16 @@ package mdmtest
 
 import (
 	"bytes"
+	"crypto/rsa"
 	"crypto/tls"
 	"encoding/base64"
 	"encoding/xml"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
 	"strconv"
+	"strings"
 
 	"github.com/fleetdm/fleet/v4/pkg/fleethttp"
 	"github.com/fleetdm/fleet/v4/server/fleet"
@@ -38,6 +41,10 @@ type TestWindowsMDMClient struct {
 	// queuedCommandResponses tracks the commands that will be sent next
 	// time the device responds to the server.
 	queuedCommandResponses map[string]fleet.SyncMLCmd
+	// jwtSigningKey is the key used to sign JWTs
+	jwtSigningKey *rsa.PrivateKey
+	// jwtSigningKeyID is the ID to report in the header for the signing key
+	jwtSigningKeyID string
 }
 
 // This is a test-only enrollment type to force erroneous behavior.
@@ -55,6 +62,19 @@ func TestWindowsMDMClientDebug() TestWindowsMDMClientOption {
 	}
 }
 
+func TestWindowsMDMClientNotInOOBE() TestWindowsMDMClientOption {
+	return func(c *TestWindowsMDMClient) {
+		c.notInOOBE = true
+	}
+}
+
+func TestWindowsMDMClientWithSigningKey(signingKey *rsa.PrivateKey, signingKeyID string) TestWindowsMDMClientOption {
+	return func(c *TestWindowsMDMClient) {
+		c.jwtSigningKey = signingKey
+		c.jwtSigningKeyID = signingKeyID
+	}
+}
+
 func NewTestMDMClientWindowsProgramatic(serverURL string, orbitNodeKey string, opts ...TestWindowsMDMClientOption) *TestWindowsMDMClient {
 	return newTestMDMClient(serverURL, fleet.WindowsMDMProgrammaticEnrollmentType, orbitNodeKey, opts...)
 }
@@ -364,9 +384,20 @@ YioVozr1IWYySwWVzMf/SUwKZkKJCAJmSVcixE+4kxPkyPGyauIrN3wWC0zb+mjF
 </s:Envelope>
 `)
 
-	if _, err := c.request(microsoft_mdm.MDE2EnrollPath, enrollReq); err != nil {
+	resp, err := c.request(microsoft_mdm.MDE2EnrollPath, enrollReq)
+	if err != nil {
+		return err
+	}
+
+	// Check for SOAP fault
+	defer resp.Body.Close()
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
 		return err
 	}
+	if strings.Contains(string(body), "s:fault") {
+		return fmt.Errorf("enroll request returned SOAP fault: %s", string(body))
+	}
 
 	return nil
 }
@@ -406,10 +437,21 @@ func (c *TestWindowsMDMClient) Discovery() error {
 
 	// TODO: parse the response and store the policy and enroll endpoints instead
 	// of hardcoding them to truly test that the server is behaving as expected.
-	if _, err := c.request(microsoft_mdm.MDE2DiscoveryPath, discoveryReq); err != nil {
+	resp, err := c.request(microsoft_mdm.MDE2DiscoveryPath, discoveryReq)
+	if err != nil {
 		return err
 	}
 
+	// Check for SOAP fault
+	defer resp.Body.Close()
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return err
+	}
+	if strings.Contains(string(body), "s:fault") {
+		return fmt.Errorf("discovery request returned SOAP fault: %s", string(body))
+	}
+
 	return nil
 }
 
@@ -457,9 +499,19 @@ func (c *TestWindowsMDMClient) Policy() error {
 
 	// TODO: store the policy requirements to generate a certificate and generate
 	// one on the fly using them instead of using hardcoded values.
-	if _, err := c.request(microsoft_mdm.MDE2PolicyPath, policyReq); err != nil {
+	resp, err := c.request(microsoft_mdm.MDE2PolicyPath, policyReq)
+	if err != nil {
+		return err
+	}
+	// Check for SOAP fault
+	defer resp.Body.Close()
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
 		return err
 	}
+	if strings.Contains(string(body), "s:fault") {
+		return fmt.Errorf("policy request returned SOAP fault: %s", string(body))
+	}
 
 	return nil
 }
@@ -493,9 +545,13 @@ func (c *TestWindowsMDMClient) getToken() (binarySecToken string, tokenValueType
 			"unique_name": "foo_bar",
 			"scp":         "mdm_delegation",
 		}
+		if c.jwtSigningKey == nil || c.jwtSigningKeyID == "" {
+			return "", "", errors.New("jwt signing key is not set")
+		}
 
-		token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
-		tokenString, err := token.SignedString([]byte("foo"))
+		token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims)
+		token.Header["kid"] = c.jwtSigningKeyID
+		tokenString, err := token.SignedString(c.jwtSigningKey)
 		if err != nil {
 			return "", "", err
 		}

server/mdm/microsoft/wstep.go

@@ -13,11 +13,14 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
+	"os"
 	"strconv"
 	"strings"
 	"time"
 
+	"github.com/MicahParks/jwkset"
 	"github.com/fleetdm/fleet/v4/server"
+	"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
 	"github.com/fleetdm/fleet/v4/server/mdm/microsoft/syncml"
 	"github.com/fleetdm/fleet/v4/server/mdm/nanomdm/cryptoutil"
 	"github.com/golang-jwt/jwt/v4"
@@ -215,7 +218,7 @@ func (m *manager) GetSTSAuthTokenUPNClaim(tokenStr string) (string, error) {
 	}
 
 	// Since we used the private key to sign the tokens, we use the public counterpart to verify the signature
-	token, err := jwt.ParseWithClaims(tokenStr, &STSClaims{}, func(token *jwt.Token) (interface{}, error) {
+	token, err := jwt.ParseWithClaims(tokenStr, &STSClaims{}, func(token *jwt.Token) (any, error) {
 		return m.identityCert.PublicKey, nil
 	})
 	if err != nil {
@@ -235,27 +238,71 @@ func (m *manager) GetSTSAuthTokenUPNClaim(tokenStr string) (string, error) {
 
 // GetAzureAuthTokenClaims validates the given Azure AD token and returns
 // UPN, TenantID, UniqueName, DeviceID
-func GetAzureAuthTokenClaims(tokenStr string) (AzureData, error) {
+func GetAzureAuthTokenClaims(ctx context.Context, tokenStr string) (AzureData, error) {
 	if len(tokenStr) == 0 {
-		return AzureData{}, errors.New("invalid STS token")
+		return AzureData{}, ctxerr.New(ctx, "invalid STS token")
 	}
 
 	// Decode base64 token
 	tokenBytes, err := base64.StdEncoding.DecodeString(tokenStr)
 	if err != nil {
-		return AzureData{}, errors.New("invalid Azure JWT token")
+		return AzureData{}, ctxerr.Wrap(ctx, err, "invalid Azure JWT token")
 	}
 
 	// Validate token format (header.payload.signature)
 	parts := bytes.Split(tokenBytes, []byte("."))
 	if len(parts) != 3 {
-		return AzureData{}, errors.New("invalid Azure JWT format")
+		return AzureData{}, ctxerr.New(ctx, "invalid Azure JWT format")
 	}
 
 	// Parse JWT token
-	token, _, err := new(jwt.Parser).ParseUnverified(string(tokenBytes), jwt.MapClaims{})
+	jwksURI := "https://login.microsoftonline.com/common/discovery/v2.0/keys"
+	var token *jwt.Token
+	FLEET_DEV_AZURE_JWT_JWKS_URI := os.Getenv("FLEET_DEV_AZURE_JWT_JWKS_URI")
+	if FLEET_DEV_AZURE_JWT_JWKS_URI != "" {
+		jwksURI = FLEET_DEV_AZURE_JWT_JWKS_URI
+	}
+
+	keys, err := jwkset.NewDefaultHTTPClient([]string{jwksURI})
+	if err != nil {
+		return AzureData{}, ctxerr.Wrap(ctx, err, "failed to retrieve Azure JWT signing keys")
+	}
+	token, err = jwt.Parse(string(tokenBytes), func(token *jwt.Token) (any, error) {
+		tokenAlg, ok := token.Header["alg"]
+		if !ok {
+			return nil, errors.New("Azure JWT missing alg header")
+		}
+		tokenAlgStr, ok := tokenAlg.(string)
+		if !ok {
+			return nil, errors.New("invalid alg header in Azure JWT")
+		}
+
+		kid, ok := token.Header["kid"]
+		if !ok {
+			return nil, errors.New("Azure JWT missing kid header")
+		}
+		kidStr, ok := kid.(string)
+		if !ok {
+			return nil, errors.New("invalid kid header in Azure JWT")
+		}
+
+		key, err := keys.KeyRead(ctx, kidStr)
+		if err != nil {
+			if errors.Is(err, jwkset.ErrKeyNotFound) {
+				return nil, fmt.Errorf("Azure JWT signed by unknown key: %w", err)
+			}
+			return nil, fmt.Errorf("failed to retrieve Azure JWT signing key: %w", err)
+		}
+
+		// Alg is optional in the JWK but if present must match the token
+		keyAlg := key.Marshal().ALG.String()
+		if keyAlg != "" && keyAlg != tokenAlgStr {
+			return nil, fmt.Errorf("Azure JWT signing key algorithm mismatch: expected %s from key, got %s", keyAlg, tokenAlgStr)
+		}
+		return key.Key(), nil
+	})
 	if err != nil {
-		return AzureData{}, errors.New("parse error Azure JWT content")
+		return AzureData{}, ctxerr.Wrap(ctx, err, "parse error Azure JWT content")
 	}
 
 	// Parse JWT token
@@ -264,25 +311,25 @@ func GetAzureAuthTokenClaims(tokenStr string) (AzureData, error) {
 	// Get UPN claim
 	upnClaim, ok := claims["upn"].(string)
 	if !ok || len(upnClaim) == 0 {
-		return AzureData{}, errors.New("invalid UPN claim")
+		return AzureData{}, ctxerr.New(ctx, "invalid UPN claim")
 	}
 
 	// Get TenantID claim
 	tenantIDClaim, ok := claims["tid"].(string)
 	if !ok || len(tenantIDClaim) == 0 {
-		return AzureData{}, errors.New("invalid TenantID claim")
+		return AzureData{}, ctxerr.New(ctx, "invalid TenantID claim")
 	}
 
 	// Get UniqueName claim
 	uniqueNameClaim, ok := claims["unique_name"].(string)
 	if !ok {
-		return AzureData{}, errors.New("invalid UniqueName claim")
+		return AzureData{}, ctxerr.New(ctx, "invalid UniqueName claim")
 	}
 
 	// Get SCP claim
 	azureSCPClaim, ok := claims["scp"].(string)
 	if !ok || azureSCPClaim != "mdm_delegation" {
-		return AzureData{}, errors.New("invalid SCP claim")
+		return AzureData{}, ctxerr.New(ctx, "invalid SCP claim")
 	}
 
 	return AzureData{

server/service/integration_logger_test.go

@@ -371,7 +371,7 @@ func (s *integrationLoggerTestSuite) TestWindowsMDMEnrollEmptyBinarySecurityToke
 	host := createOrbitEnrolledHost(t, "windows", "", s.ds)
 	mdmDevice := mdmtest.NewTestMDMClientWindowsEmptyBinarySecurityToken(s.server.URL, *host.OrbitNodeKey)
 	err = mdmDevice.Enroll()
-	require.NoError(t, err)
+	require.Error(t, err)
 
 	t.Log(s.buf.String())
 
@@ -394,12 +394,11 @@ func (s *integrationLoggerTestSuite) TestWindowsMDMEnrollEmptyBinarySecurityToke
 			require.Equal(t, "info", m["level"])
 			require.Equal(t, "binarySecurityToken is empty", m["soap_fault"])
 		case microsoft_mdm.MDE2EnrollPath:
-			require.Equal(t, "info", m["level"])
-			require.Equal(t, "binarySecurityToken is empty", m["soap_fault"])
-			foundEnroll = true
+			foundEnroll = false
 		}
 	}
 	require.True(t, foundDiscovery)
 	require.True(t, foundPolicy)
-	require.True(t, foundEnroll)
+	// Will not enroll due to soap fault on prior request
+	require.False(t, foundEnroll)
 }

server/service/integration_mdm_lifecycle_test.go

@@ -326,6 +326,16 @@ func (s *integrationMDMTestSuite) TestTurnOnLifecycleEventsWindows() {
 				// the ack of the message should be the only returned command
 				require.Len(t, cmds, 1)
 
+				// Simulate the host having fleetd installed and reporting back in as un-enrolled
+				mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
+					_, err := q.ExecContext(context.Background(), `
+	              UPDATE host_mdm
+	              SET enrolled = 0, server_url = ''
+	              WHERE host_id = ?
+		`, host.ID)
+					return err
+				})
+
 				// re-enroll
 				require.NoError(t, device.Enroll())
 			},
@@ -358,13 +368,28 @@ func (s *integrationMDMTestSuite) TestTurnOnLifecycleEventsWindows() {
 					&orbitScriptResp,
 				)
 
+				// Simulate the host having fleetd installed after being wiped and reporting back in as un-enrolled
+				mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
+					_, err := q.ExecContext(context.Background(), `
+	              UPDATE host_mdm
+	              SET enrolled = 0, server_url = ''
+	              WHERE host_id = ?
+		`, host.ID)
+					return err
+				})
+
 				require.NoError(t, device.Enroll())
 			},
 		},
 		{
 			"host turns on MDM features out of the blue",
 			func(t *testing.T, host *fleet.Host, device *mdmtest.TestWindowsMDMClient) {
-				require.NoError(t, device.Enroll())
+				if strings.Contains(t.Name(), "automatic") {
+					require.NoError(t, device.Enroll())
+				} else {
+					// A programatically-enrolled host that randomly turns on MDM after already enabled will get a SOAP fault
+					require.Error(t, device.Enroll())
+				}
 			},
 		},
 		{
@@ -437,7 +462,7 @@ func (s *integrationMDMTestSuite) TestTurnOnLifecycleEventsWindows() {
 				host := createOrbitEnrolledHost(t, "windows", "windows_automatic", s.ds)
 
 				azureMail := "foo.bar.baz@example.com"
-				device := mdmtest.NewTestMDMClientWindowsAutomatic(s.server.URL, azureMail)
+				device := mdmtest.NewTestMDMClientWindowsAutomatic(s.server.URL, azureMail, mdmtest.TestWindowsMDMClientWithSigningKey(s.jwtSigningKey, defaultFakeJWTKeyID))
 				device.HardwareID = host.UUID
 				device.DeviceID = host.UUID
 				require.NoError(t, device.Enroll())