Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AES-CBC cipher works without providing an IV parameter. Is this correct? #44

Open
mikelbikkel opened this issue Nov 4, 2024 · 4 comments

Comments

@mikelbikkel
Copy link

Describe the bug
A cipher with MODE = CBC requires an IV.
When I create a cipher "AES/CBC/PKCS7PADDING" and I do NOT provide an IV parameter, the cipher works (encrypts an decrypts) without any warning or error. I would expect an error if the IV parameter is not provided.

To Reproduce
var
FCipher: IBufferedCipher;
FParams: ICipherParameters;
begin
FCipher := TCipherUtilities.getCipher('AES/CBC/PKCS7PADDING');
FParams := TParameterUtilities.CreateKeyParameter('AES', arKey); // arKey is a TBytes with the key.
FCipher.Init(True, FParams); // I would expect an exception at this point: MISSING IV.
FCipher.FCipher.ProcessBytes(.... etc. // Encryption succeeds without IV.
end;

The issue seems to be at this point:
procedure TCbcBlockCipher.Init(forEncryption: Boolean; const parameters: ICipherParameters);

if Supports(Lparameters, IParametersWithIV, ivParam) then
begin
iv := ivParam.GetIV();
if (System.Length(iv) <> FblockSize) then
begin
raise EArgumentCryptoLibException.CreateRes(@SInvalidIVLength);
end;
System.Move(iv[0], FIV[0], System.Length(iv) * System.SizeOf(Byte));
end;
// MY REMARK: there is no "else"-branch that handles the missing IV parameter.
// The cipher works without IV because FIV is a 16-byte zero-filled array. These zeros are used as IV.
Reset();

Expected behavior
if Supports(Lparameters, IParametersWithIV, ivParam) then
begin
iv := ivParam.GetIV();
if (System.Length(iv) <> FblockSize) then
begin
raise EArgumentCryptoLibException.CreateRes(@SInvalidIVLength);
end;
System.Move(iv[0], FIV[0], System.Length(iv) * System.SizeOf(Byte));
else
raise EArgumentCryptoLibException.CreateRes(@SMissingIV);

end;
Raise an exception if IV is missing
Reset();

Screenshots
NA

Environment (please complete the following information):

  • OS: [e.g. Windows, Linux, Mac] Windows
  • Compiler [e.g. FreePascal 3.0.0] Delphi 11 community edition
  • Package Version [e.g. 1.0] Not sure, I cloned the github repository on 21-08-2024

Additional context
Add any other context about the problem here.

@Xor-el
Copy link
Owner

Xor-el commented Nov 4, 2024

Hello @mikelbikkel
as you can see here and as you have rightly stated, the IV is zero initialized. while that isn't recommended, it isn't necessarily disallowed. if you wish to pass a custom IV, you can do this

KeyParametersWithIV := TParametersWithIV.Create(TParameterUtilities.CreateKeyParameter('AES', arKey), IVBytes);

@mikelbikkel
Copy link
Author

It's tricky in the current implementation.
Makes it a bit too easy to forget that certain modes require an IV. An explanation in the documentation comments would help.
Is tis codebase still actively maintained? I don't see activity in github for 5 years....
I m sure you put a lot of work in it, so it would be a pity if opportunities for improvements are not used.

Michel

@Xor-el
Copy link
Owner

Xor-el commented Nov 5, 2024

It's tricky in the current implementation. Makes it a bit too easy to forget that certain modes require an IV. An explanation in the documentation comments would help. Is tis codebase still actively maintained? I don't see activity in github for 5 years.... I m sure you put a lot of work in it, so it would be a pity if opportunities for improvements are not used.

Michel

@mikelbikkel Yes the code base is still maintained just that there hasn't been any new features added due to time constraints.
concerning the documentation, would you do the honours of creating a PR that documents this?

@mikelbikkel
Copy link
Author

Happy to help. It will be my first PR ever, so i need some time to prepare.
Let's get started

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants