[crypto] bcrypt: reject passwords longer than 72 bytes
Roland Shoemaker (Gerrit)
Roland Shoemaker has uploaded this change for review.
bcrypt: reject passwords longer than 72 bytes
By design, bcrypt only uses the first 72 bytes of a password when
generating a hash. Most implementations, including the reference one,
simply silently ignore any trailing input when provided passwords longer
than 72 bytes. This can cause confusion for users who expect the entire
password to be used to generate the hash.
In GenerateFromPassword, reject passwords longer than 72 bytes.
CompareHashAndPassword will still accept these passwords, since we
cannot break hashes that have already been stored.
Fixes golang/go#36546
Change-Id: I039addd2a2961a7fa9d1e4a3e892a9e3c8bf4c9a
---
M bcrypt/bcrypt.go
M bcrypt/bcrypt_test.go
2 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/bcrypt/bcrypt.go b/bcrypt/bcrypt.go
index addf56b..8602315 100644
--- a/bcrypt/bcrypt.go
+++ b/bcrypt/bcrypt.go
@@ -82,11 +82,18 @@
minor byte
}
+var errPasswordTooLong = errors.New("crypto/bcrypt: password too long")
+
// GenerateFromPassword returns the bcrypt hash of the password at the given
// cost. If the cost given is less than MinCost, the cost will be set to
// DefaultCost, instead. Use CompareHashAndPassword, as defined in this package,
// to compare the returned hashed password with its cleartext version.
+// GenerateFromPassword does not accept passwords longer than 72 bytes, which
+// is the longest password bcrypt will operate on.
func GenerateFromPassword(password []byte, cost int) ([]byte, error) {
+ if len(password) > 72 {
+ return nil, errPasswordTooLong
+ }
p, err := newFromPassword(password, cost)
if err != nil {
return nil, err
diff --git a/bcrypt/bcrypt_test.go b/bcrypt/bcrypt_test.go
index b7162d8..77dd3dc 100644
--- a/bcrypt/bcrypt_test.go
+++ b/bcrypt/bcrypt_test.go
@@ -241,3 +241,10 @@
t.Errorf("got=%q want=%q", got, want)
}
}
+
+func TestPasswordTooLong(t *testing.T) {
+ _, err := GenerateFromPassword(make([]byte, 73), 1)
+ if err != errPasswordTooLong {
+ t.Errorf("unexpected error: got %q, want %q", err, errPasswordTooLong)
+ }
+}
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.
Roland Shoemaker (Gerrit)
Attention is currently required from: Filippo Valsorda.
Patch set 1:Run-TryBot +1Auto-Submit +1
Dan Kortschak (Gerrit)
Attention is currently required from: Filippo Valsorda, Roland Shoemaker.
1 comment:
File bcrypt/bcrypt.go:
Patch Set #1, Line 85:
var errPasswordTooLong = errors.New("crypto/bcrypt: password too long")Is it worth making this an exported label so that client code can differentiate between this case and errors in `newFromPassword`?
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.
Filippo Valsorda (Gerrit)
Attention is currently required from: Roland Shoemaker.
Patch set 1:Code-Review +2
2 comments:
Patchset:
Let's give the discussion on the issue a week or so to tell us why this would break people https://go.dev/issue/36546
File bcrypt/bcrypt.go:
Patch Set #1, Line 85:
crypto/bcryptbcrypt
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.
Roland Shoemaker (Gerrit)
Attention is currently required from: Roland Shoemaker.
Roland Shoemaker uploaded patch set #2 to this change.
The following approvals got outdated and were removed: Auto-Submit+1 by Roland Shoemaker, Run-TryBot+1 by Roland Shoemaker, TryBot-Result+1 by Gopher Robot
bcrypt: reject passwords longer than 72 bytes
By design, bcrypt only uses the first 72 bytes of a password when
generating a hash. Most implementations, including the reference one,
simply silently ignore any trailing input when provided passwords longer
than 72 bytes. This can cause confusion for users who expect the entire
password to be used to generate the hash.
In GenerateFromPassword, reject passwords longer than 72 bytes.
CompareHashAndPassword will still accept these passwords, since we
cannot break hashes that have already been stored.
Fixes golang/go#36546
Change-Id: I039addd2a2961a7fa9d1e4a3e892a9e3c8bf4c9a
---
M bcrypt/bcrypt.go
M bcrypt/bcrypt_test.go
2 files changed, 37 insertions(+), 0 deletions(-)
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.
Roland Shoemaker (Gerrit)
Attention is currently required from: Dan Kortschak.
Patch set 2:Run-TryBot +1Auto-Submit +1
3 comments:
Patchset:
Let's give the discussion on the issue a week or so to tell us why this would break people https://g […]
Looks like there haven't been any objections.
File bcrypt/bcrypt.go:
Patch Set #1, Line 85:
var errPasswordTooLong = errors.New("crypto/bcrypt: password too long")Is it worth making this an exported label so that client code can differentiate between this case an […]
Done
Patch Set #1, Line 85:
crypto/bcryptbcrypt
Done
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.
Jason McNeil (Gerrit)
Attention is currently required from: Dan Kortschak, Roland Shoemaker.
Patch set 2:Code-Review +1
1 comment:
Patchset:
this is a good solution that doesn’t break compatibility with hashes but helps ensure longer passwords aren’t used with silent truncation.
this should be accepted.
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.
Sam Mortimer (Gerrit)
Attention is currently required from: Dan Kortschak, Roland Shoemaker.
1 comment:
File bcrypt/bcrypt.go:
Patch Set #2, Line 87:
var ErrPasswordTooLong = errors.New("bcrypt: password too long")how about: "bcrypt: password length exceeds 72 bytes"
so the user knows doesn't have to research what "too long" means
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.
Roland Shoemaker (Gerrit)
Attention is currently required from: Dan Kortschak, Roland Shoemaker.
Roland Shoemaker uploaded patch set #4 to this change.
bcrypt: reject passwords longer than 72 bytes
By design, bcrypt only uses the first 72 bytes of a password when
generating a hash. Most implementations, including the reference one,
simply silently ignore any trailing input when provided passwords longer
than 72 bytes. This can cause confusion for users who expect the entire
password to be used to generate the hash.
In GenerateFromPassword, reject passwords longer than 72 bytes.
CompareHashAndPassword will still accept these passwords, since we
cannot break hashes that have already been stored.
Fixes golang/go#36546
Change-Id: I039addd2a2961a7fa9d1e4a3e892a9e3c8bf4c9a
---
M bcrypt/bcrypt.go
M bcrypt/bcrypt_test.go
2 files changed, 37 insertions(+), 0 deletions(-)
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.
Roland Shoemaker (Gerrit)
Attention is currently required from: Dan Kortschak, Jason McNeil, Sam Mortimer.
Patch set 4:Run-TryBot +1Auto-Submit +1
2 comments:
Patchset:
this is a good solution that doesn’t break compatibility with hashes but helps ensure longer passwor […]
Ack
File bcrypt/bcrypt.go:
Patch Set #2, Line 87:
var ErrPasswordTooLong = errors.New("bcrypt: password too long")
how about: "bcrypt: password length exceeds 72 bytes" […]
Done
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Attention is currently required from: Dan Kortschak, Jason McNeil, Roland Shoemaker, Sam Mortimer.
Patch set 4:Code-Review +1
Gopher Robot (Gerrit)
Gopher Robot submitted this change.
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: bcrypt/bcrypt_test.go
Insertions: 2, Deletions: 2.
@@ -244,7 +244,7 @@
func TestPasswordTooLong(t *testing.T) {
_, err := GenerateFromPassword(make([]byte, 73), 1)
- if err != errPasswordTooLong {
- t.Errorf("unexpected error: got %q, want %q", err, errPasswordTooLong)
+ if err != ErrPasswordTooLong {
+ t.Errorf("unexpected error: got %q, want %q", err, ErrPasswordTooLong)
}
}
```
```
The name of the file: bcrypt/bcrypt.go
Insertions: 4, Deletions: 2.
@@ -82,7 +82,9 @@
minor byte
}
-var errPasswordTooLong = errors.New("crypto/bcrypt: password too long")
+// ErrPasswordTooLong is returned when the password passed to
+// GenerateFromPassword is too long (i.e. > 72 bytes).
+var ErrPasswordTooLong = errors.New("bcrypt: password length exceeds 72 bytes")
// GenerateFromPassword returns the bcrypt hash of the password at the given
// cost. If the cost given is less than MinCost, the cost will be set to
@@ -92,7 +94,7 @@
// is the longest password bcrypt will operate on.
func GenerateFromPassword(password []byte, cost int) ([]byte, error) {
if len(password) > 72 {
- return nil, errPasswordTooLong
+ return nil, ErrPasswordTooLong
}
p, err := newFromPassword(password, cost)
if err != nil {
```
bcrypt: reject passwords longer than 72 bytes
By design, bcrypt only uses the first 72 bytes of a password when
generating a hash. Most implementations, including the reference one,
simply silently ignore any trailing input when provided passwords longer
than 72 bytes. This can cause confusion for users who expect the entire
password to be used to generate the hash.
In GenerateFromPassword, reject passwords longer than 72 bytes.
CompareHashAndPassword will still accept these passwords, since we
cannot break hashes that have already been stored.
Fixes golang/go#36546
Change-Id: I039addd2a2961a7fa9d1e4a3e892a9e3c8bf4c9a
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/450415
Reviewed-by: Damien Neil <dn...@google.com>
Reviewed-by: Jason McNeil <jmc...@x2studios.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Reviewed-by: Filippo Valsorda <fil...@golang.org>
Auto-Submit: Roland Shoemaker <rol...@golang.org>
Run-TryBot: Roland Shoemaker <rol...@golang.org>
---
M bcrypt/bcrypt.go
M bcrypt/bcrypt_test.go
2 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/bcrypt/bcrypt.go b/bcrypt/bcrypt.go
index addf56b..5577c0f 100644
--- a/bcrypt/bcrypt.go
+++ b/bcrypt/bcrypt.go
@@ -82,11 +82,20 @@
minor byte
}
+// ErrPasswordTooLong is returned when the password passed to
+// GenerateFromPassword is too long (i.e. > 72 bytes).
+var ErrPasswordTooLong = errors.New("bcrypt: password length exceeds 72 bytes")
+
// GenerateFromPassword returns the bcrypt hash of the password at the given
// cost. If the cost given is less than MinCost, the cost will be set to
// DefaultCost, instead. Use CompareHashAndPassword, as defined in this package,
// to compare the returned hashed password with its cleartext version.
+// GenerateFromPassword does not accept passwords longer than 72 bytes, which
+// is the longest password bcrypt will operate on.
func GenerateFromPassword(password []byte, cost int) ([]byte, error) {
+ if len(password) > 72 {
+ return nil, ErrPasswordTooLong
+ }
p, err := newFromPassword(password, cost)
if err != nil {
return nil, err
diff --git a/bcrypt/bcrypt_test.go b/bcrypt/bcrypt_test.go
index b7162d8..8b589e3 100644
--- a/bcrypt/bcrypt_test.go
+++ b/bcrypt/bcrypt_test.go
@@ -241,3 +241,10 @@
t.Errorf("got=%q want=%q", got, want)
}
}
+
+func TestPasswordTooLong(t *testing.T) {
+ _, err := GenerateFromPassword(make([]byte, 73), 1)
+ if err != ErrPasswordTooLong {
+ t.Errorf("unexpected error: got %q, want %q", err, ErrPasswordTooLong)
+ }
+}
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.
Jason McNeil (Gerrit)
1 comment:
Patchset:
LGTM
To view, visit change 450415. To unsubscribe, or for help writing mail filters, visit settings.