Skip to content

Commit 681b39b

Browse files
committed
Introduces MinSaltLength class function and checked in BCrypt's Calc that the proper salt length is specified. Implemented further unit tests.
1 parent 77e52c4 commit 681b39b

File tree

3 files changed

+89
-21
lines changed

3 files changed

+89
-21
lines changed

Source/DECHash.pas

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,11 +1226,14 @@ THash_BCrypt = class(TDECPasswordHash)
12261226
/// </summary>
12271227
constructor Create; override;
12281228
/// <summary>
1229-
/// Returns the maximum supported length of the salt value
1230-
/// in byte
1229+
/// Returns the maximum supported length of the salt value in byte
12311230
/// </summary>
12321231
class function MaxSaltLength:UInt8; override;
12331232
/// <summary>
1233+
/// Returns the minimum supported length of the salt value in byte
1234+
/// </summary>
1235+
class function MinSaltLength:UInt8; override;
1236+
/// <summary>
12341237
/// Returns the maximum length of a user supplied password given for the
12351238
/// algorithm in byte
12361239
/// </summary>
@@ -1334,7 +1337,8 @@ implementation
13341337
sSHA3AbsorbFailure = 'Absorb: number of bits mod 8 <> 0 or squeezing active. '+
13351338
'Bits: %0:d, Squeezing: %1:s';
13361339
/// <summary>
1337-
/// Part of the failure message shown when setting HashSize of Shake algorithms to 0
1340+
/// Part of the failure message shown when setting HashSize of Shake
1341+
/// algorithms to 0.
13381342
/// </summary>
13391343
sHashOutputLength0 = 'HashSize must not be 0';
13401344
/// <summary>
@@ -1347,6 +1351,12 @@ implementation
13471351
/// Exception message for password hashes when a too long password is specified
13481352
/// </summary>
13491353
sPasswordTooLong = 'Password to be hashed is too long. Max. length: %0:d bytes';
1354+
/// <summary>
1355+
/// Exception message for password hashes requiring a salt when a salt value
1356+
/// which is either too short or too long has been specified
1357+
/// </summary>
1358+
sWrongSaltLength = 'Length of specified salt value must be between %0:d '+
1359+
'and %1:d bytes';
13501360

13511361
{ THash_MD2 }
13521362

@@ -5034,6 +5044,12 @@ procedure THash_BCrypt.Calc(const Data; DataSize: Integer);
50345044
if (DataSize > MaxPasswordLength) then
50355045
raise EDECHashException.CreateFmt(sPasswordTooLong, [MaxPasswordLength]);
50365046

5047+
// While this should normally be caught on setting salt already it is there
5048+
// especially to catch cases where no salt has been specified yet.
5049+
if (Length(FSalt) < MinSaltLength) or (Length(FSalt) > MaxSaltLength) then
5050+
raise EDECHashException.CreateFmt(sWrongSaltLength,
5051+
[MinSaltLength, MaxSaltLength]);
5052+
50375053
// This automatically "adds" the required #0 terminator at the end of the password
50385054
SetLength(PwdData, DataSize + 1);
50395055
Move(Data, PwdData[0], DataSize);
@@ -5301,6 +5317,11 @@ class function THash_BCrypt.MinCost: UInt8;
53015317
Result := 4;
53025318
end;
53035319

5320+
class function THash_BCrypt.MinSaltLength: UInt8;
5321+
begin
5322+
Result := 16;
5323+
end;
5324+
53045325
procedure THash_BCrypt.SetCost(const Value: UInt32);
53055326
begin
53065327
if (Value in [MinCost..MaxCost]) then

Source/DECHashAuthentication.pas

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ TDECPasswordHash = class(TDECHashAuthentication)
631631
function GetSalt: TBytes;
632632
strict protected
633633
/// <summary>
634-
/// Most if not all password hashing algorithms (like bcrypt) have a salt
634+
/// Most, if not all password hashing algorithms (like bcrypt) have a salt
635635
/// parameter to modify the entered password value.
636636
/// </summary>
637637
FSalt : TBytes;
@@ -718,6 +718,11 @@ TDECPasswordHash = class(TDECHashAuthentication)
718718
/// </summary>
719719
class function MaxSaltLength:UInt8; virtual; abstract;
720720
/// <summary>
721+
/// Returns the minimum length of a salt value given for the algorithm
722+
/// in byte
723+
/// </summary>
724+
class function MinSaltLength:UInt8; virtual; abstract;
725+
/// <summary>
721726
/// Returns the maximum length of a user supplied password given for the
722727
/// algorithm in byte
723728
/// </summary>
@@ -826,6 +831,10 @@ implementation
826831
/// </summary>
827832
sSaltValueTooLong = 'Maximum allowed salt length (%0:d byte) exceeded';
828833
/// <summary>
834+
/// Exception message when specifying a salt value shorter than allowed
835+
/// </summary>
836+
sSaltValueTooShort = 'Minumum allowed salt length (%0:d byte) exceeded';
837+
/// <summary>
829838
/// No class for the given crypt ID has been registered, so that ID is
830839
/// not supported.
831840
/// </summary>
@@ -1408,6 +1417,9 @@ procedure TDECPasswordHash.SetSalt(const Value: TBytes);
14081417
if (Length(Value) > MaxSaltLength) then
14091418
raise EDECHashException.CreateFmt(sSaltValueTooLong, [MaxSaltLength]);
14101419

1420+
if (Length(Value) < MinSaltLength) then
1421+
raise EDECHashException.CreateFmt(sSaltValueTooShort, [MinSaltLength]);
1422+
14111423
FSalt := Value;
14121424
end;
14131425

Unit Tests/Tests/TestDECHash.pas

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,14 @@ THash_TestTDECPasswordHash = class(TTestCase)
118118
procedure SetUp; override;
119119
procedure TearDown; override;
120120
protected
121-
// procedure DoTestSalt0Exception;
122121
procedure DoTestSaltTooLongException;
122+
procedure DoTestSaltTooShortException;
123123
procedure DoTestClassByCryptIdentityException;
124124
published
125125
procedure TestGetSalt;
126126
procedure TestSetSalt;
127-
// procedure TestSalt0Exception;
128127
procedure TestSaltTooLongException;
128+
procedure TestSaltTooShortException;
129129
procedure TestClassByCryptIdentitySuccess;
130130
procedure TestClassByCryptIdentityException;
131131
end;
@@ -677,20 +677,23 @@ TBCryptBSDTestData = record
677677
procedure SetUp; override;
678678
procedure DoTestCostFactorTooShortException;
679679
procedure DoTestCostFactorTooLongException;
680+
procedure DoTestNoSaltSpecified;
680681
published
681682
procedure TestDigestSize;
682683
procedure TestBlockSize;
683684
procedure TestIsPasswordHash;
684685
procedure TestClassByName;
685686
procedure TestIdentity;
686687
procedure TestMaximumSaltLength;
688+
procedure TestMinimumSaltLength;
687689
procedure TestMaximumPasswordLength;
688690
procedure TestMinCost;
689691
procedure TestMaxCost;
690692
procedure TestCostFactorTooShortException;
691693
procedure TestCostFactorTooLongException;
692694
procedure TestSetGetCostFactor;
693695
procedure TestCryptBSDFormat;
696+
procedure TestNoSaltSpecified;
694697
// procedure TestTooLongPasswordException;
695698
end;
696699

@@ -6055,6 +6058,20 @@ procedure TestTHash_BCrypt.DoTestCostFactorTooShortException;
60556058
THash_BCrypt(FHash).Cost := 3;
60566059
end;
60576060

6061+
procedure TestTHash_BCrypt.DoTestNoSaltSpecified;
6062+
var
6063+
BCrypt : THash_BCrypt;
6064+
begin
6065+
BCrypt := THash_BCrypt.Create;
6066+
try
6067+
BCrypt.Init;
6068+
BCrypt.Cost := 8;
6069+
BCrypt.CalcString('a');
6070+
finally
6071+
BCrypt.Free;
6072+
end;
6073+
end;
6074+
60586075
procedure TestTHash_BCrypt.SetUp;
60596076
var
60606077
lDataRow:IHashTestDataRowSetup;
@@ -6326,6 +6343,16 @@ procedure TestTHash_BCrypt.TestMinCost;
63266343
CheckEquals(4, THash_BCrypt(FHash).MinCost);
63276344
end;
63286345

6346+
procedure TestTHash_BCrypt.TestMinimumSaltLength;
6347+
begin
6348+
CheckEquals(16, TDECPasswordHash(FHash).MinSaltLength);
6349+
end;
6350+
6351+
procedure TestTHash_BCrypt.TestNoSaltSpecified;
6352+
begin
6353+
CheckException(DoTestNoSaltSpecified, EDECHashException);
6354+
end;
6355+
63296356
procedure TestTHash_BCrypt.TestSetGetCostFactor;
63306357
begin
63316358
THash_BCrypt(FHash).Cost := 4;
@@ -6337,14 +6364,6 @@ procedure TestTHash_BCrypt.TestSetGetCostFactor;
63376364

63386365
{ THash_TestTDECPasswordHash }
63396366

6340-
//procedure THash_TestTDECPasswordHash.DoTestSalt0Exception;
6341-
//var
6342-
// EmptySalt : TBytes;
6343-
//begin
6344-
// SetLength(EmptySalt, 0);
6345-
// FHash.Salt := EmptySalt;
6346-
//end;
6347-
63486367
procedure THash_TestTDECPasswordHash.DoTestClassByCryptIdentityException;
63496368
begin
63506369
TDECPasswordHash.ClassByCryptIdentity('nwrongID');
@@ -6359,6 +6378,14 @@ procedure THash_TestTDECPasswordHash.DoTestSaltTooLongException;
63596378
FHash.Salt := EmptySalt;
63606379
end;
63616380

6381+
procedure THash_TestTDECPasswordHash.DoTestSaltTooShortException;
6382+
var
6383+
EmptySalt : TBytes;
6384+
begin
6385+
SetLength(EmptySalt, FHash.MinSaltLength - 1);
6386+
6387+
FHash.Salt := EmptySalt;
6388+
end;
63626389

63636390
procedure THash_TestTDECPasswordHash.SetUp;
63646391
begin
@@ -6403,18 +6430,13 @@ procedure THash_TestTDECPasswordHash.TestGetSalt;
64036430
ActSalt := FHash.Salt;
64046431
CheckEquals(0, Length(ActSalt));
64056432

6406-
SetSalt := [1, 2, 3, 4];
6433+
SetSalt := [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16];
64076434
FHash.Salt := SetSalt;
64086435

64096436
ActSalt := FHash.Salt;
64106437
CheckEquals(true, System.SysUtils.CompareMem(@SetSalt[0], @ActSalt[0], Length(ActSalt)));
64116438
end;
64126439

6413-
//procedure THash_TestTDECPasswordHash.TestSalt0Exception;
6414-
//begin
6415-
// CheckException(DoTestSalt0Exception, EDECHashException);
6416-
//end;
6417-
64186440
procedure THash_TestTDECPasswordHash.TestSaltTooLongException;
64196441
var
64206442
ActSalt : TBytes;
@@ -6428,11 +6450,24 @@ procedure THash_TestTDECPasswordHash.TestSaltTooLongException;
64286450
CheckEquals(0, Length(ActSalt));
64296451
end;
64306452

6453+
procedure THash_TestTDECPasswordHash.TestSaltTooShortException;
6454+
var
6455+
ActSalt : TBytes;
6456+
begin
6457+
ActSalt := FHash.Salt;
6458+
CheckEquals(0, Length(ActSalt));
6459+
6460+
CheckException(DoTestSaltTooShortException, EDECHashException);
6461+
6462+
ActSalt := FHash.Salt;
6463+
CheckEquals(0, Length(ActSalt));
6464+
end;
6465+
64316466
procedure THash_TestTDECPasswordHash.TestSetSalt;
64326467
var
64336468
SetSalt, ActSalt : TBytes;
64346469
begin
6435-
SetSalt := [1, 2, 3, 4];
6470+
SetSalt := [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16];
64366471
FHash.Salt := SetSalt;
64376472

64386473
ActSalt := FHash.Salt;

0 commit comments

Comments
 (0)