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.

Sign up for GitHub

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

Jump to bottom

Fix verify_password gatherer for scenarios where there is not hash #305

Merged
merged 3 commits into from
Dec 12, 2023
Merged

Fix verify_password gatherer for scenarios where there is not hash #305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3c312b0
Fix verify_password gatherer for scenarios where there is not hash
arbulu89 Nov 29, 2023
499db6c
Rename verify password gatherer test
arbulu89 Nov 29, 2023
ce41f19
Handle the no password and blocked password
arbulu89 Nov 29, 2023
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
71 changes: 57 additions & 14 deletions internal/factsengine/gatherers/verifypassword.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ const (

// nolint:gochecknoglobals
var (
checkableUsernames = []string{"hacluster"}
unsafePasswords = []string{"linux"}
checkableUsernames = []string{"hacluster"}
unsafePasswords = []string{"linux"}
passwordNotSetValues = "!*:;\\" // Get more info with "man 3 crypt"
)

// nolint:gochecknoglobals
Expand All @@ -29,6 +30,21 @@ var (
Message: "requested user is not whitelisted for password check",
}

VerifyPasswordShadowError = entities.FactGatheringError{
Type: "verify-password-shadow-error",
Message: "error getting shadow output",
}

VerifyPasswordPasswordBlocked = entities.FactGatheringError{
Type: "verify-password-password-blocked",
Message: "password authentication blocked for user",
}

VerifyPasswordPasswordNotSet = entities.FactGatheringError{
Type: "verify-password-password-not-set",
Message: "password not set for user",
}

VerifyPasswordCryptError = entities.FactGatheringError{
Type: "verify-password-crypt-error",
Message: "error while verifying the password for user",
Expand Down Expand Up @@ -59,17 +75,41 @@ func (g *VerifyPasswordGatherer) Gather(factsRequests []entities.FactRequest) ([
for _, factReq := range factsRequests {
if !utils.Contains(checkableUsernames, factReq.Argument) {
gatheringError := VerifyPasswordInvalidUsername.Wrap(factReq.Argument)
fact := entities.NewFactGatheredWithError(factReq, gatheringError)
facts = append(facts, fact)
log.Error(gatheringError)
facts = append(facts, entities.NewFactGatheredWithError(factReq, gatheringError))
continue
}
username := factReq.Argument

salt, hash, err := g.getSalt(username)
if err != nil {
log.Error(err)
hash, err := g.getHash(username)

switch {
case err != nil:
{
gatheringError := VerifyPasswordShadowError.Wrap(err.Error())
log.Error(gatheringError)
facts = append(facts, entities.NewFactGatheredWithError(factReq, gatheringError))
continue
}

case len(hash) == 0:
{
gatheringError := VerifyPasswordPasswordNotSet.Wrap(username)
log.Error(gatheringError)
facts = append(facts, entities.NewFactGatheredWithError(factReq, gatheringError))
continue
}

case strings.ContainsAny(hash, passwordNotSetValues):
{
gatheringError := VerifyPasswordPasswordBlocked.Wrap(username)
log.Error(gatheringError)
facts = append(facts, entities.NewFactGatheredWithError(factReq, gatheringError))
continue
}
}
log.Debugf("Obtained salt using user %s: %s", username, salt)

log.Debugf("Obtained hash using user %s: %s", username, hash)

crypter := sha512crypt.New()
isPasswordWeak := false
Expand All @@ -83,7 +123,7 @@ func (g *VerifyPasswordGatherer) Gather(factsRequests []entities.FactRequest) ([
break
}
if !errors.Is(matchErr, crypt.ErrKeyMismatch) {
gatheringError = VerifyPasswordCryptError.Wrap(username)
gatheringError = VerifyPasswordCryptError.Wrap(username).Wrap(matchErr.Error())
break
}
}
Expand All @@ -103,13 +143,16 @@ func (g *VerifyPasswordGatherer) Gather(factsRequests []entities.FactRequest) ([
return facts, nil
}

func (g *VerifyPasswordGatherer) getSalt(user string) ([]byte, string, error) {
func (g *VerifyPasswordGatherer) getHash(user string) (string, error) {
shadow, err := g.executor.Exec("getent", "shadow", user)
if err != nil {
return nil, "", errors.Wrap(err, "Error getting salt")
return "", errors.Wrap(err, "Error getting hash")
}

fields := strings.Split(string(shadow), ":")
if len(fields) != 9 {
return "", fmt.Errorf("shadow output does not have 9 fields: %s", string(shadow))
}
salt := strings.Split(string(shadow), "$")[2]
hash := strings.Split(string(shadow), ":")[1]

return []byte(fmt.Sprintf("$6$%s", salt)), hash, nil
return fields[1], nil
}
148 changes: 145 additions & 3 deletions internal/factsengine/gatherers/verifypassword_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,115 @@ func (suite *PasswordTestSuite) TestPasswordGatherNotEqual() {
suite.ElementsMatch(expectedResults, factResults)
}

func (suite *PasswordTestSuite) TestPasswordGatherCryptError() {
shadow := []byte("hacluster:$aaaa$aaaa")
func (suite *PasswordTestSuite) TestPasswordGatherBloquedPassword() {
suite.mockExecutor.
On("Exec", "getent", "shadow", "hacluster").
Return([]byte("hacluster:!:19029::::::"), nil).
Once().
On("Exec", "getent", "shadow", "hacluster").
Return([]byte("hacluster:!$6$WFEgSAefduOyvLCN$MprO90E:19029::::::"), nil).
Once().
On("Exec", "getent", "shadow", "hacluster").
Return([]byte("hacluster:*:19029::::::"), nil)

verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor)

factRequests := []entities.FactRequest{
{
Name: "hacluster",
Gatherer: "verify_password",
Argument: "hacluster",
CheckID: "check1",
},
{
Name: "hacluster",
Gatherer: "verify_password",
Argument: "hacluster",
CheckID: "check2",
},
{
Name: "hacluster",
Gatherer: "verify_password",
Argument: "hacluster",
CheckID: "check3",
},
}

factResults, err := verifyPasswordGatherer.Gather(factRequests)

expectedResults := []entities.Fact{
{
Name: "hacluster",
CheckID: "check2",
Value: nil,
Error: &entities.FactGatheringError{
Message: "password authentication blocked for user: hacluster",
Type: "verify-password-password-blocked",
},
},
{
Name: "hacluster",
CheckID: "check1",
Value: nil,
Error: &entities.FactGatheringError{
Message: "password authentication blocked for user: hacluster",
Type: "verify-password-password-blocked",
},
},
{
Name: "hacluster",
CheckID: "check3",
Value: nil,
Error: &entities.FactGatheringError{
Message: "password authentication blocked for user: hacluster",
Type: "verify-password-password-blocked",
},
},
}

suite.NoError(err)
suite.ElementsMatch(expectedResults, factResults)
}

func (suite *PasswordTestSuite) TestPasswordGatherNoPassword() {
shadow := []byte("hacluster::19029::::::")

suite.mockExecutor.On("Exec", "getent", "shadow", "hacluster").Return(
shadow, nil)

verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor)

factRequests := []entities.FactRequest{
{
Name: "hacluster",
Gatherer: "verify_password",
Argument: "hacluster",
CheckID: "check1",
},
}

factResults, err := verifyPasswordGatherer.Gather(factRequests)

expectedResults := []entities.Fact{
{
Name: "hacluster",
CheckID: "check1",
Value: nil,
Error: &entities.FactGatheringError{
Message: "password not set for user: hacluster",
Type: "verify-password-password-not-set",
},
},
}

suite.NoError(err)
suite.ElementsMatch(expectedResults, factResults)
}

func (suite *PasswordTestSuite) TestPasswordGatherDifferentEncType() {
shadow := []byte("hacluster:$5$WFEgPAefduOyvLCN$MprO90En7b/" +
"cP8uJJpHzJ7ufTPjYuWoVF4s.3MUdOR9iwcO.6E3uCHX1waqypjey458NKGE9O7lnWpV/" +
"qd2tg1:19029::::::")

suite.mockExecutor.On("Exec", "getent", "shadow", "hacluster").Return(
shadow, nil)
Expand All @@ -113,7 +220,7 @@ func (suite *PasswordTestSuite) TestPasswordGatherCryptError() {
CheckID: "check1",
Value: nil,
Error: &entities.FactGatheringError{
Message: "error while verifying the password for user: hacluster",
Message: "error while verifying the password for user: hacluster: invalid magic prefix",
Type: "verify-password-crypt-error",
},
},
Expand All @@ -123,6 +230,41 @@ func (suite *PasswordTestSuite) TestPasswordGatherCryptError() {
suite.ElementsMatch(expectedResults, factResults)
}

func (suite *PasswordTestSuite) TestPasswordGatherInvalidShadowOutput() {
shadow := []byte("hacluster:hash:")

suite.mockExecutor.On("Exec", "getent", "shadow", "hacluster").Return(
shadow, nil)

verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor)

factRequests := []entities.FactRequest{
{
Name: "hacluster",
Gatherer: "verify_password",
Argument: "hacluster",
CheckID: "check1",
},
}

factResults, err := verifyPasswordGatherer.Gather(factRequests)

expectedResults := []entities.Fact{
{
Name: "hacluster",
CheckID: "check1",
Value: nil,
Error: &entities.FactGatheringError{
Message: "error getting shadow output: shadow output does not have 9 fields: hacluster:hash:",
Type: "verify-password-shadow-error",
},
},
}

suite.NoError(err)
suite.ElementsMatch(expectedResults, factResults)
}

func (suite *PasswordTestSuite) TestPasswordGatherWrongArguments() {
verifyPasswordGatherer := gatherers.NewVerifyPasswordGatherer(suite.mockExecutor)

Expand Down