diff --git a/internal/clients/github/client.go b/internal/clients/github/client.go index 94931248..0724c81d 100644 --- a/internal/clients/github/client.go +++ b/internal/clients/github/client.go @@ -4,12 +4,6 @@ import ( "bytes" "context" "fmt" - "log" - "net/http" - "regexp" - "strings" - "sync" - "github.com/Legit-Labs/legitify/internal/clients/github/pagination" "github.com/Legit-Labs/legitify/internal/clients/github/transport" "github.com/Legit-Labs/legitify/internal/clients/github/types" @@ -18,6 +12,11 @@ import ( "github.com/Legit-Labs/legitify/internal/common/slice_utils" commontypes "github.com/Legit-Labs/legitify/internal/common/types" "github.com/Legit-Labs/legitify/internal/screen" + "log" + "net/http" + "regexp" + "strings" + "sync" githubcollected "github.com/Legit-Labs/legitify/internal/collected/github" "github.com/Legit-Labs/legitify/internal/common/permissions" @@ -83,7 +82,6 @@ func (c *Client) initClients(ctx context.Context, token string) error { var ghClient *gh.Client var graphQLClient *githubv4.Client - rawClient, graphQLRawClient, err := newHttpClients(ctx, token) if err != nil { return err @@ -636,3 +634,25 @@ func (c *Client) GetSecurityAndAnalysisForEnterprise(enterprise string) (*types. } return &p, nil } + +func (c *Client) GetRepositorySecrets(repo, owner string) (*gh.Secrets, error) { + secrets, res, err := c.client.Actions.ListRepoSecrets(c.context, owner, repo, nil) + if err != nil { + return nil, err + } + if res.StatusCode != 200 { + return nil, fmt.Errorf("unexpected HTTP status: %d", res.StatusCode) + } + return secrets, nil +} + +func (c *Client) GetOrganizationSecrets(org string) (*gh.Secrets, error) { + secrets, res, err := c.client.Actions.ListOrgSecrets(c.context, org, nil) + if err != nil { + return nil, err + } + if res.StatusCode != 200 { + return nil, fmt.Errorf("unexpected HTTP status: %d", res.StatusCode) + } + return secrets, nil +} diff --git a/internal/collected/github/organization.go b/internal/collected/github/organization.go index 97a13868..b0d46174 100644 --- a/internal/collected/github/organization.go +++ b/internal/collected/github/organization.go @@ -45,6 +45,12 @@ type Organization struct { SamlEnabled *bool `json:"saml_enabled,omitempty"` Hooks []*github.Hook `json:"hooks"` UserRole permissions.OrganizationRole + OrgSecrets []*OrganizationSecret `json:"organization_secrets,omitempty"` +} + +type OrganizationSecret struct { + Name string `json:"name"` + UpdatedAt int `json:"updated_at"` } func (o Organization) ViolationEntityType() string { diff --git a/internal/collected/github/repository.go b/internal/collected/github/repository.go index 970181ff..35bfe728 100644 --- a/internal/collected/github/repository.go +++ b/internal/collected/github/repository.go @@ -71,6 +71,12 @@ type Repository struct { ActionsTokenPermissions *types.TokenPermissions `json:"actions_token_permissions"` DependencyGraphManifests *GitHubQLDependencyGraphManifests `json:"dependency_graph_manifests"` RulesSet []*types.RepositoryRule `json:"rules_set,omitempty"` + RepoSecrets []*RepositorySecret `json:"repository_secrets,omitempty"` +} + +type RepositorySecret struct { + Name string `json:"name"` + UpdatedAt int `json:"updated_at"` } func (r Repository) ViolationEntityType() string { diff --git a/internal/collectors/github/organization_collector.go b/internal/collectors/github/organization_collector.go index 4844ca5e..3ef9337e 100644 --- a/internal/collectors/github/organization_collector.go +++ b/internal/collectors/github/organization_collector.go @@ -91,10 +91,17 @@ func (c *organizationCollector) collectExtraData(org *ghcollected.ExtendedOrg) g c.IssueMissingPermissions(perm) } + secrets, err := c.collectOrgSecrets(org.Name()) + if err != nil { + secrets = nil + log.Printf("failed to collect secrets for %s, %s", org.Name(), err) + } + return ghcollected.Organization{ Organization: org, SamlEnabled: samlEnabled, Hooks: hooks, + OrgSecrets: secrets, } } @@ -128,3 +135,18 @@ func (c *organizationCollector) collectOrgSamlData(org string) (*bool, error) { return &samlEnabled, nil } + +func (c *organizationCollector) collectOrgSecrets(org string) ([]*ghcollected.OrganizationSecret, error) { + secrets, err := c.Client.GetOrganizationSecrets(org) + if err != nil { + return nil, err + } + var orgSecrets []*ghcollected.OrganizationSecret + for i := 0; i < len(secrets.Secrets); i++ { + orgSecrets = append(orgSecrets, &ghcollected.OrganizationSecret{ + Name: secrets.Secrets[i].Name, + UpdatedAt: int(secrets.Secrets[i].UpdatedAt.Time.UnixNano())}) + } + + return orgSecrets, nil +} diff --git a/internal/collectors/github/repository_collector.go b/internal/collectors/github/repository_collector.go index 227c914f..f3148e08 100644 --- a/internal/collectors/github/repository_collector.go +++ b/internal/collectors/github/repository_collector.go @@ -3,13 +3,12 @@ package github import ( "context" "fmt" - "log" - "net/http" - "github.com/Legit-Labs/legitify/internal/collectors" "github.com/Legit-Labs/legitify/internal/common/types" "github.com/Legit-Labs/legitify/internal/context_utils" "github.com/Legit-Labs/legitify/internal/scorecard" + "log" + "net/http" "github.com/Legit-Labs/legitify/internal/common/group_waiter" "github.com/Legit-Labs/legitify/internal/common/permissions" @@ -246,6 +245,10 @@ func (rc *repositoryCollector) collectExtraData(login string, repo = rc.withRepositoryHooks(repo, login) repo = rc.withRepoCollaborators(repo, login) repo = rc.withActionsSettings(repo, login) + repo, err = rc.withSecrets(repo, login) + if err != nil { + log.Printf("failed to collect repository secrets for %s: %s", repo.Repository.Name, err) + } repo, err = rc.withDependencyGraphManifestsCount(repo, login) if err != nil { @@ -388,6 +391,22 @@ func (rc *repositoryCollector) withRulesSet(repository ghcollected.Repository, o return repository, nil } +func (rc *repositoryCollector) withSecrets(repository ghcollected.Repository, login string) (ghcollected.Repository, error) { + secrets, err := rc.Client.GetRepositorySecrets(repository.Name(), login) + if err != nil { + return repository, err + } + var repoSecrets []*ghcollected.RepositorySecret + for i := 0; i < len(secrets.Secrets); i++ { + repoSecrets = append(repoSecrets, &ghcollected.RepositorySecret{ + Name: secrets.Secrets[i].Name, + UpdatedAt: int(secrets.Secrets[i].UpdatedAt.Time.UnixNano()), + }) + } + repository.RepoSecrets = repoSecrets + return repository, nil +} + // fixBranchProtectionInfo fixes the branch protection info for the repository, // to reflect whether there is no branch protection, or just no permission to fetch the info. func (rc *repositoryCollector) fixBranchProtectionInfo(repository ghcollected.Repository, org string) (ghcollected.Repository, error) { diff --git a/internal/enricher/enricher_manager.go b/internal/enricher/enricher_manager.go index 2e1d49ef..a4f35090 100644 --- a/internal/enricher/enricher_manager.go +++ b/internal/enricher/enricher_manager.go @@ -50,6 +50,7 @@ var mapping = map[string]enrichers.Enricher{ enrichers.Scorecard: enrichers.NewScorecardEnricher(), enrichers.MembersList: enrichers.NewMembersListEnricher(), enrichers.HooksList: enrichers.NewHooksListEnricher(), + enrichers.SecretsList: enrichers.NewSecretsListEnricher(), } func NewEnricherManager() EnricherManager { diff --git a/internal/enricher/enrichers/secrets_list_enricher.go b/internal/enricher/enrichers/secrets_list_enricher.go new file mode 100644 index 00000000..44b0b1d5 --- /dev/null +++ b/internal/enricher/enrichers/secrets_list_enricher.go @@ -0,0 +1,54 @@ +package enrichers + +import ( + "encoding/json" + "fmt" + "github.com/Legit-Labs/legitify/internal/analyzers" + "github.com/Legit-Labs/legitify/internal/common/map_utils" + "github.com/iancoleman/orderedmap" + "golang.org/x/net/context" + "log" +) + +const SecretsList = "secretsList" + +func NewSecretsListEnricher() secretsListEnricher { + return secretsListEnricher{} +} + +type secretsListEnricher struct { +} + +func (e secretsListEnricher) Enrich(_ context.Context, data analyzers.AnalyzedData) (Enrichment, bool) { + result, err := createSecretListEnrichment(data.ExtraData) + if err != nil { + log.Printf("failed to enrich secrets list: %v", err) + return nil, false + } + return result, true +} + +func (e secretsListEnricher) Parse(data interface{}) (Enrichment, error) { + return NewGenericListEnrichmentFromInterface(data) +} + +func createSecretListEnrichment(extraData interface{}) (GenericListEnrichment, error) { + asMap, ok := extraData.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("invalid hookslist extra data") + } + + result := []orderedmap.OrderedMap{} + for k := range asMap { + var secretsEnrichment map[string]string + + err := json.Unmarshal([]byte(k), &secretsEnrichment) + if err != nil { + return nil, err + } + + result = append(result, *map_utils.ToKeySortedMap(secretsEnrichment)) + } + + return result, nil +} diff --git a/policies/bundle_test.go b/policies/bundle_test.go index 670fb745..e73caea0 100644 --- a/policies/bundle_test.go +++ b/policies/bundle_test.go @@ -46,5 +46,5 @@ func TestPoliciesBundle(t *testing.T) { count, err := countBundles() require.Nilf(t, err, "counting files: %v", err) - require.Equal(t, count, 7, "Expecting 7 files in bundle") + require.Equal(t, count, 10, "Expecting 10 files in bundle") } diff --git a/policies/github/common/members.rego b/policies/github/common/members.rego new file mode 100644 index 00000000..fadadd3c --- /dev/null +++ b/policies/github/common/members.rego @@ -0,0 +1,13 @@ +package common.members + +isStale(target_last_active, count_months) { + now := time.now_ns() + diff := time.diff(now, target_last_active) + # diff[0] the year index + diff[0] >= 1 + } else { + now := time.now_ns() + diff := time.diff(now, target_last_active) + # diff[1] the months index + diff[1] >= count_months +} diff --git a/policies/github/common/secrets.rego b/policies/github/common/secrets.rego new file mode 100644 index 00000000..86e9036b --- /dev/null +++ b/policies/github/common/secrets.rego @@ -0,0 +1,6 @@ +package common.secrets + +is_stale(date) { + diff := time.diff(time.now_ns(),date) + diff[0] >= 1 +} diff --git a/policies/github/member.rego b/policies/github/member.rego index ce126f5c..53803e55 100644 --- a/policies/github/member.rego +++ b/policies/github/member.rego @@ -1,5 +1,6 @@ package member +import data.common.members as memberUtils # METADATA # scope: rule # title: Organization Should Have Fewer Than Three Owners @@ -36,7 +37,7 @@ stale_member_found[mem] := true { mem := input.members[member] mem.is_admin == false mem.last_active != -1 - isStale(mem.last_active, 6) + memberUtils.isStale(mem.last_active, 6) } # METADATA @@ -57,13 +58,5 @@ stale_admin_found[mem] := true { mem := input.members[member] mem.is_admin == true mem.last_active != -1 - isStale(mem.last_active, 6) -} - -isStale(target_last_active, count_months) { - now := time.now_ns() - diff := time.diff(now, target_last_active) - - # diff[1] the months index - diff[1] >= count_months -} + memberUtils.isStale(mem.last_active, 6) +} \ No newline at end of file diff --git a/policies/github/organization.rego b/policies/github/organization.rego index 5029e804..968862fe 100644 --- a/policies/github/organization.rego +++ b/policies/github/organization.rego @@ -1,6 +1,7 @@ package organization import data.common.webhooks as webhookUtils +import data.common.secrets as secretUtils # METADATA # scope: rule @@ -110,3 +111,29 @@ default organization_not_using_single_sign_on := true organization_not_using_single_sign_on := false { input.saml_enabled } + +# METADATA +# scope: rule +# title: Organization Has Stale Secrets +# description: Some of the organizations secrets have not been updated for over a year. It is recommended to refresh secret values regularly in order to minimize the risk of breach in case of an information leak. +# custom: +# requiredEnrichers: [secretsList] +# remediationSteps: +# - Enter your organization's landing page +# - Go to the settings tab +# - Under the 'Security' title on the left, choose 'Secrets and variables' +# - Click 'Actions' +# - Sort secrets by 'Last Updated' +# - Regenerate every secret older than one year and add the new value to GitHub's secret manager +# severity: MEDIUM +# requiredScopes: [admin:org, repo] +# threat: Sensitive data may have been inadvertently made public in the past, and an attacker who holds this data may gain access to your current CI and services. In addition, there may be old or unnecessary tokens that have not been inspected and can be used to access sensitive information. +organization_secret_is_stale[stale] := true{ + some index + secret := input.organization_secrets[index] + secretUtils.is_stale(secret.updated_at) + stale := { + "name" : secret.name, + "update date" : time.format(secret.updated_at), + } +} diff --git a/policies/github/repository.rego b/policies/github/repository.rego index 47b31948..f1e914de 100644 --- a/policies/github/repository.rego +++ b/policies/github/repository.rego @@ -538,4 +538,4 @@ users_allowed_to_bypass_ruleset := false { some index rule := input.rules_set[index] count(rule.ruleset.bypass_actors) == 0 -} \ No newline at end of file +} diff --git a/policies/github/repository_two.rego b/policies/github/repository_two.rego new file mode 100644 index 00000000..1d04bc99 --- /dev/null +++ b/policies/github/repository_two.rego @@ -0,0 +1,30 @@ +package repository + +import data.common.secrets as secretUtils + +# METADATA +# scope: rule +# title: Repository Has Stale Secrets +# description: Some of the repository secrets have not been updated for over a year. It is recommended to refresh secret values regularly in order to minimize the risk of breach in case of an information leak. +# custom: +# requiredEnrichers: [secretsList] +# remediationSteps: +# - Enter your repository's landing page +# - Go to the settings tab +# - Under the 'Security' title on the left, choose 'Secrets and variables' +# - Click 'Actions' +# - Sort secrets by 'Last Updated' +# - Regenerate every secret older than one year and add the new value to GitHub's secret manager +# severity: MEDIUM +# requiredScopes: [repo] +# threat: Sensitive data may have been inadvertently made public in the past, and an attacker who holds this data may gain access to your current CI and services. In addition, there may be old or unnecessary tokens that have not been inspected and can be used to access sensitive information. +repository_secret_is_stale[stale] := true{ + some index + secret := input.repository_secrets[index] + secretUtils.is_stale(secret.updated_at) + stale := { + "name" : secret.name, + "update date" : time.format(secret.updated_at), + } + +} diff --git a/test/member_test.go b/test/member_test.go index 5246ad34..ce635f51 100644 --- a/test/member_test.go +++ b/test/member_test.go @@ -51,7 +51,7 @@ func TestMember(t *testing.T) { hasLastActive: true, members: []githubcollected.OrganizationMember{ { - LastActive: int(time.Now().AddDate(0, -9, 0).UnixNano()), + LastActive: int(time.Now().AddDate(-1, -2, 0).UnixNano()), IsAdmin: true, }, }, diff --git a/test/organization_test.go b/test/organization_test.go index 9529ad59..ab968d4b 100644 --- a/test/organization_test.go +++ b/test/organization_test.go @@ -4,6 +4,7 @@ import ( "github.com/Legit-Labs/legitify/internal/common/scm_type" "github.com/google/go-github/v53/github" "testing" + "time" githubcollected "github.com/Legit-Labs/legitify/internal/collected/github" "github.com/Legit-Labs/legitify/internal/common/namespace" @@ -14,6 +15,7 @@ type organizationMockConfiguration struct { ssoEnabled *bool name string url string + secrets []*githubcollected.OrganizationSecret } func newOrganizationMock(config organizationMockConfiguration) githubcollected.Organization { @@ -30,11 +32,16 @@ func newOrganizationMock(config organizationMockConfiguration) githubcollected.O } hooks = append(hooks, &hook) } + var orgSecrets []*githubcollected.OrganizationSecret = nil + if config.secrets != nil { + orgSecrets = append(orgSecrets, config.secrets...) + } return githubcollected.Organization{ Organization: nil, SamlEnabled: &samlEnabledMockResult, Hooks: hooks, + OrgSecrets: orgSecrets, } } @@ -113,6 +120,56 @@ func TestOrganization(t *testing.T) { ssoEnabled: &boolTrue, }, }, + { + name: "Organization has no stale secrets", + policyName: "organization_secret_is_stale", + shouldBeViolated: false, + args: organizationMockConfiguration{ + secrets: []*githubcollected.OrganizationSecret{ + { + Name: "test1", + UpdatedAt: int(time.Now().UnixNano()) - 2628000000000000, // one month + }, + { + Name: "test2", + UpdatedAt: int(time.Now().UnixNano()) - (3 * 2628000000000000), // three months + }, + { + Name: "test3", + UpdatedAt: int(time.Now().UnixNano()) - (6 * 2628000000000000), // six month + }, + }, + }, + }, + { + name: "Organization has stale secrets", + policyName: "organization_secret_is_stale", + shouldBeViolated: true, + args: organizationMockConfiguration{ + secrets: []*githubcollected.OrganizationSecret{ + { + Name: "test1", + UpdatedAt: 1652020546000000000, //08.05.2022 + }, + { + Name: "test2", + UpdatedAt: 957796546000000000, //08.05.2000 + }, + { + Name: "test3", + UpdatedAt: int(time.Now().UnixNano()), + }, + }, + }, + }, + { + name: "Organization has no secrets", + policyName: "organization_secret_is_stale", + shouldBeViolated: false, + args: organizationMockConfiguration{ + secrets: nil, + }, + }, } for _, test := range tests { diff --git a/test/repository_test.go b/test/repository_test.go index a74bfc96..2266dfb8 100644 --- a/test/repository_test.go +++ b/test/repository_test.go @@ -309,6 +309,68 @@ func TestRepositoryActionsSettingsActionsCanApprovePullRequests(t *testing.T) { } } +func TestRepositoryWithNoStaleSecrets(t *testing.T) { + name := "repository has no secrets" + testedPolicyName := "repository_secret_is_stale" + makeMockData := func() githubcollected.Repository { + return githubcollected.Repository{ + RepoSecrets: []*githubcollected.RepositorySecret{ + { + Name: "test1", + UpdatedAt: int(time.Now().UnixNano()) - 2628000000000000, // one month + }, + { + Name: "test2", + UpdatedAt: int(time.Now().UnixNano()) - (3 * 2628000000000000), // three months + }, + { + Name: "test3", + UpdatedAt: int(time.Now().UnixNano()) - (6 * 2628000000000000), // six month + }, + }, + } + } + expectFailure := false + repositoryTestTemplate(t, name, makeMockData(), testedPolicyName, expectFailure, scm_type.GitHub) +} + +func TestRepositoryWithStaleSecrets(t *testing.T) { + name := "repository has no secrets" + testedPolicyName := "repository_secret_is_stale" + makeMockData := func() githubcollected.Repository { + return githubcollected.Repository{ + RepoSecrets: []*githubcollected.RepositorySecret{ + { + Name: "test1", + UpdatedAt: 1652020546000000000, //08.05.2022 + }, + { + Name: "test2", + UpdatedAt: 957796546000000000, //08.05.2000 + }, + { + Name: "test3", + UpdatedAt: int(time.Now().UnixNano()), + }, + }, + } + } + expectFailure := true + repositoryTestTemplate(t, name, makeMockData(), testedPolicyName, expectFailure, scm_type.GitHub) +} + +func TestRepositoryWithNoSecrets(t *testing.T) { + name := "repository has no secrets" + testedPolicyName := "repository_secret_is_stale" + makeMockData := func() githubcollected.Repository { + return githubcollected.Repository{ + RepoSecrets: nil, + } + } + expectFailure := false + repositoryTestTemplate(t, name, makeMockData(), testedPolicyName, expectFailure, scm_type.GitHub) +} + func TestGitlabRepositoryTooManyAdmins(t *testing.T) { name := "Project Has Too Many Owners" testedPolicyName := "project_has_too_many_admins"