Merge pull request #533 from Mashape/feature/basic-auth-sha1

[feat/basic-auth] password encryption

Makefile

@@ -48,7 +48,7 @@ drop:
 	@bin/kong db -c $(DEVELOPMENT_CONF) drop
 
 lint:
-	@find kong spec -name '*.lua' ! -name 'invalid-module.lua' | xargs luacheck -q
+	@find kong spec -name '*.lua' -not -name 'invalid-module.lua' -not -path 'kong/vendor/*' | xargs luacheck -q
 
 test:
 	@busted -v spec/unit

kong-0.5.0-1.rockspec

@@ -97,6 +97,7 @@ build = {
     ["kong.plugins.base_plugin"] = "kong/plugins/base_plugin.lua",
 
     ["kong.plugins.basic-auth.migrations.cassandra"] = "kong/plugins/basic-auth/migrations/cassandra.lua",
+    ["kong.plugins.basic-auth.crypto"] = "kong/plugins/basic-auth/crypto.lua",
     ["kong.plugins.basic-auth.handler"] = "kong/plugins/basic-auth/handler.lua",
     ["kong.plugins.basic-auth.access"] = "kong/plugins/basic-auth/access.lua",
     ["kong.plugins.basic-auth.schema"] = "kong/plugins/basic-auth/schema.lua",

kong/plugins/basic-auth/access.lua

@@ -2,6 +2,7 @@ local cache = require "kong.tools.database_cache"
 local stringy = require "stringy"
 local responses = require "kong.tools.responses"
 local constants = require "kong.constants"
+local crypto = require "kong.plugins.basic-auth.crypto"
 
 local AUTHORIZATION = "authorization"
 local PROXY_AUTHORIZATION = "proxy-authorization"
@@ -50,26 +51,23 @@ local function retrieve_credentials(request, header_name, conf)
   return username, password
 end
 
--- Fast lookup for credential validation depending on the type of the authentication
---
--- All methods must respect:
---
--- @param {table} credential The retrieved credential from the username passed in the request
--- @param {string} username
--- @param {string} password
--- @return {boolean} Success of authentication
-local function validate_credentials(credential, username, password)
-  if credential then
-    -- TODO: No encryption yet
-    return credential.password == password
+--- Validate a credential in the Authorization header against one fetched from the database.
+-- @param credential The retrieved credential from the username passed in the request
+-- @param given_password The password as given in the Authorization header
+-- @return Success of authentication
+local function validate_credentials(credential, given_password)
+  local digest, err = crypto.encrypt({consumer_id = credential.consumer_id, password = given_password})
+  if err then
+    ngx.log(ngx.ERR, "[basic-auth]  "..err)
   end
+  return credential.password == digest
 end
 
-local function load_credential(username)
+local function load_credential_from_db(username)
   local credential
   if username then
     credential = cache.get_or_set(cache.basicauth_credential_key(username), function()
-      local credentials, err = dao.basicauth_credentials:find_by_keys { username = username }
+      local credentials, err = dao.basicauth_credentials:find_by_keys {username = username}
       local result
       if err then
         return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
@@ -91,18 +89,18 @@ function _M.execute(conf)
   end
 
   local credential
-  local username, password = retrieve_credentials(ngx.req, PROXY_AUTHORIZATION, conf)
-  if username then
-    credential = load_credential(username)
+  local given_username, given_password = retrieve_credentials(ngx.req, PROXY_AUTHORIZATION, conf)
+  if given_username then
+    credential = load_credential_from_db(given_username)
   end
 
   -- Try with the authorization header
   if not credential then
-    username, password = retrieve_credentials(ngx.req, AUTHORIZATION, conf)
-    credential = load_credential(username)
+    given_username, given_password = retrieve_credentials(ngx.req, AUTHORIZATION, conf)
+    credential = load_credential_from_db(given_username)
   end
 
-  if not validate_credentials(credential, username, password) then
+  if not credential or not validate_credentials(credential, given_password) then
     ngx.ctx.stop_phases = true -- interrupt other phases of this request
     return responses.send_HTTP_FORBIDDEN("Invalid authentication credentials")
   end

kong/plugins/basic-auth/crypto.lua

@@ -0,0 +1,22 @@
+---
+-- Module to encrypt the basic-auth credentials password field
+
+local crypto = require "crypto"
+local format = string.format
+
+--- Salt the password
+-- Password is salted with the credential's consumer_id (long enough, unique)
+-- @param credential The basic auth credential table
+local function salt_password(credential)
+  return format("%s%s", credential.password, credential.consumer_id)
+end
+
+return {
+  --- Encrypt the password field credential table
+  -- @param credential The basic auth credential table
+  -- @return hash of the salted credential's password
+  encrypt = function(credential)
+    local salted = salt_password(credential)
+    return crypto.digest("sha1", salted)
+  end
+}

kong/plugins/basic-auth/daos.lua

@@ -1,13 +1,19 @@
 local BaseDao = require "kong.dao.cassandra.base_dao"
+local crypto = require "kong.plugins.basic-auth.crypto"
+
+local function encrypt_password(password, credential)
+  credential.password = crypto.encrypt(credential)
+  return true
+end
 
 local SCHEMA = {
   primary_key = {"id"},
   fields = {
-    id = { type = "id", dao_insert_value = true },
-    created_at = { type = "timestamp", dao_insert_value = true },
-    consumer_id = { type = "id", required = true, queryable = true, foreign = "consumers:id" },
-    username = { type = "string", required = true, unique = true, queryable = true },
-    password = { type = "string" }
+    id = {type = "id", dao_insert_value = true},
+    created_at = {type = "timestamp", dao_insert_value = true},
+    consumer_id = {type = "id", required = true, queryable = true, foreign = "consumers:id"},
+    username = {type = "string", required = true, unique = true, queryable = true},
+    password = {type = "string", func = encrypt_password}
   }
 }
 
@@ -20,4 +26,4 @@ function BasicAuthCredentials:new(properties)
   BasicAuthCredentials.super.new(self, properties)
 end
 
-return { basicauth_credentials = BasicAuthCredentials }
+return {basicauth_credentials = BasicAuthCredentials}

kong/plugins/basic-auth/schema.lua

@@ -1,6 +1,6 @@
 return {
   no_consumer = true,
   fields = {
-    hide_credentials = { type = "boolean", default = false }
+    hide_credentials = {type = "boolean", default = false}
   }
 }

kong/tools/utils.lua

@@ -121,7 +121,7 @@ function _M.load_module_if_exists(module_name)
   if status then
     return true, res
   -- Here we match any character because if a module has a dash '-' in its name, we would need to escape it.
-  elseif type(res) == "string" and string.find(res, "module '.*' not found") then
+  elseif type(res) == "string" and string.find(res, "module '"..module_name.."' not found", nil, true) then
     return false
   else
     error(res)

scripts/migration.py

@@ -10,11 +10,11 @@
 Arguments:
   -c, --config   path to your Kong configuration file
 Flags:
-  --purge    if already migrated, purge the old values
+  --purge        if already migrated, purge the old values
   -h             print help
 '''
 
-import getopt, sys, os.path, logging, json
+import getopt, sys, os.path, logging, json, hashlib
 
 log = logging.getLogger()
 log.setLevel("INFO")
@@ -27,6 +27,7 @@
     from cassandra.cluster import Cluster
     from cassandra import ConsistencyLevel, InvalidRequest
     from cassandra.query import SimpleStatement
+    from cassandra import InvalidRequest
 except ImportError as err:
     log.error(err)
     log.info("""This script requires cassandra-driver and PyYAML:
@@ -118,7 +119,8 @@ def migrate_plugins_configurations(session):
     session.execute("create index if not exists on plugins(api_id)")
     session.execute("create index if not exists on plugins(consumer_id)")
 
-    for plugin in session.execute("SELECT * FROM plugins_configurations"):
+    select_query = SimpleStatement("SELECT * FROM plugins_configurations", consistency_level=ConsistencyLevel.ALL)
+    for plugin in session.execute(select_query):
         # New plugins names
         plugin_name = plugin.name
         if plugin.name in new_names:
@@ -153,22 +155,49 @@ def migrate_rename_apis_properties(sessions):
     session.execute("CREATE INDEX IF NOT EXISTS ON apis(inbound_dns)")
 
     select_query = SimpleStatement("SELECT * FROM apis", consistency_level=ConsistencyLevel.ALL)
-
     for api in session.execute(select_query):
         session.execute("UPDATE apis SET inbound_dns = %s, upstream_url = %s WHERE id = %s", [api.public_dns, api.target_url, api.id])
 
     log.info("APIs properties renamed")
 
+def migrate_hash_passwords(session):
+    """
+    Hash all passwords in basicauth_credentials using sha1 and the consumer_id as the salt.
+    Also stores the plain passwords in a temporary column in case this script is run multiple times by the user.
+    Temporare column will be dropped on --purge.
+
+    :param session: opened cassandra session
+    """
+    log.info("Hashing basic-auth passwords...")
+
+    first_run = True
+
+    try:
+        session.execute("ALTER TABLE basicauth_credentials ADD plain_password text")
+    except InvalidRequest as err:
+        first_run = False
+
+    select_query = SimpleStatement("SELECT * FROM basicauth_credentials", consistency_level=ConsistencyLevel.ALL)
+    for credential in session.execute(select_query):
+        plain_password = credential.password if first_run else credential.plain_password
+        m = hashlib.sha1()
+        m.update(plain_password)
+        m.update(str(credential.consumer_id))
+        digest = m.hexdigest()
+        session.execute("UPDATE basicauth_credentials SET password = %s, plain_password = %s WHERE id = %s", [digest, plain_password, credential.id])
+
 def purge(session):
     session.execute("ALTER TABLE apis DROP public_dns")
     session.execute("ALTER TABLE apis DROP target_url")
+    session.execute("ALTER TABLE basicauth_credentials DROP plain_password")
     session.execute("DROP TABLE plugins_configurations")
-    session.execute("DELETE FROM schema_migrations WHERE id = 'migrations'")
+    session.execute(SimpleStatement("DELETE FROM schema_migrations WHERE id = 'migrations'", consistency_level=ConsistencyLevel.ALL))
 
 def migrate(session):
     migrate_schema_migrations_table(session)
     migrate_plugins_configurations(session)
     migrate_rename_apis_properties(session)
+    migrate_hash_passwords(session)
 
 def parse_arguments(argv):
     """
@@ -219,20 +248,20 @@ def main(argv):
             log.error("Please migrate your cluster to Kong 0.4.2 before running this script.")
             shutdown_exit(1)
 
-        if purge_cmd :
+        if purge_cmd:
             if not is_purged and is_migrated:
                 purge(session)
-                log.info("Cassandra purged from <0.5.0 data")
+                log.info("Cassandra purged from <0.5.0 data.")
             elif not is_purged and not is_migrated:
                 log.info("Cassandra not previously migrated. Run this script in migration mode before.")
                 shutdown_exit(1)
             else:
-                log.info("Cassandra already purged and migrated")
+                log.info("Cassandra already purged and migrated.")
         elif not is_migrated:
             migrate(session)
-            log.info("Cassandra migrated to Kong 0.5.0.")
+            log.info("Cassandra migrated to Kong 0.5.0. Restart Kong and run this script with '--purge'.")
         else:
-            log.info("Cassandra already migrated to Kong 0.5.0")
+            log.info("Cassandra already migrated to Kong 0.5.0. Restart Kong and run this script with '--purge'.")
 
         shutdown_exit(0)
     except getopt.GetoptError as err:

spec/plugins/basic-auth/access_spec.lua

@@ -46,7 +46,6 @@ describe("Authentication Plugin", function()
       assert.equal("Invalid authentication credentials", body.message)
     end)
 
-
     it("should return invalid credentials when the credential value is wrong in proxy-authorization", function()
       local response, status = http_client.get(PROXY_URL.."/get", {}, {host = "basicauth.com", ["proxy-authorization"] = "asd"})
       local body = cjson.decode(response)

spec/plugins/basic-auth/api_spec.lua

@@ -3,7 +3,7 @@ local http_client = require "kong.tools.http_client"
 local spec_helper = require "spec.spec_helpers"
 
 describe("Basic Auth Credentials API", function()
-  local BASE_URL, credential, consumer
+  local BASE_URL, credential, consumer, consumer_alice
 
   setup(function()
     spec_helper.prepare_db()
@@ -18,21 +18,43 @@ describe("Basic Auth Credentials API", function()
 
     setup(function()
       local fixtures = spec_helper.insert_fixtures {
-        consumer = {{ username = "bob" }}
+        consumer = {{username = "bob"}, {username = "alice"}}
       }
       consumer = fixtures.consumer[1]
+      consumer_alice = fixtures.consumer[2]
       BASE_URL = spec_helper.API_URL.."/consumers/bob/basic-auth/"
     end)
 
     describe("POST", function()
 
+      teardown(function()
+        local dao = spec_helper.get_env().dao_factory
+        local ok, err = dao.basicauth_credentials:delete(credential)
+        assert.True(ok)
+        assert.falsy(err)
+      end)
+
       it("[SUCCESS] should create a basicauth credential", function()
-        local response, status = http_client.post(BASE_URL, { username = "bob", password = "1234" })
+        local response, status = http_client.post(BASE_URL, {username = "bob", password = "1234"})
         assert.equal(201, status)
         credential = json.decode(response)
         assert.equal(consumer.id, credential.consumer_id)
       end)
 
+      it("[SUCCESS] should encrypt a password", function()
+        local base_url = spec_helper.API_URL.."/consumers/alice/basic-auth/"
+        local response, status = http_client.post(base_url, {username = "alice", password = "1234"})
+        assert.equal(201, status)
+
+        credential = json.decode(response)
+        assert.equal(consumer_alice.id, credential.consumer_id)
+        assert.not_equal("1234", credential.password)
+
+        local crypto = require "kong.plugins.basic-auth.crypto"
+        local hash = crypto.encrypt({consumer_id = consumer_alice.id, password = "1234"})
+        assert.equal(hash, credential.password)
+      end)
+
       it("[FAILURE] should return proper errors", function()
         local response, status = http_client.post(BASE_URL, {})
         assert.equal(400, status)
@@ -42,12 +64,9 @@ describe("Basic Auth Credentials API", function()
     end)
 
     describe("PUT", function()
-      setup(function()
-        spec_helper.get_env().dao_factory.basicauth_credentials:delete({id = credential.id})
-      end)
 
       it("[SUCCESS] should create and update", function()
-        local response, status = http_client.put(BASE_URL, { username = "bob", password = "1234" })
+        local response, status = http_client.put(BASE_URL, {username = "alice", password = "1234"})
         assert.equal(201, status)
         credential = json.decode(response)
         assert.equal(consumer.id, credential.consumer_id)
@@ -67,7 +86,7 @@ describe("Basic Auth Credentials API", function()
         local response, status = http_client.get(BASE_URL)
         assert.equal(200, status)
         local body = json.decode(response)
-        assert.equal(1, #(body.data))
+        assert.equal(2, #(body.data))
       end)
 
     end)