From afe06db95663cc0cb9704ba4f7014ed9bfb4de09 Mon Sep 17 00:00:00 2001 From: Bilal Mahmoud Date: Fri, 9 Jun 2023 18:21:03 +0200 Subject: [PATCH] feat: implement `crypt(3)` hashers (#3303) This PR implements md5crypt, sha256crypt, sha512crypt, which are considered legacy (like md5), but are used in legacy systems looking to convert to ory. They use the existing format of crypt(5) (which is compliant to PHC). Closes #3291 --- .grype.yaml | 1 + .trivyignore | 1 + go.mod | 4 ++- go.sum | 8 +++-- hash/hash_comparator.go | 77 ++++++++++++++++++++++++++++++++++++++++- hash/hasher_test.go | 48 +++++++++++++++++++++++++ 6 files changed, 135 insertions(+), 4 deletions(-) diff --git a/.grype.yaml b/.grype.yaml index a6ce446b593c..4621efcfbc3c 100644 --- a/.grype.yaml +++ b/.grype.yaml @@ -2,3 +2,4 @@ ignore: - vulnerability: CVE-2015-5237 - vulnerability: CVE-2022-30065 + - vulnerability: CVE-2023-2650 diff --git a/.trivyignore b/.trivyignore index ff3bc6108fc4..a142ea336cff 100644 --- a/.trivyignore +++ b/.trivyignore @@ -1 +1,2 @@ CVE-2022-30065 +CVE-2023-2650 diff --git a/go.mod b/go.mod index 2e3cfb64d24f..baa4dbdc13e9 100644 --- a/go.mod +++ b/go.mod @@ -33,6 +33,7 @@ require ( github.com/duo-labs/webauthn v0.0.0-20220330035159-03696f3d4499 github.com/fatih/color v1.13.0 github.com/ghodss/yaml v1.0.0 + github.com/go-crypt/crypt v0.2.9 github.com/go-errors/errors v1.0.1 github.com/go-openapi/strfmt v0.21.3 github.com/go-playground/validator/v10 v10.4.1 @@ -153,6 +154,7 @@ require ( github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/fullstorydev/grpcurl v1.8.1 // indirect github.com/fxamacker/cbor/v2 v2.4.0 // indirect + github.com/go-crypt/x v0.2.1 // indirect github.com/go-logr/logr v1.2.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-openapi/analysis v0.21.4 // indirect @@ -332,7 +334,7 @@ require ( go.uber.org/zap v1.21.0 // indirect golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17 // indirect golang.org/x/mod v0.10.0 // indirect - golang.org/x/sys v0.7.0 // indirect + golang.org/x/sys v0.8.0 // indirect golang.org/x/term v0.6.0 // indirect golang.org/x/text v0.8.0 // indirect golang.org/x/time v0.1.0 // indirect diff --git a/go.sum b/go.sum index fff69f234884..114351b6f774 100644 --- a/go.sum +++ b/go.sum @@ -377,6 +377,10 @@ github.com/getsentry/raven-go v0.2.0/go.mod h1:KungGk8q33+aIAZUIVWZDr2OfAEBsO49P github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/gliderlabs/ssh v0.2.2/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aevW3Awn0= +github.com/go-crypt/crypt v0.2.9 h1:5gWWTId2Qyqs9ROIsegt5pnqo9wUSRLbhpkR6JSftjg= +github.com/go-crypt/crypt v0.2.9/go.mod h1:JjzdTYE2mArb6nBoIvvpF7o46/rK/1pfmlArCRMTFUk= +github.com/go-crypt/x v0.2.1 h1:OGw78Bswme9lffCOX6tyuC280ouU5391glsvThMtM5U= +github.com/go-crypt/x v0.2.1/go.mod h1:Q/y9rms7yw4/1CavBlNGn0Itg4HqwNpe1N9FX0TxXrc= github.com/go-errors/errors v1.0.1 h1:LUHzmkK3GUKUrL/1gfBUxAHzcev3apQlezX/+O7ma6w= github.com/go-errors/errors v1.0.1/go.mod h1:f4zRHt4oKfwPJE5k8C9vpYG+aDHdBFUsgrm6/TyX73Q= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= @@ -1858,8 +1862,8 @@ golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20221010170243-090e33056c14/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU= -golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU= +golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20191110171634-ad39bd3f0407/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= diff --git a/hash/hash_comparator.go b/hash/hash_comparator.go index c8dfd3316aca..0a0bcf9b9383 100644 --- a/hash/hash_comparator.go +++ b/hash/hash_comparator.go @@ -4,6 +4,7 @@ package hash import ( + "bytes" "context" "crypto/aes" "crypto/cipher" @@ -23,15 +24,43 @@ import ( "golang.org/x/crypto/pbkdf2" "golang.org/x/crypto/scrypt" + "github.com/go-crypt/crypt" + "github.com/go-crypt/crypt/algorithm/md5crypt" + "github.com/go-crypt/crypt/algorithm/shacrypt" + "github.com/ory/kratos/driver/config" ) var ErrUnknownHashAlgorithm = errors.New("unknown hash algorithm") +func NewCryptDecoder() *crypt.Decoder { + decoder := crypt.NewDecoder() + + // The register function only returns an error if the decoder is nil or the algorithm is already registered. + // This is never the case here, if it is, we did something horribly wrong. + if err := md5crypt.RegisterDecoderCommon(decoder); err != nil { + panic(err) + } + + if err := shacrypt.RegisterDecoder(decoder); err != nil { + panic(err) + } + + return decoder +} + +var CryptDecoder = NewCryptDecoder() + func Compare(ctx context.Context, password []byte, hash []byte) error { switch { + case IsMD5CryptHash(hash): + return CompareMD5Crypt(ctx, password, hash) case IsBcryptHash(hash): return CompareBcrypt(ctx, password, hash) + case IsSHA256CryptHash(hash): + return CompareSHA256Crypt(ctx, password, hash) + case IsSHA512CryptHash(hash): + return CompareSHA512Crypt(ctx, password, hash) case IsArgon2idHash(hash): return CompareArgon2id(ctx, password, hash) case IsArgon2iHash(hash): @@ -53,6 +82,16 @@ func Compare(ctx context.Context, password []byte, hash []byte) error { } } +func CompareMD5Crypt(_ context.Context, password []byte, hash []byte) error { + // the password has successfully been validated (has prefix `$md5-crypt`), + // the decoder expect the module crypt identifier instead (`$1`), which means we need to replace the prefix + // before decoding + hash = bytes.TrimPrefix(hash, []byte("$md5-crypt")) + hash = append([]byte("$1"), hash...) + + return compareCryptHelper(password, string(hash)) +} + func CompareBcrypt(_ context.Context, password []byte, hash []byte) error { if err := validateBcryptPasswordLength(password); err != nil { return err @@ -69,6 +108,20 @@ func CompareBcrypt(_ context.Context, password []byte, hash []byte) error { return nil } +func CompareSHA256Crypt(_ context.Context, password []byte, hash []byte) error { + hash = bytes.TrimPrefix(hash, []byte("$sha256-crypt")) + hash = append([]byte("$5"), hash...) + + return compareCryptHelper(password, string(hash)) +} + +func CompareSHA512Crypt(_ context.Context, password []byte, hash []byte) error { + hash = bytes.TrimPrefix(hash, []byte("$sha512-crypt")) + hash = append([]byte("$6"), hash...) + + return compareCryptHelper(password, string(hash)) +} + func CompareArgon2id(_ context.Context, password []byte, hash []byte) error { // Extract the parameters, salt and derived key from the encoded password // hash. @@ -200,7 +253,10 @@ func CompareMD5(_ context.Context, password []byte, hash []byte) error { } var ( + isMD5CryptHash = regexp.MustCompile(`^\$md5-crypt\$`) isBcryptHash = regexp.MustCompile(`^\$2[abzy]?\$`) + isSHA256CryptHash = regexp.MustCompile(`^\$sha256-crypt\$`) + isSHA512CryptHash = regexp.MustCompile(`^\$sha512-crypt\$`) isArgon2idHash = regexp.MustCompile(`^\$argon2id\$`) isArgon2iHash = regexp.MustCompile(`^\$argon2i\$`) isPbkdf2Hash = regexp.MustCompile(`^\$pbkdf2-sha[0-9]{1,3}\$`) @@ -211,7 +267,10 @@ var ( isMD5Hash = regexp.MustCompile(`^\$md5\$`) ) +func IsMD5CryptHash(hash []byte) bool { return isMD5CryptHash.Match(hash) } func IsBcryptHash(hash []byte) bool { return isBcryptHash.Match(hash) } +func IsSHA256CryptHash(hash []byte) bool { return isSHA256CryptHash.Match(hash) } +func IsSHA512CryptHash(hash []byte) bool { return isSHA512CryptHash.Match(hash) } func IsArgon2idHash(hash []byte) bool { return isArgon2idHash.Match(hash) } func IsArgon2iHash(hash []byte) bool { return isArgon2iHash.Match(hash) } func IsPbkdf2Hash(hash []byte) bool { return isPbkdf2Hash.Match(hash) } @@ -222,7 +281,10 @@ func IsFirebaseScryptHash(hash []byte) bool { return isFirebaseScryptHash.Match( func IsMD5Hash(hash []byte) bool { return isMD5Hash.Match(hash) } func IsValidHashFormat(hash []byte) bool { - if IsBcryptHash(hash) || + if IsMD5CryptHash(hash) || + IsBcryptHash(hash) || + IsSHA256CryptHash(hash) || + IsSHA512CryptHash(hash) || IsArgon2idHash(hash) || IsArgon2iHash(hash) || IsPbkdf2Hash(hash) || @@ -397,6 +459,19 @@ func compareSHAHelper(hasher string, raw []byte, hash []byte) error { return comparePasswordHashConstantTime(encodedHash, newEncodedHash) } +func compareCryptHelper(password []byte, hash string) error { + digest, err := CryptDecoder.Decode(hash) + if err != nil { + return err + } + + if digest.MatchBytes(password) { + return nil + } + + return errors.WithStack(ErrMismatchedHashAndPassword) +} + // decodeSSHAHash decodes SSHA[1|256|512] encoded password hash in usual {SSHA...} format. func decodeSSHAHash(encodedHash string) (hasher string, salt, hash []byte, err error) { re := regexp.MustCompile(`\{([^}]*)\}`) diff --git a/hash/hasher_test.go b/hash/hasher_test.go index 030898ce63d6..f2fa9d4fdab7 100644 --- a/hash/hasher_test.go +++ b/hash/hasher_test.go @@ -365,4 +365,52 @@ func TestCompare(t *testing.T) { assert.Nil(t, hash.CompareMD5(context.Background(), []byte("ory"), []byte("$md5$pf=e1BBU1NXT1JEfXtTQUxUfSQ/$MTIzNDU2Nzg5$8PhwWanVRnpJAFK4NUjR0w=="))) // pf={PASSWORD}{SALT}$? salt=123456789 assert.Error(t, hash.CompareMD5(context.Background(), []byte("ory1"), []byte("$md5$pf=e1BBU1NXT1JEfXtTQUxUfSQ/$MTIzNDU2Nzg5$8PhwWanVRnpJAFK4NUjR0w=="))) }) + + t.Run("md5-crypt", func(t *testing.T) { + t.Parallel() + + assert.Nil(t, hash.Compare(context.Background(), []byte("test"), []byte("$md5-crypt$TVEiiKNb$SN6/pUaRQS/E8Jh46As2C/"))) + assert.Nil(t, hash.CompareMD5Crypt(context.Background(), []byte("test"), []byte("$md5-crypt$TVEiiKNb$SN6/pUaRQS/E8Jh46As2C/"))) + assert.Nil(t, hash.Compare(context.Background(), []byte("test"), []byte("$md5-crypt$$whuMjZj.HMFoaTaZRRtkO0"))) + assert.Error(t, hash.Compare(context.Background(), []byte("test"), []byte("$md5-crypt$xWMlm2eL$GGTOpgZu4p2k6ORprAu3b."))) + + assert.Nil(t, hash.Compare(context.Background(), []byte("ory"), []byte("$md5-crypt$xWMlm2eL$GGTOpgZu4p2k6ORprAu3b."))) + assert.Nil(t, hash.CompareMD5Crypt(context.Background(), []byte("ory"), []byte("$md5-crypt$xWMlm2eL$GGTOpgZu4p2k6ORprAu3b."))) + assert.Error(t, hash.Compare(context.Background(), []byte("ory"), []byte("$md5-crypt$E7zjruqF$RTglYR1CzBHwwiTk9nVzx1"))) + + assert.ErrorIs(t, hash.Compare(context.Background(), []byte("ory"), []byte("$md5-crypt$$")), hash.ErrMismatchedHashAndPassword) + assert.Error(t, hash.Compare(context.Background(), []byte("ory"), []byte("$md5-crypt$$$"))) + // per crypt(5), `md5crypt` can be run without a salt, but the salt section must still be present + assert.Error(t, hash.Compare(context.Background(), []byte("test"), []byte("$md5-crypt$whuMjZj.HMFoaTaZRRtkO0")), "md5crypt decode error: provided encoded hash has an invalid format") + }) + + t.Run("sha256-crypt", func(t *testing.T) { + t.Parallel() + + assert.Nil(t, hash.Compare(context.Background(), []byte("test"), []byte("$sha256-crypt$rounds=535000$05R.9KB6UC2kLI3w$Q/zslzx./JjkAVPTwp6th7nW5l7JU91Gte/UmIh.U78"))) + assert.Nil(t, hash.CompareSHA256Crypt(context.Background(), []byte("test"), []byte("$sha256-crypt$rounds=535000$05R.9KB6UC2kLI3w$Q/zslzx./JjkAVPTwp6th7nW5l7JU91Gte/UmIh.U78"))) + assert.Error(t, hash.Compare(context.Background(), []byte("test"), []byte("$sha256-crypt$rounds=535000$awpcR7lDlnK/S7WE$vHU7KkQwyjfGz6u4MUi7.lH9htK/l63HloTsX1ZMz.3"))) + + assert.Nil(t, hash.Compare(context.Background(), []byte("ory"), []byte("$sha256-crypt$rounds=535000$awpcR7lDlnK/S7WE$vHU7KkQwyjfGz6u4MUi7.lH9htK/l63HloTsX1ZMz.3"))) + assert.Nil(t, hash.CompareSHA256Crypt(context.Background(), []byte("ory"), []byte("$sha256-crypt$rounds=535000$awpcR7lDlnK/S7WE$vHU7KkQwyjfGz6u4MUi7.lH9htK/l63HloTsX1ZMz.3"))) + assert.Error(t, hash.Compare(context.Background(), []byte("ory"), []byte("$sha256-crypt$rounds=535000$T95kH8e37IGVdxzJ$gLeaNa6qRog.bx4Bzqp63ceWItH6nSAal6c3WmT5GHB"))) + + assert.Error(t, hash.Compare(context.Background(), []byte("ory"), []byte("$sha256-crypt$$")), "shacrypt decode error: provided encoded hash has an invalid format") + assert.Error(t, hash.Compare(context.Background(), []byte("ory"), []byte("$sha256-crypt$$$"))) + }) + + t.Run("sha512-crypt", func(t *testing.T) { + t.Parallel() + + assert.Nil(t, hash.Compare(context.Background(), []byte("test"), []byte("$sha512-crypt$rounds=656000$3LVbIAVxR//cRajw$uuNasMW.RYxlGzIRFU1Was70BPSa933AjxhZIGJdJBOlqJAHlgqa0yuiuq5JHF/ryNGryJkj87G9i3G2HPSXg1"))) + assert.Nil(t, hash.CompareSHA512Crypt(context.Background(), []byte("test"), []byte("$sha512-crypt$rounds=656000$3LVbIAVxR//cRajw$uuNasMW.RYxlGzIRFU1Was70BPSa933AjxhZIGJdJBOlqJAHlgqa0yuiuq5JHF/ryNGryJkj87G9i3G2HPSXg1"))) + assert.Error(t, hash.Compare(context.Background(), []byte("test"), []byte("$5$rounds=535000$awpcR7lDlnK/S7WE$vHU7KkQwyjfGz6u4MUi7.lH9htK/l63HloTsX1ZMz.3"))) + + assert.Nil(t, hash.Compare(context.Background(), []byte("ory"), []byte("$sha512-crypt$rounds=656000$0baQbxBrfpKqvizk$Q9cYk1MeNAlECPgpG3jjfNI2DumLqd0yHbxzLdxiX6nsSD5i9n0awcbiCf8R5DzpIYxeBPznPcb1wtzlgUKtH0"))) + assert.Nil(t, hash.CompareSHA512Crypt(context.Background(), []byte("ory"), []byte("$sha512-crypt$rounds=656000$0baQbxBrfpKqvizk$Q9cYk1MeNAlECPgpG3jjfNI2DumLqd0yHbxzLdxiX6nsSD5i9n0awcbiCf8R5DzpIYxeBPznPcb1wtzlgUKtH0"))) + assert.Error(t, hash.Compare(context.Background(), []byte("ory"), []byte("$sha512-crypt$rounds=656000$hNcDLFO63bkYVDZf$Mt9dhH0xqfxWZ6Pu8zXw.Ku5f15IRTweuaDcUc.ObXWGn7B1h8YIWLmArZd8psd2mrUVswCXLAVptmISr.8iI/"))) + + assert.Error(t, hash.Compare(context.Background(), []byte("ory"), []byte("$sha512-crypt$$")), "shacrypt decode error: provided encoded hash has an invalid format") + assert.Error(t, hash.Compare(context.Background(), []byte("ory"), []byte("$sha512-crypt$$$"))) + }) }