You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by rob05c <gi...@git.apache.org> on 2017/05/23 17:13:59 UTC
[GitHub] incubator-trafficcontrol pull request #602: Change Traffic Ops password hash...
GitHub user rob05c opened a pull request:
https://github.com/apache/incubator-trafficcontrol/pull/602
Change Traffic Ops password hashing to scrypt
Note this is not a security vulnerability or mitigation in itself. In the event the database is compromised, it prevents an attacker from learning the users' passwords.
Which is the intention of hashing the passwords in the first place; but sha1 doesn't accomplish that. Nor does sha512, the problem isn't sha1's brokenness, it's that fast hashes aren't designed to solve this problem. The hash must be computationally slow ("slow" here means several milliseconds). Scrypt is a stretching hash, and solves the problem.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rob05c/incubator-trafficcontrol to-scryptpasses
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafficcontrol/pull/602.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #602
----
commit 670f86cc0a549a346a63d493b75d499e833b6f09
Author: Robert Butts <ro...@gmail.com>
Date: 2017-05-23T17:04:18Z
Change TO password hashing to scrypt
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #602: Change Traffic Ops password hashing to ...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/602
i'm seeing this when i try to login
[2017-06-07 10:59:36,931] [DEBUG] POST "/api/1.2/user/login".
[2017-06-07 10:59:37,072] [DEBUG] Routing to controller "API::User" and action "login".
[2017-06-07 10:59:37,077] [ERROR] Undefined subroutine &Utils::Helper::sha1_hex called at /code/src/github.com/apache/incubator-trafficcontrol/traffic_ops/app/lib/Utils/Helper.pm line 147.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #602: Change Traffic Ops password hash...
Posted by dangogh <gi...@git.apache.org>.
Github user dangogh commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/602#discussion_r120945531
--- Diff: traffic_ops/install/bin/_postinstall ---
@@ -28,7 +28,7 @@ use DBI;
use POSIX;
use File::Basename qw{dirname};
use File::Path qw{make_path};
-use Digest::SHA1 qw(sha1_hex);
+use Crypt::ScryptKDF qw(scrypt_hash);
--- End diff --
not sure how I missed that.. thanks..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #602: Change Traffic Ops password hashing to ...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/602
sorry, i merged another PR that created a conflict for you...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #602: Change Traffic Ops password hash...
Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/602#discussion_r120944998
--- Diff: traffic_ops/app/lib/Utils/Helper.pm ---
@@ -132,4 +134,18 @@ sub error {
);
}
+sub hash_pass {
+ my $pass = shift;
+ return scrypt_hash($pass, \64, 16384, 8, 1, 64);
+}
+
+sub verify_pass {
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #602: Change Traffic Ops password hash...
Posted by dangogh <gi...@git.apache.org>.
Github user dangogh commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/602#discussion_r120941858
--- Diff: traffic_ops/install/bin/_postinstall ---
@@ -28,7 +28,7 @@ use DBI;
use POSIX;
use File::Basename qw{dirname};
use File::Path qw{make_path};
-use Digest::SHA1 qw(sha1_hex);
+use Crypt::ScryptKDF qw(scrypt_hash);
--- End diff --
doesn't appear to be in there.. please add to that module to `traffic_ops/app/cpanfile`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #602: Change Traffic Ops password hash...
Posted by dangogh <gi...@git.apache.org>.
Github user dangogh commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/602#discussion_r120910425
--- Diff: traffic_ops/app/lib/Utils/Helper.pm ---
@@ -132,4 +134,18 @@ sub error {
);
}
+sub hash_pass {
+ my $pass = shift;
+ return scrypt_hash($pass, \64, 16384, 8, 1, 64);
+}
+
+sub verify_pass {
--- End diff --
how about a comment here describing what's going on? I assume it's for a gradual upgrade -- trying both crypt and sha1 for passwords that haven't been upgraded yet..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #602: Change Traffic Ops password hashing to ...
Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/602
Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #602: Change Traffic Ops password hashing to ...
Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/602
Ah, `Helper.pm` didn't already use SHA1. Not sure how it worked for me; should be fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol issue #602: Change Traffic Ops password hashing to ...
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/602
I'll try to look at this tomorrow. thanks @rob05c
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #602: Change Traffic Ops password hash...
Posted by rob05c <gi...@git.apache.org>.
Github user rob05c commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/602#discussion_r120942549
--- Diff: traffic_ops/install/bin/_postinstall ---
@@ -28,7 +28,7 @@ use DBI;
use POSIX;
use File::Basename qw{dirname};
use File::Path qw{make_path};
-use Digest::SHA1 qw(sha1_hex);
+use Crypt::ScryptKDF qw(scrypt_hash);
--- End diff --
It is https://github.com/apache/incubator-trafficcontrol/pull/602/files#diff-557cbcfb07ce166bc477ace4e9c9eaaaR273
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #602: Change Traffic Ops password hash...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafficcontrol/pull/602
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafficcontrol pull request #602: Change Traffic Ops password hash...
Posted by dangogh <gi...@git.apache.org>.
Github user dangogh commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/602#discussion_r120910642
--- Diff: traffic_ops/install/bin/_postinstall ---
@@ -28,7 +28,7 @@ use DBI;
use POSIX;
use File::Basename qw{dirname};
use File::Path qw{make_path};
-use Digest::SHA1 qw(sha1_hex);
+use Crypt::ScryptKDF qw(scrypt_hash);
--- End diff --
was this added to the `cpanfile`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---