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 9, 2024
1 parent 20b3f37 commit d69f4c2
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 151 deletions.
43 changes: 41 additions & 2 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 Down Expand Up @@ -59,19 +61,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.V(3).Info("WARNING: Unexpected response when checking if tenant exists", "status_code", res.StatusCode, "body", string(bodyBytes))
return false
}
}
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
105 changes: 7 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,7 +95,7 @@ SecretLoop:
}

if verify {
if !isValidTenant(ctx, client, tenantId) {
if !azure_entra.TenantExists(logCtx, client, tenantId) {
// Tenant doesn't exist
delete(tenantIds, tenantId)
continue
Expand Down Expand Up @@ -126,78 +125,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 +170,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
37 changes: 0 additions & 37 deletions pkg/detectors/uuids.txt

This file was deleted.

17 changes: 6 additions & 11 deletions pkg/pb/detectorspb/detectors.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion proto/detectors.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,6 @@ enum DetectorType {
PyPI = 998;
RailwayApp = 999;
Meraki = 1000;
AzureRefreshToken = 1001;
}

message Result {
Expand Down

0 comments on commit d69f4c2

Please sign in to comment.