[Rt-devel] Password storage format in RT3.6

[Arne Georg Gleditsch]

Hi,

I noticed today what I belive to be a deficiency in RT's password
handling that I feel should be looked into.  The following code is
from RT/User_Overlay.pm in 3.6.0:

  sub _GeneratePassword {
      my $self = shift;
      my $password = shift;
  
      my $md5 = Digest::MD5->new();
      $md5->add($password);
      return ($md5->hexdigest);
  }

  [..]

  sub IsPassword {
      my $self  = shift;
      my $value = shift;

      [..]

      #  if it's a historical password we say ok.
      if ($self->__Value('Password') eq crypt($value,
      $self->__Value('Password'))
          or $self->_GeneratePasswordBase64($value) eq
      $self->__Value('Password'))
      {
          # ...but upgrade the legacy password inplace.
          $self->SUPER::SetPassword( $self->_GeneratePassword($value) );
          return(1);
      }

Unless there's magic here that I can't spot, this means the current
password regime is storing passwords without salts.  As a consequence,
identical passwords will always have identical hash values.  Given
this, finding users with identical passwords is a no-brainer.
Dictionary attacks are also easer to mount, since the required number
of computations (and the amount of space required to store a
pre-compiled dictionary) is much lower when there's a 1-to-1
relationship between cleartext password and hash.

Granted, you need to be able _read_ the password hashes from the
database to effectively exploit this.  Still, in any case it seems
that the "upgrade the legacy password" code path actually reduces the
security of the system.  As things stand it could probably just as
well be removed.


							Arne.