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

Commit44c04ec

Browse files
committed
fix: user passwords cleanup
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
1 parent8661f92 commit44c04ec

File tree

4 files changed

+176
-27
lines changed

4 files changed

+176
-27
lines changed
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
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(cryptorand.MustString(16))
14+
secret= []byte(cryptorand.MustString(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+
}

‎coderd/userpassword/userpassword.go

Lines changed: 64 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,69 @@ 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"
16+
17+
"github.com/coder/coder/cryptorand"
1418
)
1519

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.
20+
var (
21+
// The base64 encoder used when producing the string representation of
22+
// hashes.
23+
base64Encoder=base64.RawStdEncoding
24+
25+
// The number of iterations to use when generating the hash. This was chosen
26+
// to make it about as fast as bcrypt hashes. Increasing this causes hashes
27+
// to take longer to compute.
28+
defaultHashIter=65535
29+
30+
// This is the length of our output hash. bcrypt has a hash size of up to
31+
// 60, so we rounded up to a power of 8.
1932
hashLength=64
33+
2034
// The scheme to include in our hashed password.
2135
hashScheme="pbkdf2-sha256"
36+
37+
// A salt size of 16 is the default in passlib. A minimum of 8 can be safely
38+
// used.
39+
saltSize=16
40+
41+
// The simulated hash is used when trying to simulate password checks for
42+
// users that don't exist.
43+
simulatedHash,_=Hash(cryptorand.MustString(10))
2244
)
2345

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

5094
ifsubtle.ConstantTimeCompare([]byte(hashWithSaltAndIter(password,salt,iter)), []byte(hashed))!=1 {
5195
returnfalse,nil
5296
}
97+
5398
returntrue,nil
5499
}
55100

56101
// Hash generates a hash using pbkdf2.
57102
// See the Compare() comment for rationale.
58103
funcHash(passwordstring) (string,error) {
59-
// bcrypt uses a salt size of 16 bytes.
60-
salt:=make([]byte,16)
104+
salt:=make([]byte,saltSize)
61105
_,err:=rand.Read(salt)
62106
iferr!=nil {
63107
return"",xerrors.Errorf("read random bytes for salt: %w",err)
64108
}
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
109+
110+
returnhashWithSaltAndIter(password,salt,defaultHashIter),nil
68111
}
69112

70113
// Produces a string representation of the hash.
71114
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)
115+
var (
116+
hash=pbkdf2.Key([]byte(password),salt,iter,hashLength,sha256.New)
117+
encHash=make([]byte,base64Encoder.EncodedLen(len(hash)))
118+
encSalt=make([]byte,base64Encoder.EncodedLen(len(salt)))
119+
)
120+
121+
base64Encoder.Encode(encHash,hash)
122+
base64Encoder.Encode(encSalt,salt)
123+
124+
returnfmt.Sprintf("$%s$%d$%s$%s",hashScheme,iter,encSalt,encHash)
78125
}

‎coderd/users.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -419,21 +419,19 @@ func (api *api) postLogin(rw http.ResponseWriter, r *http.Request) {
419419
if!httpapi.Read(rw,r,&loginWithPassword) {
420420
return
421421
}
422+
422423
user,err:=api.Database.GetUserByEmailOrUsername(r.Context(), database.GetUserByEmailOrUsernameParams{
423424
Email:loginWithPassword.Email,
424425
})
425-
iferrors.Is(err,sql.ErrNoRows) {
426-
httpapi.Write(rw,http.StatusUnauthorized, httpapi.Response{
427-
Message:"invalid email or password",
428-
})
429-
return
430-
}
431-
iferr!=nil {
426+
iferr!=nil&&!xerrors.Is(err,sql.ErrNoRows) {
432427
httpapi.Write(rw,http.StatusInternalServerError, httpapi.Response{
433428
Message:fmt.Sprintf("get user: %s",err.Error()),
434429
})
435430
return
436431
}
432+
433+
// If the user doesn't exist, it will be a default struct.
434+
437435
equal,err:=userpassword.Compare(string(user.HashedPassword),loginWithPassword.Password)
438436
iferr!=nil {
439437
httpapi.Write(rw,http.StatusInternalServerError, httpapi.Response{

‎cryptorand/strings.go

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"crypto/rand"
55
"encoding/binary"
66
"strings"
7+
8+
"golang.org/x/xerrors"
79
)
810

911
// Charsets
@@ -32,7 +34,7 @@ const (
3234
Human="23456789abcdefghjkmnpqrstuvwxyz"
3335
)
3436

35-
// StringCharset generates a random string using the provided charset and size
37+
// StringCharset generates a random string using the provided charset and size.
3638
funcStringCharset(charSetStrstring,sizeint) (string,error) {
3739
charSet:= []rune(charSetStr)
3840

@@ -67,18 +69,58 @@ func StringCharset(charSetStr string, size int) (string, error) {
6769
returnbuf.String(),nil
6870
}
6971

72+
// MustStringCharset generates a random string of the given length, using the
73+
// provided charset. It will panic if an error occurs.
74+
funcMustStringCharset(charSetstring,sizeint)string {
75+
s,err:=StringCharset(charSet,size)
76+
must(err)
77+
returns
78+
}
79+
7080
// String returns a random string using Default.
7181
funcString(sizeint) (string,error) {
7282
returnStringCharset(Default,size)
7383
}
7484

85+
// MustString generates a random string of the given length, using
86+
// the Default charset. It will panic if an error occurs.
87+
funcMustString(sizeint)string {
88+
s,err:=String(size)
89+
must(err)
90+
returns
91+
}
92+
7593
// HexString returns a hexadecimal string of given length.
7694
funcHexString(sizeint) (string,error) {
7795
returnStringCharset(Hex,size)
7896
}
7997

80-
// Sha1String returns a 40-character hexadecimal string, which matches
81-
// the length of a SHA-1 hash (160 bits).
98+
// MustHexString generates a random hexadecimal string of the given
99+
// length. It will panic if an error occurs.
100+
funcMustHexString(sizeint)string {
101+
s,err:=HexString(size)
102+
must(err)
103+
returns
104+
}
105+
106+
// Sha1String returns a 40-character hexadecimal string, which matches the
107+
// length of a SHA-1 hash (160 bits).
82108
funcSha1String() (string,error) {
83109
returnStringCharset(Hex,40)
84110
}
111+
112+
// MustSha1String returns a 40-character hexadecimal string, which matches the
113+
// length of a SHA-1 hash (160 bits). It will panic if an error occurs.
114+
funcMustSha1String()string {
115+
s,err:=Sha1String()
116+
must(err)
117+
returns
118+
}
119+
120+
// must is a utility function that panics with the given error if
121+
// err is non-nil.
122+
funcmust(errerror) {
123+
iferr!=nil {
124+
panic(xerrors.Errorf("crand: %w",err))
125+
}
126+
}

0 commit comments

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