0

Ok so, I've spent days looking at codes from here and other places and I just keep piecing parts together. This script works for me, and I'm still vetting security vulnerabilities, however I have tried to create it with due diligance. I'm not a perl genius and a bit rusty with regexp.

The purpose of this script will be used as an embedded key for chatroom purposes, I will most likely use the original crypt to branch off a second key, hence the extra step and obfuscation in case someone starts pulling apart the layers of generated printed pages. I don't want to have the same key used for login stored in database as the one embedded but for obvious reasons they need to same root pass. I may have overshot cleaning characters in too many places but I figured why not, in case I make this a separate file.

use Digest::SHA qw(hmac_sha256_hex);

sub passCrypt {
    chomp(my $userin=$_[0]);            # first passed var is username
    chomp(my $passin=$_[1]);            # second passed var is plaintext password
    $userin = substr $userin, 0, 24;    # protect from DoS attacks by limiting length of input
    $passin = substr $passin, 0, 64;
    $passin =~ tr/\\\?\/\<\>/XPZQJ/;    # clean password replacing \ ? / < > with safe chars
    $passin =~ s/0x[0-9a-fA-F]//g;      # clean password from hex 0x0 - 0xF
    $userin =~ s/[^\w]//g;              # clean username from any non-word characters (a-z A-Z 0-9 _)
    $userin =~ s/[_]//g;                # clean username from the _ (for salt)
    $userin = lc($userin);              # all lowercase username
    my $salt = substr($userin, 1, 1) . substr($userin, -2, 1);  # make salt from first and last of username
    my $clnpass = crypt($passin,$salt);  # use basic crypt to prevent plain text password being utilized beyond this point
    $clnpass = substr $clnpass, 2;  # strip salt from beginning of pass
    $clnpass =~ tr/\/\\\!\@\#\$\%\^\&\*\(\)\[\]\{\}\|\?\=\+\-\_\<\>\:\;\"\'\`\,\./1234567890123456789012345678901/;  # make pass filename safe (replacement to keep consistent length)
    $clnpass =~ tr/abcdefghijklmnopqrstuvwxyz1234567890ABCDEFGHIJKLMNOPQRSTUVWXYZ/WXYZwxcd123efghFGHIJij4ABlmnopqNOPQRrstuCDEKL567MSkyzTUVa890/;  # added obfuscation
    $clnpass = reverse($clnpass);  # more obfuscation
    my $cryptpass = hmac_sha256_hex($clnpass, chr(0x0b) x 32);  # final crypt with sha256
    return $cryptpass; 
}

I'm putting this up here in case someone else doing searches runs into similar questions as I have, and hoping for lots of suggestions of improvement. Is there a better way to provide decent password hashing in a perl cgi script? I first looked into Crypt::Eksblowfish::Bcrypt but my host doesn't have that package. Thanks in advance.

Edit: It was clarified to me that all I'm doing is hashing so I replaced said text.

PeaceIsBliss
  • 3
  • 2
  • I'm voting to close this question as off-topic because there is no actual question here. Note that stackoverflow.com is a strict Q+A site and not a generic code sharing site. Apart from that it is not encryption at all but hashing. – Steffen Ullrich Feb 24 '20 at 21:52
  • Sorry guess I used the topic as my question. – PeaceIsBliss Feb 24 '20 at 21:54
  • The use case is not clear for me. It kind of looks like attempting to write your own key derivation function. No idea what this key is used for at the end. *"This script works for me,..."* - that's usually the point that it somehow works. But this is no indicator of any security. A fixed string would probably work too. – Steffen Ullrich Feb 24 '20 at 21:56
  • Login password verification compared to password stored on server database – PeaceIsBliss Feb 24 '20 at 21:57
  • In this case please study [How to securely hash passwords?](https://security.stackexchange.com/questions/211/how-to-securely-hash-passwords) and don't invent your own insecure method. – Steffen Ullrich Feb 24 '20 at 21:58
  • so passCrypt($username,$password) returns with encrypted password to compare with database file entry whenever I need it to. – PeaceIsBliss Feb 24 '20 at 22:00
  • Like I already said, just because it works does not mean it is secure. Slowness is an important aspect of a password hash - which your implementation is missing. Just used established methods like pbkdf2 or bcrypt - there are are already Perl modules for this. And again: hashing and encryption are different concepts and there is no encryption going on in your code. – Steffen Ullrich Feb 24 '20 at 22:02

2 Answers2

2

The security in your code basically relies on crypt and hmac_sha256. The rest is obfuscation. Given that the script is likely located at the same system as the password hashes (no encryption here, just hashing) it is unlikely that obfuscation will actually help against an attacker - whoever gets access to the password hashes can likely also get access to your code.

This leaves crypt and hmac_sha256 which are both fast hash functions, so essentially you create a fast function for password hashing. But using a fast hash means that an attacker could quickly brute force any existing hashes too. That's why it is an important design criteria of password hashes to be slow enough.

For more details on how to properly hash passwords see How to securely hash passwords?. Please don't invent your own but use existing methods instead. In security it is best to rely on proven methods instead of inventing your own methods, which might look secure for you but not for others.

Steffen Ullrich
  • 114,247
  • 10
  • 131
  • 172
  • So I read that whole page, and others related. Following up with a last question about it. I wanted the hash to be fast because It will recalculate every time an event occurs that the user receives or sends, for verification and to prevent unauthorized data pushing (the chat scripts I am writing integrate with javascript). What I am getting from this is, someone needs to hack the host server security for folder and file permissions to get the hash tables and my uncompiled cgi files. So the risk at that point is leaked email accounts tied to hashes. Should I worry about my server? – PeaceIsBliss Feb 25 '20 at 00:19
  • @PeaceIsBliss: The risk of compromise is leaked email and hashes where the hash is weak enough to allow brute force - which then leads to leaked email und passwords. Given that password reuse is common an attacker will try the same credentials on other sites. *"... I wanted the hash to be fast because It will recalculate every time an event occurs..."* - the established approach is not to send user and password each time but use these only to establish a temporary session with a session identifier not tied to the actual password and which can be checked fast. – Steffen Ullrich Feb 25 '20 at 05:19
  • This is why I came here, to establish good practice (since the old scripts i learned from are terrible), Thank you. The session id was something I hadn't thought of, and that's precisely what I will do. Appreciate your help. Consider this answered now. – PeaceIsBliss Feb 25 '20 at 13:03
0

So taking all of Steffen's advice, this is what I ended up with, will change a few details from what I'm actually using because this is likely similar to what I will use and frankly security seems silly if I keep posting my scripts.

use Digest::SHA qw(hmac_sha512_hex hmac_sha256_base64);
use Crypt::PBKDF2;

my $hash = &MakeHash($username,$password); #test
print &MakeSessionID($hash); #test

sub MakeHash {
  chomp(my $userin=$_[0]);  #i do this as a habbit when I work with files, I know this info should not be coming from a file, I will clean it up later
  chomp(my $passin=$_[1]);
  $userin = substr $userin, 0, 24;  #keeping this to prevent accidents while I keep working
  $passin = substr $passin, 0, 64;
  my $hashkey = hmac_sha256_base64($userin,$userin);
  my $pbkdf2 = Crypt::PBKDF2->new(hash_class => 'HMACSHA1',iterations => 200000,output_len => 24); #these are not the values im actually using but close enough
  my $result = $pbkdf2->PBKDF2_base64($hashkey,$passin);
  return $result; }

sub MakeSessionID {
  chomp(my $passin=$_[0]);
  my $salt = time*2;
  my $result = hmac_sha512_hex($passin, $salt);
  return $result; }

clearly missing my functions that utilize this but yeah

PeaceIsBliss
  • 3
  • 2