From f603cebc8f43879c64778d310fb487377c891ba6 Mon Sep 17 00:00:00 2001 From: Richard Gomez Date: Wed, 9 Oct 2024 14:37:08 -0400 Subject: [PATCH] feat(azure): finish pending items for review --- pkg/detectors/azure_entra/common.go | 58 +++++++++- pkg/detectors/azure_entra/common_test.go | 39 +++++++ .../azure_entra/serviceprincipal/sp.go | 3 +- .../azure_entra/serviceprincipal/v1/spv1.go | 9 +- .../azure_entra/serviceprincipal/v2/spv2.go | 108 ++---------------- pkg/detectors/uuids.txt | 37 ------ 6 files changed, 113 insertions(+), 141 deletions(-) delete mode 100644 pkg/detectors/uuids.txt diff --git a/pkg/detectors/azure_entra/common.go b/pkg/detectors/azure_entra/common.go index b357e124ee84..9e4a14a2d433 100644 --- a/pkg/detectors/azure_entra/common.go +++ b/pkg/detectors/azure_entra/common.go @@ -1,8 +1,10 @@ package azure_entra import ( - "context" "fmt" + "github.com/trufflesecurity/trufflehog/v3/pkg/cache/simple" + "github.com/trufflesecurity/trufflehog/v3/pkg/context" + "golang.org/x/sync/singleflight" "io" "net/http" "strings" @@ -21,7 +23,11 @@ var ( // https://learn.microsoft.com/en-us/partner-center/account-settings/find-ids-and-domain-names#find-the-microsoft-azure-ad-tenant-id-and-primary-domain-name // https://learn.microsoft.com/en-us/microsoft-365/admin/setup/domains-faq?view=o365-worldwide#why-do-i-have-an--onmicrosoft-com--domain tenantIdPat = regexp.MustCompile(fmt.Sprintf( - `(?i)(?:login\.microsoftonline\.com/|sts\.windows\.net/|(?:t[ae]n[ae]nt(?:[ ._-]?id)?|\btid)(?:.|\s){0,45}?)(%s)`, uuidStr, + //language=regexp + `(?i)(?:(?:login\.microsoftonline\.com/|(?:login|sts)\.windows\.net/|(?:t[ae]n[ae]nt(?:[ ._-]?id)?|\btid)(?:.|\s){0,60}?)(%s)|https?://(%s)|X-AnchorMailbox(?:.|\s){0,60}?@(%s))`, + uuidStr, + uuidStr, + uuidStr, )) tenantOnMicrosoftPat = regexp.MustCompile(`([\w-]+\.onmicrosoft\.com)`) @@ -34,7 +40,14 @@ func FindTenantIdMatches(data string) map[string]struct{} { uniqueMatches := make(map[string]struct{}) for _, match := range tenantIdPat.FindAllStringSubmatch(data, -1) { - m := strings.ToLower(match[1]) + var m string + if match[1] != "" { + m = strings.ToLower(match[1]) + } else if match[2] != "" { + m = strings.ToLower(match[2]) + } else if match[3] != "" { + m = strings.ToLower(match[3]) + } if _, ok := detectors.UuidFalsePositives[detectors.FalsePositive(m)]; ok { continue } @@ -59,8 +72,31 @@ func FindClientIdMatches(data string) map[string]struct{} { return uniqueMatches } +var ( + tenantCache = simple.NewCache[bool]() + tenantGroup singleflight.Group +) + // TenantExists returns whether the tenant exists according to Microsoft's well-known OpenID endpoint. func TenantExists(ctx context.Context, client *http.Client, tenant string) bool { + // Use cached value where possible. + if tenantExists, isCached := tenantCache.Get(tenant); isCached { + return tenantExists + } + + // https://www.codingexplorations.com/blog/understanding-singleflight-in-golang-a-solution-for-eliminating-redundant-work + tenantExists, _, _ := tenantGroup.Do(tenant, func() (interface{}, error) { + result := queryTenant(ctx, client, tenant) + tenantCache.Set(tenant, result) + return result, nil + }) + + return tenantExists.(bool) +} + +func queryTenant(ctx context.Context, client *http.Client, tenant string) bool { + logger := ctx.Logger().WithName("azure").WithValues("tenant", tenant) + tenantUrl := fmt.Sprintf("https://login.microsoftonline.com/%s/.well-known/openid-configuration", tenant) req, err := http.NewRequestWithContext(ctx, http.MethodGet, tenantUrl, nil) if err != nil { @@ -68,10 +104,24 @@ func TenantExists(ctx context.Context, client *http.Client, tenant string) bool } res, err := client.Do(req) + if err != nil { + logger.Error(err, "Failed to check if tenant exists") + return false + } defer func() { _, _ = io.Copy(io.Discard, res.Body) _ = res.Body.Close() }() - return res.StatusCode == http.StatusOK + switch res.StatusCode { + case http.StatusOK: + return true + case http.StatusBadRequest: + logger.V(4).Info("Tenant does not exist.") + return false + default: + bodyBytes, _ := io.ReadAll(res.Body) + logger.Error(nil, "WARNING: Unexpected response when checking if tenant exists", "status_code", res.StatusCode, "body", string(bodyBytes)) + return false + } } diff --git a/pkg/detectors/azure_entra/common_test.go b/pkg/detectors/azure_entra/common_test.go index c619a554f850..a0797c73af0a 100644 --- a/pkg/detectors/azure_entra/common_test.go +++ b/pkg/detectors/azure_entra/common_test.go @@ -35,6 +35,14 @@ func runPatTest(t *testing.T, tests map[string]testCase, matchFunc func(data str func Test_FindTenantIdMatches(t *testing.T) { cases := map[string]testCase{ // Tenant ID + "audience": { + Input: `az offazure hyperv site create --location "eastus" --service-principal-identity-details \ + application-id="cbcfc473-97da-45dd-8a00-3612d1ddf35a" \ + audience="https://bced5192-08c4-4470-9a94-666fea59efb07/aadapp" `, + Expected: map[string]struct{}{ + "bced5192-08c4-4470-9a94-666fea59efb0": {}, + }, + }, "tenant": { Input: ` "cas.authn.azure-active-directory.login-url=https://login.microsoftonline.com/common/", "cas.authn.azure-active-directory.tenant=8e439f30-da7a-482c-bd23-e45d0a732000"`, @@ -99,6 +107,12 @@ tenant_id = "57aabdfc-6ce0-4828-94a2-9abe277892ec"`, "7bb339cb-e94c-4a85-884c-48ebd9bb28c3": {}, }, }, + "login.windows.net": { + Input: `az offazure hyperv site create --location "eastus" --service-principal-identity-details aad-authority="https://login.windows.net/7bb339cb-e94c-4a85-884c-48ebd9bb28c3" application-id="e9f013df-2a2a-4871-b766-e79867f30348" \'`, + Expected: map[string]struct{}{ + "7bb339cb-e94c-4a85-884c-48ebd9bb28c3": {}, + }, + }, "sts.windows.net": { Input: `{ "aud": "00000003-0000-0000-c000-000000000000", @@ -108,6 +122,20 @@ tenant_id = "57aabdfc-6ce0-4828-94a2-9abe277892ec"`, "974fde14-c3a4-481b-9b03-cfce182c3a07": {}, }, }, + "x-anchor-mailbox": { + // The tenantID can be encoded in this parameter. + // https://github.com/AzureAD/microsoft-authentication-library-for-python/blob/95a63a7fe97d91b99979e5bf78e03f6acf40a286/msal/application.py#L185-L186 + // https://github.com/silverhack/monkey365/blob/b3f43c4a2d014fcc3aae0a4103c8f2610fbb4980/core/utils/Get-MonkeySecCompBackendUri.ps1#L70 + Input: ` User-Agent: + - python-requests/2.31.0 + X-AnchorMailbox: + - Oid:2b9b0cb5-d707-42e3-9504-d9b76ac7bec5@86843c34-863b-44d3-bb14-4f14e7c0564d + x-client-current-telemetry: + - 4|84,3|`, + Expected: map[string]struct{}{ + "86843c34-863b-44d3-bb14-4f14e7c0564d": {}, + }, + }, // Tenant onmicrosoft.com "onmicrosoft tenant": { @@ -145,6 +173,17 @@ tenant_id = "57aabdfc-6ce0-4828-94a2-9abe277892ec"`, "12fc345b-0c67-4cde-8902-dabf2cad34b5": {}, }, }, + "newline": { + Input: ` {\n \"mode\": \"Manual\"\n },\n \"bootstrapProfile\": {\n \"artifactSource\": + \"Direct\"\n }\n },\n \"identity\": {\n \"type\": \"SystemAssigned\",\n + \ \"principalId\":\"00000000-0000-0000-0000-000000000001\",\n \"tenantId\": + \"d0a69dfd-9b9e-4833-9c33-c7903dd2e012\"\n },\n \"sku\": {\n \"name\": \"Base\",\n + \ \"tier\": \"Free\"\n }\n}" + headers:`, + Expected: map[string]struct{}{ + "d0a69dfd-9b9e-4833-9c33-c7903dd2e012": {}, + }, + }, // False positives "tid shouldn't match clientId": { diff --git a/pkg/detectors/azure_entra/serviceprincipal/sp.go b/pkg/detectors/azure_entra/serviceprincipal/sp.go index 07e7f841042c..d6a796ff91fd 100644 --- a/pkg/detectors/azure_entra/serviceprincipal/sp.go +++ b/pkg/detectors/azure_entra/serviceprincipal/sp.go @@ -15,6 +15,7 @@ import ( ) var ( + Description = "Azure is a cloud service offering a wide range of services including compute, analytics, storage, and networking. Azure credentials can be used to access and manage these services." ErrSecretInvalid = errors.New("invalid client secret provided") ErrSecretExpired = errors.New("the provided secret is expired") ErrTenantNotFound = errors.New("tenant not found") @@ -35,7 +36,6 @@ type TokenErrResponse struct { func VerifyCredentials(ctx context.Context, client *http.Client, tenantId string, clientId string, clientSecret string) (bool, map[string]string, error) { data := url.Values{} data.Set("client_id", clientId) - //data.Set("scope", "https://management.core.windows.net/.default") data.Set("scope", "https://graph.microsoft.com/.default") data.Set("client_secret", clientSecret) data.Set("grant_type", "client_credentials") @@ -91,6 +91,7 @@ func VerifyCredentials(ctx context.Context, client *http.Client, tenantId string case http.StatusBadRequest, http.StatusUnauthorized: // Error codes can be looked up by removing the `AADSTS` prefix. // https://login.microsoftonline.com/error?code=9002313 + // TODO: Handle AADSTS900382 (https://github.com/Azure/azure-sdk-for-js/issues/30557) d := errResp.Description switch { case strings.HasPrefix(d, "AADSTS700016:"): diff --git a/pkg/detectors/azure_entra/serviceprincipal/v1/spv1.go b/pkg/detectors/azure_entra/serviceprincipal/v1/spv1.go index cecdeac101ed..738f2005af9b 100644 --- a/pkg/detectors/azure_entra/serviceprincipal/v1/spv1.go +++ b/pkg/detectors/azure_entra/serviceprincipal/v1/spv1.go @@ -2,6 +2,7 @@ package v1 import ( "context" + "github.com/trufflesecurity/trufflehog/v3/pkg/detectors/azure_entra/serviceprincipal" "net/http" regexp "github.com/wasilibs/go-re2" @@ -29,7 +30,8 @@ var ( // TODO: Azure storage access keys and investigate other types of creds. // https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-client-creds-grant-flow#second-case-access-token-request-with-a-certificate // https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-client-creds-grant-flow#third-case-access-token-request-with-a-federated-credential - //clientSecretPat = regexp.MustCompile(`(?i)(?:secret|password| -p[ =]).{0,40}?([a-z0-9~@_\-[\]:.?]{32,34})`) + //clientSecretPat = regexp.MustCompile(`(?i)(?:secret|password| -p[ =]).{0,80}?([\w~@[\]:.?*/+=-]{31,34}`) + // TODO: Tighten this regex and replace it with above. secretPat = regexp.MustCompile(`(?i)(?:secret|password| -p[ =]).{0,80}[^A-Za-z0-9!#$%&()*+,\-./:;<=>?@[\\\]^_{|}~]([A-Za-z0-9!#$%&()*+,\-./:;<=>?@[\\\]^_{|}~]{31,34})[^A-Za-z0-9!#$%&()*+,\-./:;<=>?@[\\\]^_{|}~]`) ) @@ -61,6 +63,7 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result if client == nil { client = defaultClient } + // The handling logic is identical for both versions. processedResults := v2.ProcessData(ctx, clientSecrets, clientIds, tenantIds, verify, client) for _, result := range processedResults { results = append(results, result) @@ -72,6 +75,10 @@ func (s Scanner) Type() detectorspb.DetectorType { return detectorspb.DetectorType_Azure } +func (s Scanner) Description() string { + return serviceprincipal.Description +} + // region Helper methods. func findSecretMatches(data string) map[string]struct{} { uniqueMatches := make(map[string]struct{}) diff --git a/pkg/detectors/azure_entra/serviceprincipal/v2/spv2.go b/pkg/detectors/azure_entra/serviceprincipal/v2/spv2.go index a2fcb6d73421..29cba5a5228f 100644 --- a/pkg/detectors/azure_entra/serviceprincipal/v2/spv2.go +++ b/pkg/detectors/azure_entra/serviceprincipal/v2/spv2.go @@ -3,8 +3,7 @@ package v2 import ( "context" "errors" - "fmt" - "io" + logContext "github.com/trufflesecurity/trufflehog/v3/pkg/context" "net/http" "regexp" "strings" @@ -32,7 +31,6 @@ var ( defaultClient = common.SaneHttpClient() SecretPat = regexp.MustCompile(`(?:[^a-zA-Z0-9_~.-]|\A)([a-zA-Z0-9_~.-]{3}\dQ~[a-zA-Z0-9_~.-]{31,34})(?:[^a-zA-Z0-9_~.-]|\z)`) - //clientSecretPat = regexp.MustCompile(`(?:[^a-zA-Z0-9_~.-]|\A)([a-zA-Z0-9_~.-]{3}\dQ~[a-zA-Z0-9_~.-]{31,34})(?:[^a-zA-Z0-9_~.-]|\z)|(?:secret|password| -p[ =]).{0,80}[^A-Za-z0-9!#$%&()*+,\-./:;<=>?@[\\\]^_{|}~]([A-Za-z0-9!#$%&()*+,\-./:;<=>?@[\\\]^_{|}~]{31,34})[^A-Za-z0-9!#$%&()*+,\-./:;<=>?@[\\\]^_{|}~]`) ) func (s Scanner) Version() int { @@ -69,6 +67,7 @@ func (s Scanner) FromData(ctx context.Context, verify bool, data []byte) (result } func ProcessData(ctx context.Context, clientSecrets, clientIds, tenantIds map[string]struct{}, verify bool, client *http.Client) (results []detectors.Result) { + logCtx := logContext.AddLogger(ctx) invalidClientsForTenant := make(map[string]map[string]struct{}) SecretLoop: @@ -96,10 +95,13 @@ SecretLoop: } if verify { - if !isValidTenant(ctx, client, tenantId) { + if !azure_entra.TenantExists(logCtx, client, tenantId) { // Tenant doesn't exist delete(tenantIds, tenantId) continue + } else { + // Ensure this isn't attempted as a clientId. + delete(clientIds, tenantId) } isVerified, extraData, verificationErr := serviceprincipal.VerifyCredentials(ctx, client, tenantId, clientId, clientSecret) @@ -126,78 +128,6 @@ SecretLoop: r = createResult(tenantId, clientId, clientSecret, isVerified, extraData, verificationErr) break ClientLoop } - - // The result may be valid for another client/tenant. - // - // - //// https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-auth-code-flow#request-an-access-token-with-a-client_secret - //cred := auth.NewClientCredentialsConfig(clientId, clientSecret, tenantId) - //token, err := cred.ServicePrincipalToken() - //if err != nil { - // // This can only fail if a value is empty, which shouldn't be possible. - // continue - //} - // - //err = token.Refresh() - //if err != nil { - // var refreshError adal.TokenRefreshError - // if ok := errors.As(err, &refreshError); ok { - // resp := refreshError.Response() - // defer func() { - // // Ensure we drain the response body so this connection can be reused. - // _, _ = io.Copy(io.Discard, resp.Body) - // _ = resp.Body.Close() - // }() - // - // status := resp.StatusCode - // errStr := refreshError.Error() - // if status == 400 { - // if strings.Contains(errStr, `"error_description":"AADSTS90002:`) { - // // Tenant doesn't exist - // delete(tenantIds, tenantId) - // continue - // } else if strings.Contains(errStr, `"error_description":"AADSTS700016:`) { - // // Tenant is valid but the ClientID doesn't exist. - // invalidTenantsForClientId[clientId] = append(invalidTenantsForClientId[clientId], tenantId) - // continue - // } else { - // // Unexpected error. - // r.SetVerificationError(refreshError, clientSecret) - // break - // } - // } else if status == 401 { - // // Tenant exists and the clientID is valid, but something is wrong. - // if strings.Contains(errStr, `"error_description":"AADSTS7000215:`) { - // // Secret is not valid. - // setValidTenantIdForClientId(clientId, tenantId, tenantIds, invalidTenantsForClientId) - // continue IdLoop - // } else if strings.Contains(errStr, `"error_description":"AADSTS7000222:`) { - // // The secret is expired. - // setValidTenantIdForClientId(clientId, tenantId, tenantIds, invalidTenantsForClientId) - // continue SecretLoop - // } else { - // // TODO: Investigate if it's possible to get a 401 with a valid id/secret. - // r.SetVerificationError(refreshError, clientSecret) - // break - // } - // } else { - // // Unexpected status code. - // r.SetVerificationError(refreshError, clientSecret) - // break - // } - // } else { - // // Unexpected error. - // r.SetVerificationError(err, clientSecret) - // break - // } - //} else { - // r.Verified = true - // r.ExtraData = map[string]string{ - // "token": token.OAuthToken(), - // } - // setValidTenantIdForClientId(clientId, tenantId, tenantIds, invalidTenantsForClientId) - // break - //} } } } @@ -243,32 +173,14 @@ func createResult(tenantId string, clientId string, clientSecret string, verifie return r } -func isValidTenant(ctx context.Context, client *http.Client, tenant string) bool { - req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("https://login.microsoftonline.com/%s/.well-known/openid-configuration", tenant), nil) - if err != nil { - return false - } - res, err := client.Do(req) - defer func() { - _, _ = io.Copy(io.Discard, res.Body) - _ = res.Body.Close() - }() - - if res.StatusCode == 200 { - return true - } else if res.StatusCode == 400 { - fmt.Printf("Invalid tenant: %s\n", tenant) - return false - } else { - fmt.Printf("[azure] Unexpected status code: %d for %s\n", res.StatusCode, tenant) - return false - } -} - func (s Scanner) Type() detectorspb.DetectorType { return detectorspb.DetectorType_Azure } +func (s Scanner) Description() string { + return serviceprincipal.Description +} + // region Helper methods. func findSecretMatches(data string) map[string]struct{} { uniqueMatches := make(map[string]struct{}) diff --git a/pkg/detectors/uuids.txt b/pkg/detectors/uuids.txt deleted file mode 100644 index 89a42a2efc36..000000000000 --- a/pkg/detectors/uuids.txt +++ /dev/null @@ -1,37 +0,0 @@ -00000000-0000-0000-0000-000000000000 -11111111-1111-1111-1111-111111111111 -22222222-2222-2222-2222-222222222222 -33333333-3333-3333-3333-333333333333 -44444444-4444-4444-4444-444444444444 -55555555-5555-5555-5555-555555555555 -66666666-6666-6666-6666-666666666666 -77777777-7777-7777-7777-777777777777 -88888888-8888-8888-8888-888888888888 -99999999-9999-9999-9999-999999999999 -12345678-1234-1234-1234-123456789abc -23456789-2345-2345-2345-23456789abcd -34567890-3456-3456-3456-34567890bcde -45678901-4567-4567-4567-45678901cdef -56789012-5678-5678-5678-56789012def0 -aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa -bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb -cccccccc-cccc-cccc-cccc-cccccccccccc -dddddddd-dddd-dddd-dddd-dddddddddddd -eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee -ffffffff-ffff-ffff-ffff-ffffffffffff -deadbeef-dead-beef-dead-beefdeadbeef -cafebabe-cafe-babe-cafe-babecafebabe -badc0ffee-badc-0ffe-badc-0ffeebadc0f -deadface-dead-face-dead-facedeadface -feedface-feed-face-feed-facefeedface -a1b2c3d4-a1b2-c3d4-a1b2-c3d4a1b2c3d4 -98765432-9876-5432-9876-543298765432 -abcdefab-cdef-abcd-efab-cdefabcdefab -a0a0a0a0-a0a0-a0a0-a0a0-a0a0a0a0a0a0 -b0b0b0b0-b0b0-b0b0-b0b0-b0b0b0b0b0b0 -c0c0c0c0-c0c0-c0c0-c0c0-c0c0c0c0c0c0 -d0d0d0d0-d0d0-d0d0-d0d0-d0d0d0d0d0d0 -e0e0e0e0-e0e0-e0e0-e0e0-e0e0e0e0e0e0 -f0f0f0f0-f0f0-f0f0-f0f0-f0f0f0f0f0f0 -xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx --xxxx-xxxx-xxxx-xxxxxxxxxxxx