From ab8b04017442a2aca52013f438ca0546e94c98ff Mon Sep 17 00:00:00 2001 From: ljf Date: Tue, 28 Aug 2018 21:33:19 +0200 Subject: [PATCH 01/12] [enh] Validate password as configured --- helpers.lua | 36 +++++++++++++++++++++++++----------- portal/locales/en.json | 9 +++++++++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/helpers.lua b/helpers.lua index ea7f67b..3d926d0 100644 --- a/helpers.lua +++ b/helpers.lua @@ -626,22 +626,36 @@ function edit_user() then -- and the new password against the confirmation field's content if args.newpassword == args.confirm then - local dn = conf["ldap_identifier"].."="..user..","..conf["ldap_group"] + -- Check password validity + local validatepw = io.popen("python /usr/lib/moulinette/yunohost/utils/password.py '" ..args.newpassword:gsub("'", "'\\''").."' 2>&1 || echo ::ERROR::", 'r') + local validation = validatepw:read() + local validation_error = validatepw:read() + validatepw:close() + if validation_error == nil then - -- Open the LDAP connection - local ldap = lualdap.open_simple(conf["ldap_host"], dn, args.currentpassword) + local dn = conf["ldap_identifier"].."="..user..","..conf["ldap_group"] - local password = hash_password(args.newpassword) + -- Open the LDAP connection + local ldap = lualdap.open_simple(conf["ldap_host"], dn, args.currentpassword) + + local password = hash_password(args.newpassword) - -- Modify the LDAP information - if ldap:modify(dn, {'=', userPassword = password }) then - flash("win", t("password_changed")) + -- Modify the LDAP information + if ldap:modify(dn, {'=', userPassword = password }) then + if validation == nil then + flash("win", t("password_changed")) + else + flash("win", t(validation)) + end - -- Reset the password cache - cache:set(user.."-password", args.newpassword, conf["session_timeout"]) - return redirect(conf.portal_url.."info.html") + -- Reset the password cache + cache:set(user.."-password", args.newpassword, conf["session_timeout"]) + return redirect(conf.portal_url.."info.html") + else + flash("fail", t("password_changed_error")) + end else - flash("fail", t("password_changed_error")) + flash("fail", t(validation)) end else flash("fail", t("password_not_match")) diff --git a/portal/locales/en.json b/portal/locales/en.json index 9730672..6fbf0c4 100644 --- a/portal/locales/en.json +++ b/portal/locales/en.json @@ -22,6 +22,15 @@ "password_changed": "Password successfully changed", "password_changed_error": "An error occurred on password changing", "password_not_match": "New passwords don't match", + "password_too_simple_1": "Password needs to be at least 6 characters long", + "password_too_simple_2": "Password needs to be at least 8 characters long and contains digit, upper and lower characters", + "password_too_simple_3": "Password needs to be at least 8 characters long and contains digit, upper, lower and special characters", + "password_too_simple_4": "Password needs to be at least 12 characters long and contains digit, upper, lower and special characters", + "password_listed_1": "This password is in a well known list. Please make it unique. Password needs to be at least 6 characters long", + "password_listed_2": "This password is in a well known list. Please make it unique. Password needs to be at least 8 characters long and contains digit, upper and lower characters", + "password_listed_3": "This password is in a well known list. Please make it unique. Password needs to be at least 8 characters long and contains digit, upper, lower and special characters", + "password_listed_4": "This password is in a well known list. Please make it unique. Password needs to be at least 12 characters long and contains digit, upper, lower and special characters", + "password_advice": "Password successfully changed. Note: to improve your password make it with at least 8 characters and put digits, upper, lower and special characters", "wrong_current_password": "Current password is wrong", "invalid_mail": "Invalid mail address", "invalid_domain": "Invalid domain in", From 95e1c1cd2f89af4914fb49ebdea97b307cbc59bf Mon Sep 17 00:00:00 2001 From: ljf Date: Wed, 29 Aug 2018 00:07:48 +0200 Subject: [PATCH 02/12] [fix] Secure password transmission --- helpers.lua | 46 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/helpers.lua b/helpers.lua index 3d926d0..89fba97 100644 --- a/helpers.lua +++ b/helpers.lua @@ -597,6 +597,30 @@ function ensure_user_password_uses_strong_hash(ldap, user, password) end end +-- Read result of a command after given it securely the password +function secure_cmd_password(cmd, password) + -- Check password validity + math.randomseed( os.time() ) + local tmp_file = "/tmp/ssowat_"..math.random() + local w_pwd = io.popen(string.format(cmd, tmp_file), 'w') + w_pwd:write(password) + w_pwd:write("") + w_pwd:close() + local r_pwd = io.open(tmp_file, 'r') + local i = 0 + local text = "" + for line in io.lines(tmp_file) do + i = i + 1 + if i > 4 then + text = text..line.."\n" + end + end + r_pwd:close() + os.remove(tmp_file) + ngx.log(ngx.STDERR, text) + return text +end + -- Compute the user modification POST request -- It has to update cached information and edit the LDAP user entry -- according to the changes detected. @@ -627,10 +651,20 @@ function edit_user() -- and the new password against the confirmation field's content if args.newpassword == args.confirm then -- Check password validity - local validatepw = io.popen("python /usr/lib/moulinette/yunohost/utils/password.py '" ..args.newpassword:gsub("'", "'\\''").."' 2>&1 || echo ::ERROR::", 'r') - local validation = validatepw:read() - local validation_error = validatepw:read() - validatepw:close() + local valid_result = secure_cmd_password("( python /usr/lib/moulinette/yunohost/utils/password.py 2>&1 || echo ::ERROR:: ) | tee -a %s", args.newpassword) + -- We remove 4 lines due to a Warning message + local i = 0 + local validation_error = nil + local result_msg = nil + + for line in string.gmatch(valid_result, "[^\n]+") do + if i == 0 then + result_msg = line + else + validation_error = line + end + i = i + 1 + end if validation_error == nil then local dn = conf["ldap_identifier"].."="..user..","..conf["ldap_group"] @@ -645,7 +679,7 @@ function edit_user() if validation == nil then flash("win", t("password_changed")) else - flash("win", t(validation)) + flash("win", t(result_msg)) end -- Reset the password cache @@ -655,7 +689,7 @@ function edit_user() flash("fail", t("password_changed_error")) end else - flash("fail", t(validation)) + flash("fail", t(result_msg)) end else flash("fail", t("password_not_match")) From 945b04cc67da3c4ae034129bb5cc79dfd60ae7e7 Mon Sep 17 00:00:00 2001 From: ljf Date: Wed, 29 Aug 2018 00:47:59 +0200 Subject: [PATCH 03/12] [fix] Regex todo --- helpers.lua | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/helpers.lua b/helpers.lua index 89fba97..e32bb9b 100644 --- a/helpers.lua +++ b/helpers.lua @@ -598,7 +598,7 @@ function ensure_user_password_uses_strong_hash(ldap, user, password) end -- Read result of a command after given it securely the password -function secure_cmd_password(cmd, password) +function secure_cmd_password(cmd, password, start) -- Check password validity math.randomseed( os.time() ) local tmp_file = "/tmp/ssowat_"..math.random() @@ -611,10 +611,13 @@ function secure_cmd_password(cmd, password) local text = "" for line in io.lines(tmp_file) do i = i + 1 - if i > 4 then + if i > start then text = text..line.."\n" end end + if i > start then + text = text:sub(1, -2) + end r_pwd:close() os.remove(tmp_file) ngx.log(ngx.STDERR, text) @@ -651,7 +654,7 @@ function edit_user() -- and the new password against the confirmation field's content if args.newpassword == args.confirm then -- Check password validity - local valid_result = secure_cmd_password("( python /usr/lib/moulinette/yunohost/utils/password.py 2>&1 || echo ::ERROR:: ) | tee -a %s", args.newpassword) + local valid_result = secure_cmd_password("( python /usr/lib/moulinette/yunohost/utils/password.py 2>&1 || echo ::ERROR:: ) | tee -a %s", args.newpassword, 4) -- We remove 4 lines due to a Warning message local i = 0 local validation_error = nil @@ -883,11 +886,9 @@ end -- hash the user password using sha-512 and using {CRYPT} to uses linux auth system -- because ldap doesn't support anything stronger than sha1 function hash_password(password) - -- TODO is the password checked by regex? we don't want to - -- allow shell injection - local mkpasswd = io.popen("mkpasswd --method=sha-512 '" ..password:gsub("'", "'\\''").."'") - local hashed_password = "{CRYPT}"..mkpasswd:read() - mkpasswd:close() + local hashed_password = secure_cmd_password("mkpasswd --method=sha-512 | tee -a %s", password, 0) + ngx.log(ngx.STDERR, hashed_password) + hashed_password = "{CRYPT}"..hashed_password return hashed_password end @@ -901,7 +902,7 @@ function login() local uri_args = ngx.req.get_uri_args() args.user = string.lower(args.user) - + local user = authenticate(args.user, args.password) if user then ngx.status = ngx.HTTP_CREATED From d83b522d5091720bdbe699ea7a6c3454febce7b7 Mon Sep 17 00:00:00 2001 From: ljf Date: Wed, 29 Aug 2018 00:56:24 +0200 Subject: [PATCH 04/12] [fix] Remove some nginx debug log --- helpers.lua | 1 - 1 file changed, 1 deletion(-) diff --git a/helpers.lua b/helpers.lua index e32bb9b..a384efb 100644 --- a/helpers.lua +++ b/helpers.lua @@ -887,7 +887,6 @@ end -- because ldap doesn't support anything stronger than sha1 function hash_password(password) local hashed_password = secure_cmd_password("mkpasswd --method=sha-512 | tee -a %s", password, 0) - ngx.log(ngx.STDERR, hashed_password) hashed_password = "{CRYPT}"..hashed_password return hashed_password end From 349d486cec74add0ddd4a7f556013ad9a82dd0ba Mon Sep 17 00:00:00 2001 From: ljf Date: Wed, 29 Aug 2018 01:08:36 +0200 Subject: [PATCH 05/12] [fix] Remove some nginx debug log --- helpers.lua | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/helpers.lua b/helpers.lua index a384efb..723b62b 100644 --- a/helpers.lua +++ b/helpers.lua @@ -602,8 +602,10 @@ function secure_cmd_password(cmd, password, start) -- Check password validity math.randomseed( os.time() ) local tmp_file = "/tmp/ssowat_"..math.random() - local w_pwd = io.popen(string.format(cmd, tmp_file), 'w') + local w_pwd = io.popen("("..cmd..") tee -a "..tmp_file, 'w') w_pwd:write(password) + -- This second write is just to validate the password question + -- Do not remove w_pwd:write("") w_pwd:close() local r_pwd = io.open(tmp_file, 'r') @@ -654,7 +656,7 @@ function edit_user() -- and the new password against the confirmation field's content if args.newpassword == args.confirm then -- Check password validity - local valid_result = secure_cmd_password("( python /usr/lib/moulinette/yunohost/utils/password.py 2>&1 || echo ::ERROR:: ) | tee -a %s", args.newpassword, 4) + local valid_result = secure_cmd_password("python /usr/lib/moulinette/yunohost/utils/password.py 2>&1 || echo ::ERROR::", args.newpassword, 4) -- We remove 4 lines due to a Warning message local i = 0 local validation_error = nil @@ -886,7 +888,7 @@ end -- hash the user password using sha-512 and using {CRYPT} to uses linux auth system -- because ldap doesn't support anything stronger than sha1 function hash_password(password) - local hashed_password = secure_cmd_password("mkpasswd --method=sha-512 | tee -a %s", password, 0) + local hashed_password = secure_cmd_password("mkpasswd --method=sha-512", password, 0) hashed_password = "{CRYPT}"..hashed_password return hashed_password end From 7627101eb5c10a2ea77ad5ba0743407303d9263b Mon Sep 17 00:00:00 2001 From: ljf Date: Wed, 29 Aug 2018 01:26:19 +0200 Subject: [PATCH 06/12] [enh] Simplify code thanks to change on password.py --- helpers.lua | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/helpers.lua b/helpers.lua index 723b62b..99cfc83 100644 --- a/helpers.lua +++ b/helpers.lua @@ -609,20 +609,9 @@ function secure_cmd_password(cmd, password, start) w_pwd:write("") w_pwd:close() local r_pwd = io.open(tmp_file, 'r') - local i = 0 - local text = "" - for line in io.lines(tmp_file) do - i = i + 1 - if i > start then - text = text..line.."\n" - end - end - if i > start then - text = text:sub(1, -2) - end + text = r_pwd:read "*a" r_pwd:close() os.remove(tmp_file) - ngx.log(ngx.STDERR, text) return text end @@ -656,19 +645,11 @@ function edit_user() -- and the new password against the confirmation field's content if args.newpassword == args.confirm then -- Check password validity - local valid_result = secure_cmd_password("python /usr/lib/moulinette/yunohost/utils/password.py 2>&1 || echo ::ERROR::", args.newpassword, 4) - -- We remove 4 lines due to a Warning message - local i = 0 - local validation_error = nil - local result_msg = nil + local result_msg = secure_cmd_password("python /usr/lib/moulinette/yunohost/utils/password.py", args.newpassword) + validation_error = true - for line in string.gmatch(valid_result, "[^\n]+") do - if i == 0 then - result_msg = line - else - validation_error = line - end - i = i + 1 + if result_msg == 'password_advice' or result_msg == nil or result_msg == "" then + validation_error = nil end if validation_error == nil then @@ -888,7 +869,7 @@ end -- hash the user password using sha-512 and using {CRYPT} to uses linux auth system -- because ldap doesn't support anything stronger than sha1 function hash_password(password) - local hashed_password = secure_cmd_password("mkpasswd --method=sha-512", password, 0) + local hashed_password = secure_cmd_password("mkpasswd --method=sha-512", password) hashed_password = "{CRYPT}"..hashed_password return hashed_password end From 410ba2e4a7be437a6c5a47d52d2afa0a8e84beed Mon Sep 17 00:00:00 2001 From: ljf Date: Wed, 29 Aug 2018 02:55:02 +0200 Subject: [PATCH 07/12] [fix] Remove extra end line of the cmd run with popen --- helpers.lua | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/helpers.lua b/helpers.lua index 99cfc83..5e6f4b4 100644 --- a/helpers.lua +++ b/helpers.lua @@ -602,7 +602,7 @@ function secure_cmd_password(cmd, password, start) -- Check password validity math.randomseed( os.time() ) local tmp_file = "/tmp/ssowat_"..math.random() - local w_pwd = io.popen("("..cmd..") tee -a "..tmp_file, 'w') + local w_pwd = io.popen("("..cmd..") | tee -a "..tmp_file, 'w') w_pwd:write(password) -- This second write is just to validate the password question -- Do not remove @@ -610,6 +610,9 @@ function secure_cmd_password(cmd, password, start) w_pwd:close() local r_pwd = io.open(tmp_file, 'r') text = r_pwd:read "*a" + if text:sub(-1, -1) == "\n" then + text = text:sub(1, -2) + end r_pwd:close() os.remove(tmp_file) return text @@ -647,7 +650,7 @@ function edit_user() -- Check password validity local result_msg = secure_cmd_password("python /usr/lib/moulinette/yunohost/utils/password.py", args.newpassword) validation_error = true - + ngx.log(ngx.STDERR, result_msg) if result_msg == 'password_advice' or result_msg == nil or result_msg == "" then validation_error = nil end From deeb30637e9b327c8f9a720cc020817d1ca692d1 Mon Sep 17 00:00:00 2001 From: ljf Date: Wed, 29 Aug 2018 02:58:17 +0200 Subject: [PATCH 08/12] [fix] Remove nginx log --- helpers.lua | 1 - 1 file changed, 1 deletion(-) diff --git a/helpers.lua b/helpers.lua index 5e6f4b4..bd8903a 100644 --- a/helpers.lua +++ b/helpers.lua @@ -650,7 +650,6 @@ function edit_user() -- Check password validity local result_msg = secure_cmd_password("python /usr/lib/moulinette/yunohost/utils/password.py", args.newpassword) validation_error = true - ngx.log(ngx.STDERR, result_msg) if result_msg == 'password_advice' or result_msg == nil or result_msg == "" then validation_error = nil end From e4ee83cc8efa7f07d90c39f7b89c6fed07773d6e Mon Sep 17 00:00:00 2001 From: ljf Date: Wed, 29 Aug 2018 03:00:13 +0200 Subject: [PATCH 09/12] [fix] Add a small comment --- helpers.lua | 2 ++ 1 file changed, 2 insertions(+) diff --git a/helpers.lua b/helpers.lua index bd8903a..f9ce0b2 100644 --- a/helpers.lua +++ b/helpers.lua @@ -610,6 +610,8 @@ function secure_cmd_password(cmd, password, start) w_pwd:close() local r_pwd = io.open(tmp_file, 'r') text = r_pwd:read "*a" + + -- Remove the extra end line if text:sub(-1, -1) == "\n" then text = text:sub(1, -2) end From 90998555f049c3007032167a24f651ff7a48504a Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 26 Oct 2018 22:16:25 +0000 Subject: [PATCH 10/12] Add warning about password strength --- portal/locales/en.json | 1 + portal/password.html | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/portal/locales/en.json b/portal/locales/en.json index 6fbf0c4..e442750 100644 --- a/portal/locales/en.json +++ b/portal/locales/en.json @@ -31,6 +31,7 @@ "password_listed_3": "This password is in a well known list. Please make it unique. Password needs to be at least 8 characters long and contains digit, upper, lower and special characters", "password_listed_4": "This password is in a well known list. Please make it unique. Password needs to be at least 12 characters long and contains digit, upper, lower and special characters", "password_advice": "Password successfully changed. Note: to improve your password make it with at least 8 characters and put digits, upper, lower and special characters", + "good_practices_about_user_password": "You are now about to define a new user password. The password should be at least 8 characters - though it is good practice to use longer password (i.e. a passphrase) and/or to use various kind of characters (uppercase, lowercase, digits and special characters).", "wrong_current_password": "Current password is wrong", "invalid_mail": "Invalid mail address", "invalid_domain": "Invalid domain in", diff --git a/portal/password.html b/portal/password.html index 3864f16..8b235b7 100644 --- a/portal/password.html +++ b/portal/password.html @@ -10,6 +10,11 @@
+ +
+ {{t_good_practices_about_user_password}} +
+
From d305f6e9199d800d311ace27d1d486caf1833ad9 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 31 Oct 2018 18:40:36 +0000 Subject: [PATCH 11/12] Sync messages with yunohost's branch --- portal/locales/en.json | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/portal/locales/en.json b/portal/locales/en.json index e442750..e09467a 100644 --- a/portal/locales/en.json +++ b/portal/locales/en.json @@ -22,15 +22,11 @@ "password_changed": "Password successfully changed", "password_changed_error": "An error occurred on password changing", "password_not_match": "New passwords don't match", - "password_too_simple_1": "Password needs to be at least 6 characters long", + "password_listed": "This password is among the most used password in the world. Please choose something a bit more unique.", + "password_too_simple_1": "Password needs to be at least 8 characters long", "password_too_simple_2": "Password needs to be at least 8 characters long and contains digit, upper and lower characters", "password_too_simple_3": "Password needs to be at least 8 characters long and contains digit, upper, lower and special characters", "password_too_simple_4": "Password needs to be at least 12 characters long and contains digit, upper, lower and special characters", - "password_listed_1": "This password is in a well known list. Please make it unique. Password needs to be at least 6 characters long", - "password_listed_2": "This password is in a well known list. Please make it unique. Password needs to be at least 8 characters long and contains digit, upper and lower characters", - "password_listed_3": "This password is in a well known list. Please make it unique. Password needs to be at least 8 characters long and contains digit, upper, lower and special characters", - "password_listed_4": "This password is in a well known list. Please make it unique. Password needs to be at least 12 characters long and contains digit, upper, lower and special characters", - "password_advice": "Password successfully changed. Note: to improve your password make it with at least 8 characters and put digits, upper, lower and special characters", "good_practices_about_user_password": "You are now about to define a new user password. The password should be at least 8 characters - though it is good practice to use longer password (i.e. a passphrase) and/or to use various kind of characters (uppercase, lowercase, digits and special characters).", "wrong_current_password": "Current password is wrong", "invalid_mail": "Invalid mail address", From cb96f848d35e93e08cb895ef8037758175c16bf3 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 31 Oct 2018 18:55:07 +0000 Subject: [PATCH 12/12] This got removed --- helpers.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helpers.lua b/helpers.lua index f9ce0b2..2ee003f 100644 --- a/helpers.lua +++ b/helpers.lua @@ -652,7 +652,7 @@ function edit_user() -- Check password validity local result_msg = secure_cmd_password("python /usr/lib/moulinette/yunohost/utils/password.py", args.newpassword) validation_error = true - if result_msg == 'password_advice' or result_msg == nil or result_msg == "" then + if result_msg == nil or result_msg == "" then validation_error = nil end if validation_error == nil then