Browse Source

minor security improvements

Ugochukwu Mmaduekwe 6 years ago
parent
commit
40620dbdc3

+ 4 - 0
CryptoLib/src/Crypto/Engines/ClpAesEngine.pas

@@ -30,6 +30,7 @@ uses
   ClpCheck,
   ClpBits,
   ClpConverters,
+  ClpArrayUtils,
   ClpCryptoLibTypes;
 
 resourcestring
@@ -499,6 +500,7 @@ begin
   keyLen := System.Length(key);
   if ((keyLen < 16) or (keyLen > 32) or ((keyLen and 7) <> 0)) then
   begin
+    TArrayUtils.ZeroFill(key);
     raise EArgumentCryptoLibException.CreateRes(@SInvalidKeyLength);
   end;
 
@@ -676,6 +678,7 @@ begin
       end
   else
     begin
+      TArrayUtils.ZeroFill(key);
       raise EInvalidOperationCryptoLibException.CreateRes(@SInvalidOperation);
     end;
   end;
@@ -696,6 +699,7 @@ begin
 
   result := bigW;
 
+  TArrayUtils.ZeroFill(key);
 end;
 
 function TAesEngine.GetAlgorithmName: String;

+ 4 - 0
CryptoLib/src/Crypto/Engines/ClpAesLightEngine.pas

@@ -30,6 +30,7 @@ uses
   ClpCheck,
   ClpBits,
   ClpConverters,
+  ClpArrayUtils,
   ClpCryptoLibTypes;
 
 resourcestring
@@ -425,6 +426,7 @@ begin
   keyLen := System.Length(key);
   if ((keyLen < 16) or (keyLen > 32) or ((keyLen and 7) <> 0)) then
   begin
+    TArrayUtils.ZeroFill(key);
     raise EArgumentCryptoLibException.CreateRes(@SInvalidKeyLength);
   end;
 
@@ -602,6 +604,7 @@ begin
       end
   else
     begin
+      TArrayUtils.ZeroFill(key);
       raise EInvalidOperationCryptoLibException.CreateRes(@SInvalidOperation);
     end;
   end;
@@ -622,6 +625,7 @@ begin
 
   result := bigW;
 
+  TArrayUtils.ZeroFill(key);
 end;
 
 function TAesLightEngine.GetAlgorithmName: String;

+ 4 - 0
CryptoLib/src/Crypto/Engines/ClpBlowfishEngine.pas

@@ -29,6 +29,7 @@ uses
   ClpIBlowfishEngine,
   ClpCheck,
   ClpConverters,
+  ClpArrayUtils,
   ClpCryptoLibTypes;
 
 resourcestring
@@ -326,6 +327,7 @@ begin
   if ((keyLength < 4) or (keyLength > 56) or (((keyLength * 8) and 7) <> 0))
   then
   begin
+    TArrayUtils.ZeroFill(key);
     raise EArgumentCryptoLibException.CreateRes(@SInvalidKeyLength);
   end;
 
@@ -410,6 +412,8 @@ begin
   ProcessTable(FS0[SBOX_SK - 2], FS0[SBOX_SK - 1], FS1);
   ProcessTable(FS1[SBOX_SK - 2], FS1[SBOX_SK - 1], FS2);
   ProcessTable(FS2[SBOX_SK - 2], FS2[SBOX_SK - 1], FS3);
+
+  TArrayUtils.ZeroFill(key);
 end;
 
 procedure TBlowfishEngine.EncryptBlock(const src: TCryptoLibByteArray;

+ 6 - 0
CryptoLib/src/Crypto/Engines/ClpChaChaEngine.pas

@@ -27,6 +27,7 @@ uses
   ClpIChaChaEngine,
   ClpSalsa20Engine,
   ClpConverters,
+  ClpArrayUtils,
   ClpCryptoLibTypes;
 
 type
@@ -242,6 +243,8 @@ begin
   begin
     if not(Byte(System.Length(keyBytes)) in [16, 32]) then
     begin
+      TArrayUtils.ZeroFill(keyBytes);
+      TArrayUtils.ZeroFill(ivBytes);
       raise EArgumentCryptoLibException.CreateResFmt(@SInvalidKeySize,
         [AlgorithmName]);
     end;
@@ -259,6 +262,9 @@ begin
   // IV
   TConverters.le32_copy(PByte(ivBytes), 0, PCardinal(FEngineState),
     14 * System.SizeOf(UInt32), 2 * System.SizeOf(UInt32));
+
+  TArrayUtils.ZeroFill(keyBytes);
+  TArrayUtils.ZeroFill(ivBytes);
 end;
 
 end.

+ 4 - 0
CryptoLib/src/Crypto/Engines/ClpRijndaelEngine.pas

@@ -28,6 +28,7 @@ uses
   ClpIRijndaelEngine,
   ClpIKeyParameter,
   ClpICipherParameters,
+  ClpArrayUtils,
   ClpCryptoLibTypes;
 
 resourcestring
@@ -546,6 +547,7 @@ begin
       KC := 8
   else
     begin
+      TArrayUtils.ZeroFill(key);
       raise EArgumentCryptoLibException.CreateRes(@SInvalidKeyLength);
     end;
   end;
@@ -654,6 +656,8 @@ begin
     end;
   end;
   result := W;
+
+  TArrayUtils.ZeroFill(key);
 end;
 
 procedure TRijndaelEngine.PackBlock(const bytes: TCryptoLibByteArray;

+ 11 - 0
CryptoLib/src/Crypto/Engines/ClpSalsa20Engine.pas

@@ -31,6 +31,7 @@ uses
   ClpICipherParameters,
   ClpIParametersWithIV,
   ClpConverters,
+  ClpArrayUtils,
   ClpCryptoLibTypes;
 
 resourcestring
@@ -215,6 +216,7 @@ begin
   iv := ivParams.GetIV();
   if ((iv = Nil) or (System.Length(iv) <> NonceSize)) then
   begin
+    TArrayUtils.ZeroFill(iv);
     raise EArgumentCryptoLibException.CreateResFmt(@SInvalidIV,
       [AlgorithmName, NonceSize]);
   end;
@@ -476,6 +478,8 @@ begin
   begin
     if not(Byte(System.Length(keyBytes)) in [16, 32]) then
     begin
+      TArrayUtils.ZeroFill(keyBytes);
+      TArrayUtils.ZeroFill(ivBytes);
       raise EArgumentCryptoLibException.CreateResFmt(@SInvalidKeySize,
         [AlgorithmName]);
     end;
@@ -497,6 +501,13 @@ begin
   // IV
   TConverters.le32_copy(PByte(ivBytes), 0, PCardinal(FEngineState),
     6 * System.SizeOf(UInt32), 2 * System.SizeOf(UInt32));
+
+  if (Self.ClassType = TSalsa20Engine) then
+  begin
+    TArrayUtils.ZeroFill(keyBytes);
+    TArrayUtils.ZeroFill(ivBytes);
+  end;
+
 end;
 
 end.

+ 10 - 5
CryptoLib/src/Crypto/Engines/ClpSpeckEngine.pas

@@ -28,6 +28,7 @@ uses
   ClpIBlockCipher,
   ClpICipherParameters,
   ClpIKeyParameter,
+  ClpArrayUtils,
   ClpCryptoLibTypes;
 
 resourcestring
@@ -601,7 +602,12 @@ procedure TSpeckEngine.EngineInit(forEncryption: Boolean;
   const keyBytes: TCryptoLibByteArray);
 begin
   FforEncryption := forEncryption;
-  CheckKeySize(System.Length(keyBytes));
+  // ensure we clear "Key" from memory in case of exceptions when checking KeyLength
+  try
+    CheckKeySize(System.Length(keyBytes));
+  except
+    TArrayUtils.ZeroFill(keyBytes);
+  end;
   SetKey(keyBytes);
   Finitialised := true;
 end;
@@ -620,7 +626,6 @@ procedure TSpeckEngine.Init(forEncryption: Boolean;
   const parameters: ICipherParameters);
 var
   keyParameter: IKeyParameter;
-  keyBytes: TCryptoLibByteArray;
 begin
 
   if not Supports(parameters, IKeyParameter, keyParameter) then
@@ -628,8 +633,7 @@ begin
     raise EArgumentCryptoLibException.CreateResFmt(@SInvalidParameterSpeckInit,
       [(parameters as TObject).ToString]);
   end;
-  keyBytes := keyParameter.GetKey;
-  EngineInit(forEncryption, keyBytes);
+  EngineInit(forEncryption, keyParameter.GetKey());
 end;
 
 function TSpeckEngine.ProcessBlock(const input: TCryptoLibByteArray;
@@ -817,6 +821,7 @@ begin
 
   end;
 
+  TArrayUtils.ZeroFill(keyBytes);
 end;
 
 { TSpeckUInt64Engine }
@@ -980,6 +985,7 @@ begin
 
   end;
 
+  TArrayUtils.ZeroFill(keyBytes);
 end;
 
 { TSpeck32Engine }
@@ -1088,4 +1094,3 @@ begin
 end;
 
 end.
-

+ 10 - 4
CryptoLib/src/Crypto/Engines/ClpSpeckLegacyEngine.pas

@@ -28,6 +28,7 @@ uses
   ClpIBlockCipher,
   ClpICipherParameters,
   ClpIKeyParameter,
+  ClpArrayUtils,
   ClpCryptoLibTypes;
 
 resourcestring
@@ -598,7 +599,12 @@ procedure TSpeckLegacyEngine.EngineInit(forEncryption: Boolean;
   const keyBytes: TCryptoLibByteArray);
 begin
   FforEncryption := forEncryption;
-  CheckKeySize(System.Length(keyBytes));
+  // ensure we clear "Key" from memory in case of exceptions when checking KeyLength
+  try
+    CheckKeySize(System.Length(keyBytes));
+  except
+    TArrayUtils.ZeroFill(keyBytes);
+  end;
   SetKey(keyBytes);
   Finitialised := true;
 end;
@@ -617,7 +623,6 @@ procedure TSpeckLegacyEngine.Init(forEncryption: Boolean;
   const parameters: ICipherParameters);
 var
   keyParameter: IKeyParameter;
-  keyBytes: TCryptoLibByteArray;
 begin
 
   if not Supports(parameters, IKeyParameter, keyParameter) then
@@ -625,8 +630,7 @@ begin
     raise EArgumentCryptoLibException.CreateResFmt
       (@SInvalidParameterSpeckLegacyInit, [(parameters as TObject).ToString]);
   end;
-  keyBytes := keyParameter.GetKey;
-  EngineInit(forEncryption, keyBytes);
+  EngineInit(forEncryption, keyParameter.GetKey());
 end;
 
 function TSpeckLegacyEngine.ProcessBlock(const input: TCryptoLibByteArray;
@@ -817,6 +821,7 @@ begin
 
   end;
 
+  TArrayUtils.ZeroFill(keyBytes);
 end;
 
 { TSpeckUInt64LegacyEngine }
@@ -983,6 +988,7 @@ begin
 
   end;
 
+  TArrayUtils.ZeroFill(keyBytes);
 end;
 
 { TSpeck32LegacyEngine }

+ 6 - 0
CryptoLib/src/Crypto/Engines/ClpXSalsa20Engine.pas

@@ -26,6 +26,7 @@ uses
   ClpSalsa20Engine,
   ClpIXSalsa20Engine,
   ClpConverters,
+  ClpArrayUtils,
   ClpCryptoLibTypes;
 
 resourcestring
@@ -79,6 +80,8 @@ begin
 
   if (System.Length(keyBytes) <> 32) then
   begin
+    TArrayUtils.ZeroFill(keyBytes);
+    TArrayUtils.ZeroFill(ivBytes);
     raise EArgumentCryptoLibException.CreateResFmt(@SInvalidKeySize,
       [AlgorithmName]);
   end;
@@ -110,6 +113,9 @@ begin
   TConverters.le32_copy(PByte(ivBytes), 16 * System.SizeOf(Byte),
     PCardinal(FEngineState), 6 * System.SizeOf(UInt32),
     2 * System.SizeOf(UInt32));
+
+  TArrayUtils.ZeroFill(keyBytes);
+  TArrayUtils.ZeroFill(ivBytes);
 end;
 
 end.