From 7b68a543bf18ce6a386b951f1e2d1546f4a30572 Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Fri, 20 Oct 2017 00:42:33 +0200 Subject: [PATCH] Strengthen password in LDAP using SHA512 crypt algorithm Uses the crypt() function to do password encryption. This function handles several schemes such as: MD5, Blowfish, SHA1, SHA2. SHA-512 is used in Authelia for best security. The algorithm is fully described in https://www.akkadia.org/drepper/SHA-crypt.txt The 'crypt3' npm package has been added as a dependency to use the crypt() function. The package needs to be compiled in order to call the c function, that's why python, make and C++ compiler are installed temporarily in the Docker image. --- Dockerfile | 8 +++-- docker-compose.dev.yml | 1 - example/ldap/access.rules | 3 ++ example/ldap/base.ldif | 10 +++--- package.json | 2 +- server/src/index.ts | 1 - server/src/lib/IdentityCheckMiddleware.ts | 1 - server/src/lib/ServerVariablesInitializer.ts | 5 +-- server/src/lib/ldap/Client.ts | 32 +++++++++---------- server/src/lib/ldap/ClientFactory.ts | 13 +++----- server/src/lib/utils/HashGenerator.ts | 21 ++++++++++++ server/test/ServerConfiguration.test.ts | 4 +-- .../test/SessionConfigurationBuilder.test.ts | 6 ++-- server/test/ldap/Client.test.ts | 5 ++- server/test/ldap/PasswordUpdater.test.ts | 24 +++++++------- server/test/server/PrivatePages.ts | 3 +- server/test/server/PublicPages.ts | 3 +- server/test/utils/HashGenerator.test.ts | 18 +++++++++++ server/types/Dependencies.ts | 3 -- 19 files changed, 95 insertions(+), 68 deletions(-) create mode 100644 server/src/lib/utils/HashGenerator.ts create mode 100644 server/test/utils/HashGenerator.test.ts diff --git a/Dockerfile b/Dockerfile index 7700fd8d..d0dbb23e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,9 +1,13 @@ -FROM node:7-alpine +FROM node:8.7.0-alpine WORKDIR /usr/src COPY package.json /usr/src/package.json -RUN npm install --production + +RUN apk --update add --no-cache --virtual \ + .build-deps make g++ python && \ + npm install --production && \ + apk del .build-deps COPY dist/server /usr/src/server COPY dist/shared /usr/src/shared diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 083b5763..1290afd1 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -5,7 +5,6 @@ services: - ./test:/usr/src/test - ./dist/server:/usr/src/server - ./dist/shared:/usr/src/shared - - ./node_modules:/usr/src/node_modules - ./config.template.yml:/etc/authelia/config.yml:ro networks: - example-network diff --git a/example/ldap/access.rules b/example/ldap/access.rules index 125b4ce9..8762639e 100644 --- a/example/ldap/access.rules +++ b/example/ldap/access.rules @@ -2,3 +2,6 @@ olcAccess: {0}to attrs=userPassword,shadowLastChange by self write by anonymou s auth by * none # olcAccess: {1}to dn.base="" by * read # olcAccess: {2}to * by * read + +olcPasswordHash: {CRYPT} +olcPasswordCryptSaltFormat: $6$rounds=50000$%.16s diff --git a/example/ldap/base.ldif b/example/ldap/base.ldif index 50f65ee9..3136eb89 100644 --- a/example/ldap/base.ldif +++ b/example/ldap/base.ldif @@ -27,7 +27,7 @@ objectclass: inetOrgPerson objectclass: top mail: john.doe@authelia.com sn: John Doe -userpassword: {SHA}W6ph5Mm5Pz8GgiULbPgzG37mj9g= +userpassword: {CRYPT}$6$rounds=500000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/ dn: cn=harry,ou=users,dc=example,dc=com cn: harry @@ -35,7 +35,7 @@ objectclass: inetOrgPerson objectclass: top mail: harry.potter@authelia.com sn: Harry Potter -userpassword: {SHA}W6ph5Mm5Pz8GgiULbPgzG37mj9g= +userpassword: {CRYPT}$6$rounds=500000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/ dn: cn=bob,ou=users,dc=example,dc=com cn: bob @@ -43,7 +43,7 @@ objectclass: inetOrgPerson objectclass: top mail: bob.dylan@authelia.com sn: Bob Dylan -userpassword: {SHA}W6ph5Mm5Pz8GgiULbPgzG37mj9g= +userpassword: {CRYPT}$6$rounds=500000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/ dn: cn=james,ou=users,dc=example,dc=com cn: james @@ -51,7 +51,7 @@ objectclass: inetOrgPerson objectclass: top mail: james.dean@authelia.com sn: James Dean -userpassword: {SHA}W6ph5Mm5Pz8GgiULbPgzG37mj9g= +userpassword: {CRYPT}$6$rounds=500000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/ dn: cn=blackhat,ou=users,dc=example,dc=com cn: blackhat @@ -59,4 +59,4 @@ objectclass: inetOrgPerson objectclass: top mail: billy.blackhat@authelia.com sn: Billy BlackHat -userpassword: {SHA}W6ph5Mm5Pz8GgiULbPgzG37mj9g= +userpassword: {CRYPT}$6$rounds=500000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/ diff --git a/package.json b/package.json index cdcd6fd7..36fa2d78 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "bluebird": "3.5.0", "body-parser": "^1.15.2", "connect-redis": "^3.3.0", - "dovehash": "0.0.5", + "crypt3": "^1.0.0", "ejs": "^2.5.5", "express": "^4.14.0", "express-request-id": "^1.4.0", diff --git a/server/src/index.ts b/server/src/index.ts index 429cc857..286da1fc 100755 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -15,7 +15,6 @@ const yamlContent = YAML.load(configurationFilepath); const deps: GlobalDependencies = { u2f: require("u2f"), - dovehash: require("dovehash"), ldapjs: require("ldapjs"), session: require("express-session"), winston: require("winston"), diff --git a/server/src/lib/IdentityCheckMiddleware.ts b/server/src/lib/IdentityCheckMiddleware.ts index 17f39b33..8f017524 100644 --- a/server/src/lib/IdentityCheckMiddleware.ts +++ b/server/src/lib/IdentityCheckMiddleware.ts @@ -7,7 +7,6 @@ import Exceptions = require("./Exceptions"); import fs = require("fs"); import ejs = require("ejs"); import { IUserDataStore } from "./storage/IUserDataStore"; -import { Winston } from "../../types/Dependencies"; import express = require("express"); import ErrorReplies = require("./ErrorReplies"); import AuthenticationSessionHandler = require("./AuthenticationSession"); diff --git a/server/src/lib/ServerVariablesInitializer.ts b/server/src/lib/ServerVariablesInitializer.ts index 37c76330..7f60adcc 100644 --- a/server/src/lib/ServerVariablesInitializer.ts +++ b/server/src/lib/ServerVariablesInitializer.ts @@ -69,7 +69,8 @@ export class ServerVariablesInitializer { const mailSenderBuilder = new MailSenderBuilder(Nodemailer); const notifier = NotifierFactory.build(config.notifier, mailSenderBuilder); const ldapClientFactory = new LdapClientFactory(config.ldap, deps.ldapjs); - const clientFactory = new ClientFactory(config.ldap, ldapClientFactory, deps.dovehash, deps.winston); + const clientFactory = new ClientFactory(config.ldap, ldapClientFactory, + deps.winston); const ldapAuthenticator = new Authenticator(config.ldap, clientFactory); const ldapPasswordUpdater = new PasswordUpdater(config.ldap, clientFactory); @@ -97,5 +98,5 @@ export class ServerVariablesInitializer { }; return BluebirdPromise.resolve(variables); }); - } + } } diff --git a/server/src/lib/ldap/Client.ts b/server/src/lib/ldap/Client.ts index e71a82d0..5744a49c 100644 --- a/server/src/lib/ldap/Client.ts +++ b/server/src/lib/ldap/Client.ts @@ -1,9 +1,5 @@ - -import util = require("util"); import BluebirdPromise = require("bluebird"); import exceptions = require("../Exceptions"); -import Dovehash = require("dovehash"); - import { EventEmitter } from "events"; import { IClient, GroupsAndEmails } from "./IClient"; import { ILdapClient } from "./ILdapClient"; @@ -11,20 +7,18 @@ import { ILdapClientFactory } from "./ILdapClientFactory"; import { LdapConfiguration } from "../configuration/Configuration"; import { Winston } from "../../../types/Dependencies"; import Util = require("util"); - +import { HashGenerator } from "../utils/HashGenerator"; export class Client implements IClient { private userDN: string; private password: string; private ldapClient: ILdapClient; private logger: Winston; - private dovehash: typeof Dovehash; private options: LdapConfiguration; constructor(userDN: string, password: string, options: LdapConfiguration, - ldapClientFactory: ILdapClientFactory, dovehash: typeof Dovehash, logger: Winston) { + ldapClientFactory: ILdapClientFactory, logger: Winston) { this.options = options; - this.dovehash = dovehash; this.logger = logger; this.userDN = userDN; this.password = password; @@ -128,18 +122,22 @@ export class Client implements IClient { modifyPassword(username: string, newPassword: string): BluebirdPromise { const that = this; - const encodedPassword = this.dovehash.encode("SSHA", newPassword); - const change = { - operation: "replace", - modification: { - userPassword: encodedPassword - } - }; - this.logger.debug("LDAP: update password of user '%s'", username); return this.searchUserDn(username) .then(function (userDN: string) { - that.ldapClient.modifyAsync(userDN, change); + return BluebirdPromise.join( + HashGenerator.ssha512(newPassword), + BluebirdPromise.resolve(userDN)); + }) + .then(function (res: string[]) { + const change = { + operation: "replace", + modification: { + userPassword: res[0] + } + }; + that.logger.debug("Password new='%s'", change.modification.userPassword); + return that.ldapClient.modifyAsync(res[1], change); }) .then(function () { return that.ldapClient.unbindAsync(); diff --git a/server/src/lib/ldap/ClientFactory.ts b/server/src/lib/ldap/ClientFactory.ts index 2f2b9217..bbef51da 100644 --- a/server/src/lib/ldap/ClientFactory.ts +++ b/server/src/lib/ldap/ClientFactory.ts @@ -4,27 +4,24 @@ import { Client } from "./Client"; import { SanitizedClient } from "./SanitizedClient"; import { ILdapClientFactory } from "./ILdapClientFactory"; import { LdapConfiguration } from "../configuration/Configuration"; - import Ldapjs = require("ldapjs"); -import Dovehash = require("dovehash"); import Winston = require("winston"); export class ClientFactory implements IClientFactory { private config: LdapConfiguration; private ldapClientFactory: ILdapClientFactory; - private dovehash: typeof Dovehash; private logger: typeof Winston; - constructor(ldapConfiguration: LdapConfiguration, ldapClientFactory: ILdapClientFactory, - dovehash: typeof Dovehash, logger: typeof Winston) { + constructor(ldapConfiguration: LdapConfiguration, + ldapClientFactory: ILdapClientFactory, + logger: typeof Winston) { this.config = ldapConfiguration; this.ldapClientFactory = ldapClientFactory; - this.dovehash = dovehash; this.logger = logger; } create(userDN: string, password: string): IClient { - return new SanitizedClient(new Client(userDN, password, this.config, this.ldapClientFactory, - this.dovehash, this.logger)); + return new SanitizedClient(new Client(userDN, password, + this.config, this.ldapClientFactory, this.logger)); } } \ No newline at end of file diff --git a/server/src/lib/utils/HashGenerator.ts b/server/src/lib/utils/HashGenerator.ts new file mode 100644 index 00000000..410b522f --- /dev/null +++ b/server/src/lib/utils/HashGenerator.ts @@ -0,0 +1,21 @@ +import BluebirdPromise = require("bluebird"); +import RandomString = require("randomstring"); +import Util = require("util"); +const crypt = require("crypt3"); + +export class HashGenerator { + static ssha512(password: string, salt?: string): BluebirdPromise { + const rounds = 500000; + const saltSize = 16; + // $6 means SHA512 + const _salt = Util.format("$6$rounds=%d$%s", rounds, + (salt) ? salt : RandomString.generate(16)); + + const cryptAsync = BluebirdPromise.promisify(crypt); + + return cryptAsync(password, _salt) + .then(function (hash: string) { + return BluebirdPromise.resolve(Util.format("{CRYPT}%s", hash)); + }); + } +} \ No newline at end of file diff --git a/server/test/ServerConfiguration.test.ts b/server/test/ServerConfiguration.test.ts index bc7f705b..2b8ac0a8 100644 --- a/server/test/ServerConfiguration.test.ts +++ b/server/test/ServerConfiguration.test.ts @@ -7,7 +7,6 @@ import winston = require("winston"); import speakeasy = require("speakeasy"); import u2f = require("u2f"); import session = require("express-session"); - import { AppConfiguration, UserConfiguration } from "../src/lib/configuration/Configuration"; import { GlobalDependencies } from "../types/Dependencies"; import Server from "../src/lib/Server"; @@ -34,8 +33,7 @@ describe("test server configuration", function () { }) }, session: sessionMock as any, - ConnectRedis: Sinon.spy(), - dovehash: Sinon.spy() as any + ConnectRedis: Sinon.spy() }; }); diff --git a/server/test/SessionConfigurationBuilder.test.ts b/server/test/SessionConfigurationBuilder.test.ts index 6273382f..4318747c 100644 --- a/server/test/SessionConfigurationBuilder.test.ts +++ b/server/test/SessionConfigurationBuilder.test.ts @@ -62,8 +62,7 @@ describe("test session configuration builder", function () { session: Sinon.spy() as any, speakeasy: Sinon.spy() as any, u2f: Sinon.spy() as any, - winston: Sinon.spy() as any, - dovehash: Sinon.spy() as any + winston: Sinon.spy() as any }; const options = SessionConfigurationBuilder.build(configuration, deps); @@ -144,8 +143,7 @@ describe("test session configuration builder", function () { session: Sinon.spy() as any, speakeasy: Sinon.spy() as any, u2f: Sinon.spy() as any, - winston: Sinon.spy() as any, - dovehash: Sinon.spy() as any + winston: Sinon.spy() as any }; const options = SessionConfigurationBuilder.build(configuration, deps); diff --git a/server/test/ldap/Client.test.ts b/server/test/ldap/Client.test.ts index d2f9c4b1..bdf6b6cb 100644 --- a/server/test/ldap/Client.test.ts +++ b/server/test/ldap/Client.test.ts @@ -7,7 +7,6 @@ import { LdapClientStub } from "../mocks/ldap/LdapClientStub"; import Sinon = require("sinon"); import BluebirdPromise = require("bluebird"); import Assert = require("assert"); -import Dovehash = require("dovehash"); import Winston = require("winston"); describe("test authelia ldap client", function () { @@ -34,7 +33,7 @@ describe("test authelia ldap client", function () { ldapClient.searchAsyncStub.returns(BluebirdPromise.resolve([{ cn: "group1" }])); - const client = new Client(ADMIN_USER_DN, ADMIN_PASSWORD, options, factory, Dovehash, Winston); + const client = new Client(ADMIN_USER_DN, ADMIN_PASSWORD, options, factory, Winston); return client.searchGroups("user1") .then(function () { @@ -80,7 +79,7 @@ describe("test authelia ldap client", function () { cn: "group1" }])); - const client = new Client(ADMIN_USER_DN, ADMIN_PASSWORD, options, factory, Dovehash, Winston); + const client = new Client(ADMIN_USER_DN, ADMIN_PASSWORD, options, factory, Winston); return client.searchGroups("user1") .then(function (groups: string[]) { diff --git a/server/test/ldap/PasswordUpdater.test.ts b/server/test/ldap/PasswordUpdater.test.ts index 634b22f1..aa875284 100644 --- a/server/test/ldap/PasswordUpdater.test.ts +++ b/server/test/ldap/PasswordUpdater.test.ts @@ -1,13 +1,11 @@ - -import { PasswordUpdater } from "../../src/lib/ldap/PasswordUpdater"; -import { LdapConfiguration } from "../../src/lib/configuration/Configuration"; - import Sinon = require("sinon"); import BluebirdPromise = require("bluebird"); import Assert = require("assert"); - +import { PasswordUpdater } from "../../src/lib/ldap/PasswordUpdater"; +import { LdapConfiguration } from "../../src/lib/configuration/Configuration"; import { ClientFactoryStub } from "../mocks/ldap/ClientFactoryStub"; import { ClientStub } from "../mocks/ldap/ClientStub"; +import { HashGenerator } from "../../src/lib/utils/HashGenerator"; describe("test password update", function () { const USERNAME = "username"; @@ -18,10 +16,9 @@ describe("test password update", function () { let clientFactoryStub: ClientFactoryStub; let adminClientStub: ClientStub; - let passwordUpdater: PasswordUpdater; let ldapConfig: LdapConfiguration; - let dovehash: any; + let ssha512HashGenerator: Sinon.SinonStub; beforeEach(function () { clientFactoryStub = new ClientFactoryStub(); @@ -39,19 +36,20 @@ describe("test password update", function () { users_filter: "cn={0}" }; - dovehash = { - encode: Sinon.stub() - }; - + ssha512HashGenerator = Sinon.stub(HashGenerator, "ssha512"); passwordUpdater = new PasswordUpdater(ldapConfig, clientFactoryStub); }); + afterEach(function () { + ssha512HashGenerator.restore(); + }); + describe("success", function () { it("should update the password successfully", function () { clientFactoryStub.createStub.withArgs(ADMIN_USER_DN, ADMIN_PASSWORD) .returns(adminClientStub); - dovehash.encode.returns("{SSHA}AQmxaKfobGY9HSQa6aDYkAWOgPGNhGYn"); + ssha512HashGenerator.returns("{CRYPT}$6$abcdefghijklm$AQmxaKfobGY9HSQa6aDYkAWOgPGNhGYn"); adminClientStub.modifyPasswordStub.withArgs(USERNAME, NEW_PASSWORD).returns(BluebirdPromise.resolve()); adminClientStub.openStub.returns(BluebirdPromise.resolve()); adminClientStub.closeStub.returns(BluebirdPromise.resolve()); @@ -65,7 +63,7 @@ describe("test password update", function () { clientFactoryStub.createStub.withArgs(ADMIN_USER_DN, ADMIN_PASSWORD) .returns(adminClientStub); - dovehash.encode.returns("{SSHA}AQmxaKfobGY9HSQa6aDYkAWOgPGNhGYn"); + ssha512HashGenerator.returns("{CRYPT}$6$abcdefghijklm$AQmxaKfobGY9HSQa6aDYkAWOgPGNhGYn"); adminClientStub.modifyPasswordStub.withArgs(USERNAME, NEW_PASSWORD) .returns(BluebirdPromise.reject(new Error("Error while updating password"))); adminClientStub.openStub.returns(BluebirdPromise.resolve()); diff --git a/server/test/server/PrivatePages.ts b/server/test/server/PrivatePages.ts index a4421160..18af2147 100644 --- a/server/test/server/PrivatePages.ts +++ b/server/test/server/PrivatePages.ts @@ -116,8 +116,7 @@ describe("Private pages of the server must not be accessible without session", f session: ExpressSession, winston: Winston, speakeasy: speakeasy, - ConnectRedis: Sinon.spy(), - dovehash: Sinon.spy() as any + ConnectRedis: Sinon.spy() }; server = new Server(deps); diff --git a/server/test/server/PublicPages.ts b/server/test/server/PublicPages.ts index 12e4cc32..cefd414a 100644 --- a/server/test/server/PublicPages.ts +++ b/server/test/server/PublicPages.ts @@ -116,8 +116,7 @@ describe("Public pages of the server must be accessible without session", functi session: ExpressSession, winston: Winston, speakeasy: speakeasy, - ConnectRedis: Sinon.spy(), - dovehash: Sinon.spy() as any + ConnectRedis: Sinon.spy() }; server = new Server(deps); diff --git a/server/test/utils/HashGenerator.test.ts b/server/test/utils/HashGenerator.test.ts new file mode 100644 index 00000000..e1637c5f --- /dev/null +++ b/server/test/utils/HashGenerator.test.ts @@ -0,0 +1,18 @@ +import Assert = require("assert"); +import { HashGenerator } from "../../src/lib/utils/HashGenerator"; + +describe("test HashGenerator", function () { + it("should compute correct ssha512 (password)", function () { + return HashGenerator.ssha512("password", "jgiCMRyGXzoqpxS3") + .then(function (hash: string) { + Assert.equal(hash, "{CRYPT}$6$rounds=500000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/"); + }); + }); + + it("should compute correct ssha512 (test)", function () { + return HashGenerator.ssha512("test", "abcdefghijklmnop") + .then(function (hash: string) { + Assert.equal(hash, "{CRYPT}$6$rounds=500000$abcdefghijklmnop$sTlNGf0VO/HTQIOXemmaBbV28HUch/qhWOA1/4dsDj6CDQYhUgXbYSPL6gccAsWMr2zD5fFWwhKmPdG.yxphs."); + }); + }); +}); \ No newline at end of file diff --git a/server/types/Dependencies.ts b/server/types/Dependencies.ts index 0eda3614..47c5454a 100644 --- a/server/types/Dependencies.ts +++ b/server/types/Dependencies.ts @@ -6,9 +6,7 @@ import nedb = require("nedb"); import ldapjs = require("ldapjs"); import u2f = require("u2f"); import RedisSession = require("connect-redis"); -import dovehash = require("dovehash"); -export type Dovehash = typeof dovehash; export type Speakeasy = typeof speakeasy; export type Winston = typeof winston; export type Session = typeof session; @@ -19,7 +17,6 @@ export type ConnectRedis = typeof RedisSession; export interface GlobalDependencies { u2f: U2f; - dovehash: Dovehash; ldapjs: Ldapjs; session: Session; ConnectRedis: ConnectRedis;