Skip to content

Commit

Permalink
feat(azure): finish pending items for review
Browse files Browse the repository at this point in the history
  • Loading branch information
rgmz committed Oct 28, 2024
1 parent b272991 commit 2047f64
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 141 deletions.
58 changes: 54 additions & 4 deletions pkg/detectors/azure_entra/common.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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)`)

Expand All @@ -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
}
Expand All @@ -59,19 +72,56 @@ 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 {
return false
}

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
}
}
39 changes: 39 additions & 0 deletions pkg/detectors/azure_entra/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`,
Expand Down Expand Up @@ -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",
Expand All @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down
3 changes: 2 additions & 1 deletion pkg/detectors/azure_entra/serviceprincipal/sp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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:"):
Expand Down
9 changes: 8 additions & 1 deletion pkg/detectors/azure_entra/serviceprincipal/v1/spv1.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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!#$%&()*+,\-./:;<=>?@[\\\]^_{|}~]`)
)

Expand Down Expand Up @@ -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)
Expand All @@ -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{})
Expand Down
108 changes: 10 additions & 98 deletions pkg/detectors/azure_entra/serviceprincipal/v2/spv2.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ package v2
import (
"context"
"errors"
"fmt"
"io"
logContext "github.com/trufflesecurity/trufflehog/v3/pkg/context"
"net/http"
"regexp"
"strings"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand All @@ -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
//}
}
}
}
Expand Down Expand Up @@ -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{})
Expand Down
Loading

0 comments on commit 2047f64

Please sign in to comment.