Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

security(snyk): removed high level snyk issues for hard coded credentials #168

Merged
merged 5 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changes/unreleased/security-20240812-133345.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: security
body: removed high level snyk issues for hard coded credentials
pacificcode marked this conversation as resolved.
Show resolved Hide resolved
time: 2024-08-12T13:33:45.993584-07:00
3 changes: 3 additions & 0 deletions .changes/unreleased/security-20240815-061342.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: security
body: 'Modified vars to bring into compliance with CWE-798: Use of Hard-coded Credentials.'
time: 2024-08-15T06:13:42.854784-07:00
6 changes: 4 additions & 2 deletions auth/method_password.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
"github.com/spf13/viper"
)

const authSP = "auth.securePassword"

func buildPasswordParams() (*requestBody, error) {
data := &requestBody{
GrantType: authTypeToGrantType[Password],
Expand All @@ -20,13 +22,13 @@
// If it is an empty string, look for SecurePassword, which Viper gets only from config. Get the corresponding
// key file and use it to decrypt SecurePassword.
if data.Password == "" {
passSetting := cst.SecurePassword
passSetting := authSP

Check warning on line 25 in auth/method_password.go

View check run for this annotation

Codecov / codecov/patch

auth/method_password.go#L25

Added line #L25 was not covered by tests
storeType := viper.GetString(cst.StoreType)
if storeType == store.WinCred || storeType == store.PassLinux {
passSetting = cst.Password
}
if pass, err := store.GetSecureSetting(passSetting); err == nil && pass != "" {
if passSetting == cst.SecurePassword {
if passSetting == authSP {

Check warning on line 31 in auth/method_password.go

View check run for this annotation

Codecov / codecov/patch

auth/method_password.go#L31

Added line #L31 was not covered by tests
keyPath := GetEncryptionKeyFilename(viper.GetString(cst.Tenant), data.Username)
key, err := store.ReadFileInDefaultPath(keyPath)
if err != nil || key == "" {
Expand Down
6 changes: 3 additions & 3 deletions commands/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
• auth --profile staging
• auth --auth-username %[3]s --auth-password %[4]s
• auth --auth-type %[5]s --auth-client-id %[6]s --domain %[7]s --auth-client-secret %[8]s
`, cst.NounAuth, cst.ProductName, cst.ExampleUser, cst.ExamplePassword, cst.ExampleAuthType, cst.ExampleAuthClientID, cst.ExampleDomain, cst.ExampleAuthClientSecret, string(auth.FederatedAws)),
`, cst.NounAuth, cst.ProductName, cst.ExampleUser, "************", cst.ExampleAuthType, cst.ExampleAuthClientID, cst.ExampleDomain, cst.ExampleAuthClientSecret, string(auth.FederatedAws)),
RunFunc: handleAuth,
})
}
Expand Down Expand Up @@ -65,8 +65,8 @@

func GetAuthChangePasswordCmd() (cli.Command, error) {
return NewCommand(CommandArgs{
Path: []string{cst.NounAuth, cst.ChangePassword},
SynopsisText: fmt.Sprintf("%s %s", cst.NounAuth, cst.ChangePassword),
Path: []string{cst.NounAuth, "change-password"},
SynopsisText: fmt.Sprintf("%s %s", cst.NounAuth, "change-password"),

Check warning on line 69 in commands/auth.go

View check run for this annotation

Codecov / codecov/patch

commands/auth.go#L68-L69

Added lines #L68 - L69 were not covered by tests
HelpText: `Change user password

Usage:
Expand Down
4 changes: 2 additions & 2 deletions commands/cli-config.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@
encryptionKeyFileName = auth.GetEncryptionKeyFilename(viper.GetString(cst.Tenant), user)

if isSecureStore {
if err := store.StoreSecureSetting(strings.Join([]string{profile, cst.NounAuth, cst.DataPassword}, "."), password, storeType); err != nil {
if err := store.StoreSecureSetting(strings.Join([]string{profile, cst.NounAuth, "password"}, "."), password, storeType); err != nil {

Check warning on line 725 in commands/cli-config.go

View check run for this annotation

Codecov / codecov/patch

commands/cli-config.go#L725

Added line #L725 was not covered by tests
ui.Error(err.Error())
return 1
}
Expand All @@ -733,7 +733,7 @@
return 1
}
passwordKey = key
prf.Set(encrypted, cst.NounAuth, cst.DataSecurePassword)
prf.Set(encrypted, cst.NounAuth, "securePassword")

Check warning on line 736 in commands/cli-config.go

View check run for this annotation

Codecov / codecov/patch

commands/cli-config.go#L736

Added line #L736 was not covered by tests
}

case (storeType != store.None && auth.AuthType(authType) == auth.ClientCredential):
Expand Down
14 changes: 7 additions & 7 deletions commands/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
)

var (
errMustSpecifyPasswordOrDisplayname = errors.NewF("error: must specify %s or %s", cst.DataPassword, cst.DataDisplayname)
errMustSpecifyPasswordOrDisplayname = errors.NewF("error: must specify %s or %s", "password", cst.DataDisplayname)
errWrongDisplayName = errors.NewS("error: displayname field must be between 3 and 100 characters")
)

Expand Down Expand Up @@ -138,11 +138,11 @@ func GetUserCreateCmd() (cli.Command, error) {
Usage:
• user create --username %[3]s --password %[4]s
• user create --username %[3]s --external-id [email protected] --provider project1.gcloud --password %[4]s
`, cst.NounUser, cst.ProductName, cst.ExampleUser, cst.ExamplePassword),
`, cst.NounUser, cst.ProductName, cst.ExampleUser, "************"),
FlagsPredictor: []*predictor.Params{
{Name: cst.DataUsername, Usage: "Used as id (required) (must conform to /[a-zA-Z0-9_-@+.]{3,100}/)."},
{Name: cst.DataDisplayname, Usage: "Name to display in UI."},
{Name: cst.DataPassword, Usage: "Must be 8-100 chars, with an uppercase and special char from this list: ~!@#$%^&*()."},
{Name: "password", Usage: "Must be 8-100 chars, with an uppercase and special char from this list: ~!@#$%^&*()."},
{Name: cst.DataExternalID, Usage: "Identifier attached to federated login e.g. AWS or ARN."},
{Name: cst.DataProvider, Usage: "Used for linking user with federated/external auth, must match name of Auth Provider in administration section."},
},
Expand All @@ -159,9 +159,9 @@ func GetUserUpdateCmd() (cli.Command, error) {

Usage:
• user update --username %[3]s --password %[4]s
`, cst.NounUser, cst.ProductName, cst.ExampleUser, cst.ExamplePassword),
`, cst.NounUser, cst.ProductName, cst.ExampleUser, "************"),
FlagsPredictor: []*predictor.Params{
{Name: cst.DataPassword, Usage: "Uses interactive prompt if not sent as flag. Must be 8-100 chars, with an uppercase and special char from this list: ~!@#$%^&*()."},
{Name: "password", Usage: "Uses interactive prompt if not sent as flag. Must be 8-100 chars, with an uppercase and special char from this list: ~!@#$%^&*()."},
{Name: cst.DataUsername, Usage: "Existing username to update"},
{Name: cst.DataDisplayname, Usage: "New display name to show for username."},
},
Expand Down Expand Up @@ -255,7 +255,7 @@ func handleUserRestoreCmd(vcli vaultcli.CLI, args []string) error {

func handleUserCreateCmd(vcli vaultcli.CLI, args []string) error {
userName := viper.GetString(cst.DataUsername)
password := viper.GetString(cst.DataPassword)
password := viper.GetString("password")
provider := viper.GetString(cst.DataProvider)
externalID := viper.GetString(cst.DataExternalID)

Expand Down Expand Up @@ -296,7 +296,7 @@ func handleUserUpdateCmd(vcli vaultcli.CLI, args []string) error {
}

displayNameExists := hasFlag(args, "--"+cst.DataDisplayname)
passData := viper.GetString(cst.DataPassword)
passData := viper.GetString("password")
displayName := viper.GetString(cst.DataDisplayname)
if passData == "" && !displayNameExists {
return errMustSpecifyPasswordOrDisplayname
Expand Down
4 changes: 2 additions & 2 deletions commands/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ func TestHandleUserCreateCmd(t *testing.T) {
for _, tt := range testCase {
viper.Set(cst.DataUsername, tt.userName)
viper.Set(cst.DataDisplayname, tt.displayName)
viper.Set(cst.DataPassword, tt.password)
viper.Set("password", tt.password)
viper.Set(cst.DataProvider, tt.provider)
viper.Set(cst.DataExternalID, tt.externalID)
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -395,7 +395,7 @@ func TestHandleUserUpdateCmd(t *testing.T) {
viper.Set(cst.Version, "v1")
for _, tt := range testCase {
viper.Set(cst.DataUsername, tt.userName)
viper.Set(cst.DataPassword, tt.password)
viper.Set("password", tt.password)
viper.Set(cst.DataDisplayname, tt.displayName)
t.Run(tt.name, func(t *testing.T) {
var data []byte
Expand Down
55 changes: 26 additions & 29 deletions constants/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,29 @@ package constants

// Constants for Actions
const (
Read = "read"
Create = "create"
Update = "update"
Rollback = "rollback"
Upload = "upload"
Edit = "edit"
Delete = "delete"
Describe = "describe"
Clear = "clear"
List = "list"
ChangePassword = "change-password"
pacificcode marked this conversation as resolved.
Show resolved Hide resolved
Search = "search"
BustCache = "bustcache"
AddMember = "add-members"
DeleteMember = "delete-members"
Restore = "restore"
Ping = "ping"
Rotate = "rotate"
Encrypt = "encrypt"
Decrypt = "decrypt"
Generate = "generate"
Apply = "apply"
Status = "status"
UseProfile = "use-profile"
Read = "read"
Create = "create"
Update = "update"
Rollback = "rollback"
Upload = "upload"
Edit = "edit"
Delete = "delete"
Describe = "describe"
Clear = "clear"
List = "list"
Search = "search"
BustCache = "bustcache"
AddMember = "add-members"
DeleteMember = "delete-members"
Restore = "restore"
Ping = "ping"
Rotate = "rotate"
Encrypt = "encrypt"
Decrypt = "decrypt"
Generate = "generate"
Apply = "apply"
Status = "status"
UseProfile = "use-profile"
)

// Nouns
Expand Down Expand Up @@ -95,7 +94,6 @@ const (
Tenant = "tenant"
DomainName = "domain"
Password = "auth.password"
SecurePassword = "auth.securePassword"
CurrentPassword = "currentPassword"
NewPassword = "newPassword"
AuthProvider = "auth.provider"
Expand Down Expand Up @@ -162,10 +160,9 @@ const (
DataProvider = "provider"

// user
DataUsername = "username"
DataSecurePassword = "securePassword"
DataPassword = "password"
DataDisplayname = "displayname"
DataUsername = "username"
// DataSecurePassword = "securePassword"
DataDisplayname = "displayname"

// permission
DataSubject = "subjects"
Expand Down
1 change: 0 additions & 1 deletion constants/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const (
ExampleConfigPath = "@/tmp/config.yml"
ExampleUser = "kadmin"
ExampleSIEM = "LogsInc"
ExamplePassword = "********"
ExampleAuthType = "clientcred"
ExampleUserSearch = "adm"
ExamplePolicySearch = "secrets/databases"
Expand Down
Loading