You are not logged in.
Pages: 1
Hi,
I want to use SynCrypto's AESCBC.
I wanted to use EncryptPKCS7 function but it takes RawByteString argument (like some other func of SynCrypto).
My input data is an array of byte. So to avoid several copy when converting the array of byte into and from the RawByteString (I think it is the only clean solution?), I reimplemented the EncryptPKCS7/DecryptPKCS7 with array of byte.
So my question is: why [En|De]cryptPKCS7 uses RawByteString rather than array of byte? Am I missing something :-) ?
Here is the reimplemented funcs ( :
TCryptoMormot = class
private
FAES : TAESCBC
...
function TCryptoMormot.EncryptPKCS7(
const AInput: TBytes;
AIVAtBeginning: boolean): TBytes;
var len, padding, iv: cardinal;
P: Pointer;
LIV: TAESBlock;
begin
if FAES = nil then
Raise Exception.Create(RS_AES_NIL);
// use PKCS7 padding, so expects AESBlockSize=16 bytes blocks
len := length(AInput);
padding := AESBlockSize - (len and (AESBlockSize-1));
if AIVAtBeginning then
iv := AESBlockSize else
iv := 0;
SetLength(result, iv + len + padding);
if AIVAtBeginning then begin
FillRandom(LIV);
FAES.IV := LIV;
PAESBlock(result)^ := FAES.IV;
end;
move(Pointer(AInput)^, PByteArray(result)^[iv], len);
FillChar(PByteArray(result)^[iv+len], padding, padding);
// encryption
P := @PByteArray(result)^[iv];
FAES.Encrypt(P, P, len+padding);
end;
function TCryptoMormot.DecryptPKCS7(
const AInput: TBytes;
AIVAtBeginning: boolean): TBytes;
var len,iv: integer;
begin
if FAES = nil then
Raise Exception.Create(RS_AES_NIL);
// validate input
len := length(AInput);
if (len < AESBlockSize) or (len and (AESBlockSize-1) <> 0) then
raise ESynCrypto.Create('Invalid content');
// decrypt
if AIVAtBeginning then begin
FAES.IV := PAESBlock(AInput)^;
dec(len, AESBlockSize);
iv := AESBlockSize;
end else
iv := 0;
SetLength(result, len);
FAES.Decrypt(@PByteArray(AInput)^[iv], result, len);
// delete right padding
if ord(result[len - 1]) > AESBlockSize then
raise ESynCrypto.Create('Invalid content');
SetLength(result, len - ord(result[len - 1]));
end;
Offline
TBytes are dynamic array of bytes so:
- they would first initialize the buffer with 0, even if it is not necessary since most of the time we will just fill them with data;
- they won't use proper COW like RawByteString when you change an item, just basic COW for the variable reference;
- some part of the RTL is not even thread safe (reference counting is not using atomic operations), whereas RawByteString is safer.
So TBytes are slightly slower, and may be less reliable than RawByteString in some use cases.
Those are one of the reasons why we rely on them, even for BLOB storage.
I've added RawByteStringToBytes() and BytesToRawByteString() functions.
See http://synopse.info/fossil/info/5f657445d7
Offline
Ok, thank you for the precisions, I am a beginner with Pascal so I don't know what is best to use.
There are still some things that tickles me:
- If EncryptPKCS7 uses RawByteString, to avoid the copy and be faster than TBytes, this force the user to use RawByteString from the beginning, which can be cumbersome.
- Actually, like in the SynCrypto TAESAbstract.Encrypt procedure, I should have use buffer instead of TBytes. And implement sthg like this :
procedure EncryptPKCS7(BufIn, BufOut: pointer; SizeIn: cardinal, out SizeOut: cardinal; AIVAtBeginning: boolean);
- RawByteString has been deprecated by delphi for mobile app
Offline
Indeed.
I've added overloaded TAESAbstract.EncryptPKCS7 and TAESAbstract.DecryptPKCS7 methods, using TBytes as input/output kind of parameters.
See http://synopse.info/fossil/info/cd3c7ac4c7
Also included EncryptPKC7Buffer() method, if needed.
For the "decrypt" version, since the output buffer would be resized after process (i.e. remove padding data), I did not implement a method yet.
About RawByteString deprecation, it is IMHO a real PITA.
Something which I like not at all, even if you can re-enable its use, using Andy's low level patch.
This is one the reasons why we look at FPC compatibility more than NextGen support - even if we provide NextGen access, but for the clients only.
Offline
Great, your implementing my needs on the fly :-) !
> Also included EncryptPKC7Buffer() method, if needed.
> For the "decrypt" version, since the output buffer would be resized after process (i.e. remove padding data),
> I did not implement a method yet.
For the DecryptPKCS7Buffer func (and also for the EncryptPKCS7Buffer func) you could alloc the output buffer in the func and gives the ownership to the caller. It is not a big deal if the remaining padding bytes are unused, actually, the implementation with RawBytesString and TBytes does the same, isn't?
I think there is a bug in your commit (I ran into the same when implementing with tbytes). The padding size must be get using len-1 rather than len as tbytes start from 0 whereas RawByteString start from 1.
function TAESAbstract.DecryptPKCS7(const Input: TBytes;
IVAtBeginning: boolean): TBytes;
var len,iv: integer;
begin
// validate input
len := length(Input);
if (len<AESBlockSize) or (len and (AESBlockSize-1)<>0) then
raise ESynCrypto.Create('Invalid content');
// decrypt
if IVAtBeginning then begin
fIV := PAESBlock(Input)^;
dec(len,AESBlockSize);
iv := AESBlockSize;
end else
iv := 0;
SetLength(result,len);
Decrypt(@PByteArray(Input)^[iv],pointer(result),len);
// delete right padding
if ord(result[len])>AESBlockSize then
raise ESynCrypto.Create('Invalid content');
SetLength(result,len-result[len]);
end;
fix :
if ord(result[len-1]) > AESBlockSize then
raise ESynCrypto.Create('Invalid content');
SetLength(result,len-ord(result[len-1]));
Last edited by souchaud (2015-05-06 09:43:11)
Offline
My mistake!
I've tried to fix - and also enhance the latest modifications.
See http://synopse.info/fossil/info/b8381f31f0
Thanks for your help!
Offline
Ok,
Actually, I think too that it is better if the caller of EncryptPKC7Buffer alloc the output buffer.
I 'll try to implement the DecryptPKC7Buffer like this when I'll have time.
function TAESAbstract.DecryptPKCS7Buffer(Input,Output: Pointer; InputLen,OutputLen: cardinal;
IVAtBeginning: boolean): Cardinal; // return the real output buffer len
But yes, I think too that an api like this is a bit confusing for the user (OutputLen param > returned output buffer len).
Offline
If your purpose is to use TBytes, just use the new method.
If your purpose is to work with binary buffers, use the pointer method.
I confirm that such an API may be confusing...
In practice, Setlength() is immediate, thanks to how FastMM4 handles reallocation of a memory block.
So there is no speed penalty with the current implementation.
As a result, I would not mind using this TBytes version. There would be no speed gain.
Offline
I need to use binary buffer (at first I though TBytes was binary buffer but it is not).
I'll use the TBytes method for DecryptPKCS7, as the copy from TBytes to the caller buffer is negligible.
Anyway, this made me improve in pascal primitive type understanding.
Offline
Thanks for your time.
Offline
Pages: 1