From f5ee2f68b97e64c1bdb6275520cbfb0d04b3119b Mon Sep 17 00:00:00 2001 From: Garritt McCune Date: Mon, 29 Mar 2021 19:15:15 -0500 Subject: [PATCH] Changed the settings getting functions to accept a fll key path instead of only a Section / Key pair. Changed the placement of the pepper in the password hashing function to be in complicance with the OWASP's recommendation for using a pwpper. --- .vs/SecureCore/v16/.suo | Bin 83456 -> 84992 bytes SecureCore/AppSettingsManager.cs | 8 ++++---- SecureCore/Authentication/PasswordManager.cs | 10 +++++----- SecureCore/Controllers/AuthController.cs | 16 ++++++++++++++-- SecureCore/appsettings.json | 3 ++- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/.vs/SecureCore/v16/.suo b/.vs/SecureCore/v16/.suo index db49d7eb4384388c8a0dee7f2b43a6dbcf078d22..23974ff24e825749a01faa7a32fa0e3a3ba79dc3 100644 GIT binary patch delta 5305 zcmZqZVQrYfI>AQNk%57Mg@J+L-~a#r85kHCm>C!tBsK<0Gw~jfTy^fQ0|SFS0|SE#)C7=P5e5bZ z6DVJqfq@}>awm&13mXFi!{h@j2Bvll3=EtM3=AF&3=Fmm3=B353=B>T3=FJLEg(mM zi~(U@1_lOq1_lQ2$*Qc{yv7U+3`$T{5tCzC!+HCmyaEOWhKh-S@(NW93=B043=Gu_ z3=Fjl3=H)Q3=DM)3=9nn3=B;S3=ET)a(E9|d!NS}X7$Kn266@n!@>^Pd7!Yv2r~u- z21N!2202iSFfcHvFfcHvK|>A}iXihqp$MWu@qreKVj%JVSc7r0h>pPIGb}1NLy;XC zBFN4pH53&Ih2sCoj9kK-Rk-djPAm}MU}R)q;9_K8n9Sj%GbvAGvWHUwhc+VvgAPb+ zgOdUa$SsqvIB6Jy7$EG#$iU#t$iU#j2uaf*F&Oq_WMJ@uiBC+tJc-X`vWuv}<`TXh zMp|1XP{Nkaz`&3pG`ULP6o&x=0|P%O)yl^zPBvg-XEd8!Dp=2GI{B;Me^!ul87BV} zs%G?dMH#z_R(LC_B?6krGD7&7~sUjEf|g7Rj(6dtmw{K1Q|aUeb(Cn=_PV zFi&sbW^98ClX^yh$)%?C-29;MfRs9u|C@WV zfC`Vv(12WAV!_D7$2hsnvWZ2ApMha|4L4&8F~NY{kpeP=y|>7YWl{i|qbHXdcQ10` zSro;zD2M}x(d+OUJ-Jawo3V0ZpgZHD7^X>tQ%`w4t0^M`1JmZ8m0nDf4~y7tZis7O zT$IGJD1&WN+F?dc#zY1N22i?#1{F6e$Zrgr|K>BZEXrfrRPvP(RCIzX0mjMy%6~DY zZ2non&NwN~g#)Bj11Y&~E{W@5oK*J%q@<3W9jYBvyhF4zFf8g|n$&lK1ys^Q?PK5k zrH_YkQ_o>0R)z$SS^cp(o4-sDXJs)rvvk|cKh2P_K81mSfsuiM0aVh&@aS~@RCHK) zh}po&pg3z$*~WQ_H>3@m3jY}&d?+=yc)o;zQ-0*0hoZHLjvzI{K`-Chob7(@AY|aQ z=z~t#@zs-Zp8wHvnijUH>C>TOX42vYpsIjDWoL%M=bw5g>;wfxrNgZ`llM>eVRkXF z+{`!4Xuut_dH+mDP6JRH{>{w5pkiQRYG7h+V5Vzqo@}aXl4y{kYnft{sGDkLX<%S! zVQ!LUlEluidUD`n@5%9N6B$=RSuzsEsYPX($*ILLrNya5DTyVCP=U!C7i&+xv~@OA>=2Hs~c6GpYrrCYKhaI_DRq#z2LO zW5`rDxp%eOJhTqK%A;W-(3LbYODf za>dC3YvxaG*nD8~`!#}$ldr8+n=Z}F=qw0w1Ek!6l-{zF`Tqq>j$bP?*=ia8(5>1Up6A>f}$xMbOB1VBlXP73xUpbS=l(gyMMn=XdlO6ZEPHzxnAX}swXl$==w zLCG029o}eUoSc1GfpHbGaT1Og?qDbeC#7PBRE8o31%^b1Qic+S3;5{6WUT!sRM z9ELarJxh9``YYi{o0zT$3PWm!S43z31<(b9-c=5Xx z99eo_FHKflEefhf2*#NRmMX+=@`ZmU(y)letUAE*@PvRSk6C3L1v?5-T?kJOe5f&b z!B_6de{&QVgOPcYcmH+<>};$v9c?ksGu|5oC0R>Xd=f(v$!H z^ydPlSWwT*Wf#~=y|3Jp7c4cJY_?jA(F3ZGdvfe=F(kI=|Z^Ja3B-V+fiE?_|Xs@yY+* z224J%R%rSuCdNPzF-5Qp5{nEB40;DvfqKo`^Vu0EvP}NC!o!3>?ZvtPd&^xdK z)bS?LH7ua^!DRhq!Jr(w+3!I;m&EUqzkWVyqll8uOL4%)0DFl*;{}gdyOCq3T z45=1~iwvR3_qI3_YN|{xv|^Nf+Ba5Da9Ediy0$t((n zGDaqbhQ^L=uC+D65$8c2GX{p0GZY`F2wMGPoILTd{&WX!MvcjZD5amSsnZqY8Re!s2r!CFH{oaOm>wd)xPWmDNXl~h20_M(>0bmGqe#`zG(E(cF%4w2 z%l1n`j8=?{6F?@WPG6wHsKY2Z`M#Ut^bLHBUeo)m7)2Oaw#$n$9%f{mGTl*%v3#AQNg@J*Ag@J+L-~a#r85kHCm>C!t#5V>?Gx0KP?+MlvyUys%z`$@| zawStWW837fO!cf>j0_AMlPj64>nDKpN`Z7SFi0>kF#P?G3P5H$GB7Z3GcYi4GB7aM zFfcGkGcYhnGB7ZJR4OtsFvvmeR$*XZP=ktxGcYi4LCq6@im@{=FgQTj?hslGWX69H z1_lNb1_lOJ1_lOY1_p+R$(=056C(sBpJ7q4VPjxmuxDUk;9y{2@MK_Muw!6guw`Ii zP=J{5-w8^COai%K^PW@Ag6%%AdD^W948yHh&zH*!Ga2;5)@RR zD8Ld_AZd6=fub8DqQj*w03goJPlNq^$SwLRdEWq`Gkr9nQiO+>25tk*itgVa;41$w? zic~Y!O)eF!XRT&rU|^a2Q@on7d~>NpH{&8PrbSXrlOzQu3-Fm>w{DRP%Q81crs<{oS`&>87#EOkuhp>l*Te<(MARahE`~5067SR`6mWy^Gt#YP6oNq$G~E; zh=In&f)K_{dK^rX4UA+aCdlAaGs&ccv37E)Nj+oD?LIn2~`Ygpq+El#ziU zjFEvMoRNVcf{}qCl97QSijjdKnvsDahLM3GmXU!Wj*)>Oo{=GoA%T&BA(4@RA&HTJ zA(@eZA%&5FA(fGVA&rrNA)S$dA%l^DA(N4TA&ZfLA)AqbA%~HHA(xSXA&-%PA)k?f zp@5Nrp^%Y*p@@-zp_q|@p@flvp_CC)mVyGMf{}rtl97R-YO*1ZFk3w%149Gj=6}vi zJd>Qz;&gJUY4@Tajzs}Xi(;52Rk?8JGcYh9$Kd9Yq#j1B32YIzIAaA_|8HYrIpd-f zmPJiWi!#_2l`t(TVA)jml@XL#!19y-3YHVfMDg`p+KdbgOyE*}^1t$5j1ilERShnNkV3jY}&d?+=yc)o;z)3mTnO`i@OGm{oK0G03zJ3g-N{`^zVsW)$R z*W@Kr(^(8HjV)+k_U0wiV>k^!IqWwx1A|Ixl4+8;sfDR-nu%$mu8BpGsjfwuiJ`7# za*C00T8d?wnW-fk!|KTsA9_#Tuqko#oyCESlg$?|o!q`?!)D#3p^PkS3@+=S(lV3Z zFY=f?S6yiG+7&Cgl%Q3r%RIRB{Kedp{g>!Y{=ah8WWQgWn-{HeViW_VA#l_&=p6u6 z#h{YNWd>Z&)TKs~g*WC-uV-XbnVhoe?V=e>lhz!V9JtwG(OSky8xBlXT&XFT3o(F! zfkE%UItB&?-l_bI0h2du0=bBPIv*<|_vD6+2POw@(VuS3!6-a=@~-*QtvMMLCfjVb z=3vwaP6g?oTwS0vU7DRyisiw(|Nkd%T&_BO9w(zH(|X25vsfnW5t+=mLT9qW9z*`j z{JfZg#Ny)e{GycH#JtT7TN)Urhv_n^Ojg+yBT&f3zyK=e!3lziL1eO^uL!dsgr0O( zax&{)nav;e2(Td{!6eDhG||M&Qr9xgBt_T6EYU#M!q7NHH_gnlXvdNBnU8gq~F!D^+IiL)U zCQxt+Fgj2-ZtzCTjYdYpOon8JM1~TEOon`hJcbyCVun0`A(^3g@|jy<(-RFCwI}QB;bUZ&9Dh)MvioKJ%?-E9m_$q&Oc;z9%ovOqZu2uR z2!pfy}Hmsx!7v5@2j*+3mz*?zaYryIz63@@jGVL-JZ|RIFV`c!M{kVpeY9|#j=cx zk!hO>qX{Ep)AUAN#?oXo^WNZP6Sx$eT&u9ZOQh-ry z`h5e&xzne(GNw)MFlN+Z6q|fdNq+K!x!;%>7+9wVnlMhB{=}KlYWe{eMuX`xCX5!0 ztDt6RGwOjFVlIq2)9;%wuAd%a0&#@Q_5&7-4vdq1W5u?M*)jSsPiJysGy}OCY>Ez( zBLi;#xlC7*XOx-#%#l$BDyEp#X$B9sqlHlIQ@`5W7PC0P(U{5GFnY;SRgSy%9YUy6wBZs^8m$9 x&Nc_eBu2EbWOUsA-<^?_Y4QgZsqJzejE@+}3cnT8@mV}MFO*|DhX>;iW&ouM-QEBI diff --git a/SecureCore/AppSettingsManager.cs b/SecureCore/AppSettingsManager.cs index 10c6a96..3e17b4a 100644 --- a/SecureCore/AppSettingsManager.cs +++ b/SecureCore/AppSettingsManager.cs @@ -27,13 +27,13 @@ namespace SecureCore catch { return false; } } - public static bool TryGetSettingString(string sectionName, string key, out string setting) + public static bool TryGetSettingString(string keyPath, out string setting) { setting = string.Empty; try { - var token = Settings.SelectToken($"{sectionName}.{key}"); + var token = Settings.SelectToken(keyPath); if (token == null) return false; @@ -45,13 +45,13 @@ namespace SecureCore catch { return false; } } - public static bool TryGetSettingInt(string sectionName, string key, out int setting) + public static bool TryGetSettingInt(string keyPath, out int setting) { setting = 0; try { - var token = Settings.SelectToken($"{sectionName}.{key}"); + var token = Settings.SelectToken(keyPath); if (token == null) return false; diff --git a/SecureCore/Authentication/PasswordManager.cs b/SecureCore/Authentication/PasswordManager.cs index 22578f6..c7e0d5e 100644 --- a/SecureCore/Authentication/PasswordManager.cs +++ b/SecureCore/Authentication/PasswordManager.cs @@ -26,7 +26,7 @@ namespace SecureCore.Authentication { if (!string.IsNullOrEmpty(Pepper)) throw new InvalidOperationException("The PasswordManager's settings have already been initialized. Operation aborted."); - if (AppSettingsManager.TryGetSettingInt(SectionName, "MaxLength", out int maxPasswordLength)) + if (AppSettingsManager.TryGetSettingInt($"{SectionName}.MaxLength", out int maxPasswordLength)) { //As noted in this article https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#maximum-password-lengths //allowing passwords that are too long can result in a denial-of-service attack. So we must enforce an upper bound @@ -36,7 +36,7 @@ namespace SecureCore.Authentication } else maxPasswordLength = AbsoluteMaxPasswordLength; - if (AppSettingsManager.TryGetSettingInt(SectionName, "MinLength", out int minPasswordLength)) + if (AppSettingsManager.TryGetSettingInt($"{SectionName}.MinLength", out int minPasswordLength)) { //There was no mention of a min password length in the above article, so I've chosen on a whim that 16 should //be a safe enough min on a password's length. So as usual, just ignore settings that are out of bounds and @@ -54,7 +54,7 @@ namespace SecureCore.Authentication MinPasswordLength = minPasswordLength; MaxPasswordLength = maxPasswordLength; //Now read in the pepper. A pepper being a string of characters at least 32 characters long that is NOT stored in the database and is used in conjunction with hashing sensitive user data. - if (AppSettingsManager.TryGetSettingString(SectionName, "Pepper", out string pepper)) + if (AppSettingsManager.TryGetSettingString($"{SectionName}.Pepper", out string pepper)) { //As noted here https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html a pepper should be at least 32 bytes in size. if (pepper.Length < AbsoluteMinPepperLength) throw new Exception("A pepper must be at least 32 characters long for security reasons."); @@ -64,7 +64,7 @@ namespace SecureCore.Authentication Pepper = pepper; //Finally, the work factor (A.K.A. iterations) for the hashing algorithm. - if (AppSettingsManager.TryGetSettingInt(SectionName, "Iterations", out int iterations)) + if (AppSettingsManager.TryGetSettingInt($"{SectionName}.Iterations", out int iterations)) { //The work factor must be of a certain strength and if it fails this check then we will be forced to ignore it and use the recommended work factor //as stated here: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pbkdf2 @@ -171,7 +171,7 @@ namespace SecureCore.Authentication private static string GetHash(string password, byte[] salt) { - return Convert.ToBase64String(KeyDerivation.Pbkdf2($"{password}{Pepper}", salt, KeyType, Iterations, KeySize)); + return Convert.ToBase64String(KeyDerivation.Pbkdf2($"{Pepper}{password}", salt, KeyType, Iterations, KeySize)); } } } diff --git a/SecureCore/Controllers/AuthController.cs b/SecureCore/Controllers/AuthController.cs index 173bcb4..6ff3bf8 100644 --- a/SecureCore/Controllers/AuthController.cs +++ b/SecureCore/Controllers/AuthController.cs @@ -12,7 +12,12 @@ namespace SecureCore.Controllers [ApiController] public class AuthController : Controller { - public static string BaseUrl { get; set; } + [HttpPost("IsLoggedIn")] + [AcceptVerbs("POST")] + public IActionResult IsLoggedIn() + { + return Ok(); + } //TODO: Login will only ever return messages like "Wrong username / password." whereas register can return messages like "User exists.", "Password to weak", or "Password in top 100 most used.". [HttpPost("login")] [AcceptVerbs("POST")] @@ -167,9 +172,11 @@ namespace SecureCore.Controllers { PasswordManager.InsertPasswordResetRequest(email, token, DateTime.Now.AddHours(1), agent, ip, connectionString); + AppSettingsManager.TryGetSettingString("PasswordSettings.BaseURL", out string baseAddress); + token = HttpUtility.UrlEncode(token); //TODO: Email the link to the supplied email. - return Ok($"192.168.255.200:5000/auth/ResetPassword?token={token}{Environment.NewLine}"); + return Ok($"{baseAddress}/ResetPassword?token={token}{Environment.NewLine}"); } catch(Exception e) { @@ -177,6 +184,11 @@ namespace SecureCore.Controllers } } + //TODO: Review https://docs.microsoft.com/en-us/aspnet/core/security/anti-request-forgery?view=aspnetcore-5.0 and https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html + /// + /// + /// + /// private CookieOptions GetCookieOptions() { return new CookieOptions diff --git a/SecureCore/appsettings.json b/SecureCore/appsettings.json index ab529bb..d55f02e 100644 --- a/SecureCore/appsettings.json +++ b/SecureCore/appsettings.json @@ -16,6 +16,7 @@ "PasswordSettings": { "Pepper": "rVk/OwQUw01qy76Q+5WimPk+NdqUMMghftMXyJzzckOj/+eFn056PDYzBD61E/ZNjRdgiMK6RhcHEcdfpJdbcw==", "MaxLength": 128, - "MinLength": 22 + "MinLength": 22, + "BaseURL": "localhost:3000" } }