Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit6accc9d

Browse files
coadlerkylecarbs
authored andcommitted
fix: user passwords cleanup (#1202)
1. Adds benchmarks comparing bcrypt and our pbkdf2 settings1. Changes the pbkdf2 hash iterations back to 65k. 1024 is insecure1. Gets rid of the short circuit when the user isn't found, preventing timing attacks which can reveal which emails exist on a deployment```$ go test -bench .goos: linuxgoarch: amd64pkg: github.com/coder/coder/coderd/userpasswordcpu: Intel(R) Core(TM) i9-9900K CPU @ 3.60GHzBenchmarkBcryptMinCost-16 1651 702727 ns/op 5165 B/op 10 allocs/opBenchmarkPbkdf2MinCost-16 1669 714843 ns/op 804 B/op 10 allocs/opBenchmarkBcryptDefaultCost-16 27 42676316 ns/op 5246 B/op 10 allocs/opBenchmarkPbkdf2-16 26 45902236 ns/op 804 B/op 10 allocs/opPASSok github.com/coder/coder/coderd/userpassword5.036s```
1 parenta4c1a0b commit6accc9d

File tree

3 files changed

+137
-24
lines changed

3 files changed

+137
-24
lines changed
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package userpassword_test
2+
3+
import (
4+
"crypto/sha256"
5+
"testing"
6+
7+
"github.com/coder/coder/cryptorand"
8+
"golang.org/x/crypto/bcrypt"
9+
"golang.org/x/crypto/pbkdf2"
10+
)
11+
12+
var (
13+
salt= []byte(must(cryptorand.String(16)))
14+
secret= []byte(must(cryptorand.String(24)))
15+
16+
resBcrypt []byte
17+
resPbkdf2 []byte
18+
)
19+
20+
funcBenchmarkBcryptMinCost(b*testing.B) {
21+
varr []byte
22+
b.ReportAllocs()
23+
24+
fori:=0;i<b.N;i++ {
25+
r,_=bcrypt.GenerateFromPassword(secret,bcrypt.MinCost)
26+
}
27+
28+
resBcrypt=r
29+
}
30+
31+
funcBenchmarkPbkdf2MinCost(b*testing.B) {
32+
varr []byte
33+
b.ReportAllocs()
34+
35+
fori:=0;i<b.N;i++ {
36+
r=pbkdf2.Key(secret,salt,1024,64,sha256.New)
37+
}
38+
39+
resPbkdf2=r
40+
}
41+
42+
funcBenchmarkBcryptDefaultCost(b*testing.B) {
43+
varr []byte
44+
b.ReportAllocs()
45+
46+
fori:=0;i<b.N;i++ {
47+
r,_=bcrypt.GenerateFromPassword(secret,bcrypt.DefaultCost)
48+
}
49+
50+
resBcrypt=r
51+
}
52+
53+
funcBenchmarkPbkdf2(b*testing.B) {
54+
varr []byte
55+
b.ReportAllocs()
56+
57+
fori:=0;i<b.N;i++ {
58+
r=pbkdf2.Key(secret,salt,65536,64,sha256.New)
59+
}
60+
61+
resPbkdf2=r
62+
}
63+
64+
funcmust(sstring,errerror)string {
65+
iferr!=nil {
66+
panic(err)
67+
}
68+
69+
returns
70+
}

‎coderd/userpassword/userpassword.go

Lines changed: 62 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,67 @@ import (
66
"crypto/subtle"
77
"encoding/base64"
88
"fmt"
9+
"os"
910
"strconv"
1011
"strings"
1112

1213
"golang.org/x/crypto/pbkdf2"
14+
"golang.org/x/exp/slices"
1315
"golang.org/x/xerrors"
1416
)
1517

16-
const (
17-
// This is the length of our output hash.
18-
// bcrypt has a hash size of 59, so we rounded up to a power of 8.
18+
var (
19+
// The base64 encoder used when producing the string representation of
20+
// hashes.
21+
base64Encoding=base64.RawStdEncoding
22+
23+
// The number of iterations to use when generating the hash. This was chosen
24+
// to make it about as fast as bcrypt hashes. Increasing this causes hashes
25+
// to take longer to compute.
26+
defaultHashIter=65535
27+
28+
// This is the length of our output hash. bcrypt has a hash size of up to
29+
// 60, so we rounded up to a power of 8.
1930
hashLength=64
31+
2032
// The scheme to include in our hashed password.
2133
hashScheme="pbkdf2-sha256"
34+
35+
// A salt size of 16 is the default in passlib. A minimum of 8 can be safely
36+
// used.
37+
defaultSaltSize=16
38+
39+
// The simulated hash is used when trying to simulate password checks for
40+
// users that don't exist.
41+
simulatedHash,_=Hash("hunter2")
2242
)
2343

24-
// Compare checks the equality of passwords from a hashed pbkdf2 string.
25-
// This uses pbkdf2 to ensure FIPS 140-2 compliance. See:
44+
// Make password hashing much faster in tests.
45+
funcinit() {
46+
args:=os.Args[1:]
47+
48+
// Ensure this can never be enabled if running in server mode.
49+
ifslices.Contains(args,"server") {
50+
return
51+
}
52+
53+
for_,flag:=rangeargs {
54+
ifstrings.HasPrefix(flag,"-test.") {
55+
defaultHashIter=1
56+
return
57+
}
58+
}
59+
}
60+
61+
// Compare checks the equality of passwords from a hashed pbkdf2 string. This
62+
// uses pbkdf2 to ensure FIPS 140-2 compliance. See:
2663
// https://csrc.nist.gov/csrc/media/templates/cryptographic-module-validation-program/documents/security-policies/140sp2261.pdf
2764
funcCompare(hashedstring,passwordstring) (bool,error) {
65+
// If the hased password provided is empty, simulate comparing a real hash.
66+
ifhashed=="" {
67+
hashed=simulatedHash
68+
}
69+
2870
iflen(hashed)<hashLength {
2971
returnfalse,xerrors.Errorf("hash too short: %d",len(hashed))
3072
}
@@ -42,37 +84,40 @@ func Compare(hashed string, password string) (bool, error) {
4284
iferr!=nil {
4385
returnfalse,xerrors.Errorf("parse iter from hash: %w",err)
4486
}
45-
salt,err:=base64.RawStdEncoding.DecodeString(parts[3])
87+
salt,err:=base64Encoding.DecodeString(parts[3])
4688
iferr!=nil {
4789
returnfalse,xerrors.Errorf("decode salt: %w",err)
4890
}
4991

5092
ifsubtle.ConstantTimeCompare([]byte(hashWithSaltAndIter(password,salt,iter)), []byte(hashed))!=1 {
5193
returnfalse,nil
5294
}
95+
5396
returntrue,nil
5497
}
5598

5699
// Hash generates a hash using pbkdf2.
57100
// See the Compare() comment for rationale.
58101
funcHash(passwordstring) (string,error) {
59-
// bcrypt uses a salt size of 16 bytes.
60-
salt:=make([]byte,16)
102+
salt:=make([]byte,defaultSaltSize)
61103
_,err:=rand.Read(salt)
62104
iferr!=nil {
63105
return"",xerrors.Errorf("read random bytes for salt: %w",err)
64106
}
65-
// The default hash iteration is 1024 for speed.
66-
// As this is increased, the password is hashed more.
67-
returnhashWithSaltAndIter(password,salt,1024),nil
107+
108+
returnhashWithSaltAndIter(password,salt,defaultHashIter),nil
68109
}
69110

70111
// Produces a string representation of the hash.
71112
funchashWithSaltAndIter(passwordstring,salt []byte,iterint)string {
72-
hash:=pbkdf2.Key([]byte(password),salt,iter,hashLength,sha256.New)
73-
hash= []byte(base64.RawStdEncoding.EncodeToString(hash))
74-
salt= []byte(base64.RawStdEncoding.EncodeToString(salt))
75-
// This format is similar to bcrypt. See:
76-
// https://en.wikipedia.org/wiki/Bcrypt#Description
77-
returnfmt.Sprintf("$%s$%d$%s$%s",hashScheme,iter,salt,hash)
113+
var (
114+
hash=pbkdf2.Key([]byte(password),salt,iter,hashLength,sha256.New)
115+
encHash=make([]byte,base64Encoding.EncodedLen(len(hash)))
116+
encSalt=make([]byte,base64Encoding.EncodedLen(len(salt)))
117+
)
118+
119+
base64Encoding.Encode(encHash,hash)
120+
base64Encoding.Encode(encSalt,salt)
121+
122+
returnfmt.Sprintf("$%s$%d$%s$%s",hashScheme,iter,encSalt,encHash)
78123
}

‎coderd/users.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -461,21 +461,19 @@ func (api *api) postLogin(rw http.ResponseWriter, r *http.Request) {
461461
if!httpapi.Read(rw,r,&loginWithPassword) {
462462
return
463463
}
464+
464465
user,err:=api.Database.GetUserByEmailOrUsername(r.Context(), database.GetUserByEmailOrUsernameParams{
465466
Email:loginWithPassword.Email,
466467
})
467-
iferrors.Is(err,sql.ErrNoRows) {
468-
httpapi.Write(rw,http.StatusUnauthorized, httpapi.Response{
469-
Message:"invalid email or password",
470-
})
471-
return
472-
}
473-
iferr!=nil {
468+
iferr!=nil&&!xerrors.Is(err,sql.ErrNoRows) {
474469
httpapi.Write(rw,http.StatusInternalServerError, httpapi.Response{
475470
Message:fmt.Sprintf("get user: %s",err.Error()),
476471
})
477472
return
478473
}
474+
475+
// If the user doesn't exist, it will be a default struct.
476+
479477
equal,err:=userpassword.Compare(string(user.HashedPassword),loginWithPassword.Password)
480478
iferr!=nil {
481479
httpapi.Write(rw,http.StatusInternalServerError, httpapi.Response{

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp