fix(verdaccio-htpasswd): generate non-constant legacy 2 byte salt (#357)

* fix(verdaccio-htpasswd): generate non-constant legacy 2 byte salt

The crypt implementation in use does only support the legacy crypt
hashing with a 2 byte salt. Supplying it with a prefixed salt will
simply make the given prefix a constant salt value ('$6' with the
defaults here).

Note that the crypt hashing is weak either way, this just further
weakens it by essentially removing the extra complexity a random
salt provides for the individual entries.

This change is backwards compatible. Existing entries will retain
their constant '$6' salt, new and updated ones will get variable
salts.

* fix(verdaccio-htpasswd): supply old password to crypt3 on change

To generate the proper comparison string, crypt3 needs the salt from the
original entry. This previously worked only due to the constant salt.
The verifyPassword function already does this correctly.

Add a test that verifies that the password change works and a
different result is returned when a new salt is generated.

plugins/htpasswd/src/crypt3.ts

@@ -16,8 +16,12 @@ import crypt from 'unix-crypt-td-js';
  * distros), sha256 or sha512. Default is sha512.
  * @returns {string} Generated salt string
  */
-export function createSalt(type = 'sha512'): string {
+export function createSalt(type = 'crypt'): string {
   switch (type) {
+    case 'crypt':
+      // Legacy crypt salt with no prefix (only the first 2 bytes will be used).
+      return crypto.randomBytes(2).toString('base64');
+
     case 'md5':
       return '$1$' + crypto.randomBytes(10).toString('base64');
 

plugins/htpasswd/src/utils.ts

@@ -159,21 +159,21 @@ export function getCryptoPassword(password: string): string {
  * @returns {string}
  */
 export function changePasswordToHTPasswd(body: string, user: string, passwd: string, newPasswd: string): string {
-  let _passwd;
-  let _newPasswd;
-  if (crypt3) {
-    _passwd = crypt3(passwd);
-    _newPasswd = crypt3(newPasswd);
-  } else {
-    _passwd = getCryptoPassword(passwd);
-    _newPasswd = getCryptoPassword(newPasswd);
-  }
-
   let lines = body.split('\n');
   lines = lines.map(line => {
     const [username, password] = line.split(':', 3);
 
     if (username === user) {
+      let _passwd;
+      let _newPasswd;
+      if (crypt3) {
+        _passwd = crypt3(passwd, password);
+        _newPasswd = crypt3(newPasswd);
+      } else {
+        _passwd = getCryptoPassword(passwd);
+        _newPasswd = getCryptoPassword(newPasswd);
+      }
+
       if (password == _passwd) {
         // replace old password hash with new password hash
         line = line.replace(_passwd, _newPasswd);

plugins/htpasswd/tests/crypt3.test.ts

@@ -12,6 +12,7 @@ jest.mock('crypto', () => {
 
 describe('createSalt', () => {
   test('should match with the correct salt type', () => {
+    expect(createSalt('crypt')).toEqual('/UEGzD0RxSNDZA==');
     expect(createSalt('md5')).toEqual('$1$/UEGzD0RxSNDZA==');
     expect(createSalt('blowfish')).toEqual('$2a$/UEGzD0RxSNDZA==');
     expect(createSalt('sha256')).toEqual('$5$/UEGzD0RxSNDZA==');
@@ -23,4 +24,8 @@ describe('createSalt', () => {
       createSalt('bad');
     }).toThrow(/Unknown salt type at crypt3.createSalt: bad/);
   });
+
+  test('should generate legacy crypt salt by default', () => {
+    expect(createSalt()).toEqual(createSalt('crypt'));
+  });
 });

plugins/htpasswd/tests/htpasswd.test.ts

@@ -1,3 +1,4 @@
+import crypto from 'crypto';
 // @ts-ignore
 import fs from 'fs';
 
@@ -22,6 +23,12 @@ describe('HTPasswd', () => {
   beforeEach(() => {
     wrapper = new HTPasswd(config, (stuff as unknown) as VerdaccioConfigApp);
     jest.resetModules();
+
+    crypto.randomBytes = jest.fn(() => {
+      return {
+        toString: (): string => '$6',
+      };
+    });
   });
 
   describe('constructor()', () => {

plugins/htpasswd/tests/utils.test.ts

@@ -1,3 +1,5 @@
+import crypto from 'crypto';
+
 import {
   verifyPassword,
   lockAndRead,
@@ -89,6 +91,12 @@ describe('addUserToHTPasswd - crypt3', () => {
         toJSON: (): string => '2018-01-14T11:17:40.712Z',
       };
     });
+
+    crypto.randomBytes = jest.fn(() => {
+      return {
+        toString: (): string => '$6',
+      };
+    });
   });
 
   it('should add new htpasswd to the end', () => {
@@ -222,6 +230,20 @@ describe('changePasswordToHTPasswd', () => {
     expect(changePasswordToHTPasswd(body, 'root', 'demo123', 'newPassword')).toMatchSnapshot();
   });
 
+  test('should generate a different result on salt change', () => {
+    crypto.randomBytes = jest.fn(() => {
+      return {
+        toString: (): string => 'AB',
+      };
+    });
+
+    const body = 'root:$6qLTHoPfGLy2:autocreated 2018-08-20T13:38:12.164Z';
+
+    expect(changePasswordToHTPasswd(body, 'root', 'demo123', 'demo123')).toEqual(
+      'root:ABfaAAjDKIgfw:autocreated 2018-08-20T13:38:12.164Z'
+    );
+  });
+
   test('should change the password when crypt3 is not available', () => {
     jest.resetModules();
     jest.doMock('../src/crypt3.ts', () => false);