Recent

Author Topic: BlowFish busted in Laz 1.9 / Fpc 3.1.1  (Read 14009 times)

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #30 on: January 05, 2018, 04:44:56 am »
What text do you get for TEncoding.Default.EncodingName?

RedOctober

  • Sr. Member
  • ****
  • Posts: 452
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #31 on: January 05, 2018, 04:54:16 am »
windows-1252.

If I do a TEncodings.FreeEncodings, the EncodeStringBase64 throws a memory access error.

How do I set TEncodings to default to seUTF8?


engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #32 on: January 05, 2018, 05:01:56 am »
Here is my test based on some code in this thread, I used fp.exe:
Code: Pascal  [Select][+][-]
  1. program Project1;
  2.  
  3. {$mode objfpc}{$H+}
  4. uses
  5.   LazUTF8, classes, BlowFish, base64, sysutils
  6.   ;
  7.  
  8. procedure ShowMessage(s: string);
  9. begin
  10.   WriteLn(s);
  11. end;
  12.  
  13. procedure Button2Click(Sender: TObject);
  14. var
  15.   mime_str_in, mime_str_out: String;
  16.   plain_text, master_pwd: String;
  17.  
  18.   enc_str, dec_str: RawByteString;
  19.   bfsh_enc_strm: TBlowFishEncryptStream;
  20.   bfsh_dec_strm: TBlowFishDecryptStream;
  21.   strm: TStringStream;
  22.   e: TEncoding;
  23.   L: Longint;
  24.  
  25. begin
  26.   TEncoding.FreeEncodings;
  27.   e := TMBCSEncoding.Create(DefaultSystemCodePage); //<--- needs to be freed *** FREEDOM ***
  28.   WriteLn('DefaultSystemCodePage: ', DefaultSystemCodePage);
  29.   BytesOf(plain_text);
  30.   WriteLn('TEncoding.Default: ',TEncoding.Default.EncodingName);
  31.   master_pwd := '12345678';
  32.   plain_text := 'MyPlainText';
  33.   ShowMessage('plain_text: ' + plain_text);
  34.  
  35.   //--- Encrypt first
  36.   strm := TStringStream.Create('');
  37.   bfsh_enc_strm := TBlowfishEncryptStream.Create(master_pwd, strm);
  38.   bfsh_enc_strm.WriteAnsiString(plain_text);
  39.   bfsh_enc_strm.Flush;
  40.   enc_str := strm.DataString;
  41.   WriteLn('StringCodePage(enc_str): ',StringCodePage(enc_str));
  42.   WriteLn(StringCodePage('Encrypted String: ' + enc_str));//TEncoding
  43.  
  44.   ShowMessage('Encrypted String: ' + enc_str);//TEncoding
  45.  
  46.   // Now MIME encode it
  47.   mime_str_in := EncodeStringBase64(enc_str);
  48.   ShowMessage('mime_text: ' + mime_str_in);
  49.  
  50.   bfsh_enc_strm.Free;
  51.   strm.Free;
  52.   // Encryption/Encoding completed
  53.  
  54.   // Decode MIME first, before we decrypt
  55.   dec_str := DecodeStringBase64(PChar(mime_str_in{.ToCharArray}), False);
  56.  
  57.   ShowMessage('dec_str: ' + dec_str);
  58.  
  59.   if SameText(enc_str, dec_str) then
  60.     ShowMessage('enc_str and dec_str match')
  61.   else
  62.     ShowMessage('enc_str and dec_str DO NOT match');
  63.  
  64.   // Now, Decrypt
  65.   strm := TStringStream.Create(dec_str);
  66.   bfsh_dec_strm := TBlowfishDecryptStream.Create(master_pwd, strm);
  67.   dec_str := bfsh_dec_strm.ReadAnsiString;
  68.  
  69.   ShowMessage('dec_str: ' + dec_str);
  70.  
  71.   if SameText(plain_text, dec_str) then
  72.     ShowMessage('enc_str and dec_str match')
  73.   else
  74.     ShowMessage('enc_str and dec_str DO NOT match');
  75.  
  76.   bfsh_dec_strm.Free;
  77.   strm.Free;
  78. end;
  79.  
  80. begin
  81.   Button2Click(nil);
  82. end.

Its output using trunk:
Quote
DefaultSystemCodePage: 65001
TEncoding.Default: utf-8
plain_text: MyPlainText
StringCodePage(enc_str): 65001
65001
Encrypted String: b#Jod|♫QK
mime_text: YrUj6+yvSm/gZHySDtlRSw==
dec_str: b#Jod|♫QK
enc_str and dec_str match
dec_str: MyPlainText
enc_str and dec_str match
« Last Edit: January 05, 2018, 05:06:09 am by engkin »

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #33 on: January 05, 2018, 05:17:09 am »
windows-1252.

If I do a TEncodings.FreeEncodings, the EncodeStringBase64 throws a memory access error.

How do I set TEncodings to default to seUTF8?
No GetDefault calls GetANSI which returns FStandardEncodings[seAnsi] if already created. or creates FStandardEncodings[seAnsi] using DefaultSystemCodePage.

Since you received windows-1252 that means FStandardEncodings[seAnsi] either already created windows-1252 or your DefaultSystemCodePage is not 65001. What value do you have for DefaultSystemCodePage?

The question is how to free and nil FStandardEncodings[seAnsi] properly so it gets created based on DefaultSystemCodePage value.

Now we have in public:
Code: Pascal  [Select][+][-]
  1. class procedure TEncoding.FreeEncodings;
  2. var
  3.   E: TStandardEncoding;
  4. begin
  5. {$ifdef FPC_HAS_FEATURE_THREADING}
  6.   EnterCriticalSection(FLock);
  7.   try
  8. {$endif}
  9.     for E := Low(FStandardEncodings) to High(FStandardEncodings) do
  10.       FStandardEncodings[E].Free;
  11. {$ifdef FPC_HAS_FEATURE_THREADING}
  12.   finally
  13.     LeaveCriticalSection(FLock);
  14.   end;
  15. {$endif}
  16. end;

and in strict private:

Code: Pascal  [Select][+][-]
  1. class constructor TEncoding.Create;
  2. var
  3.   E: TStandardEncoding;
  4. begin
  5.   for E := Low(FStandardEncodings) to High(FStandardEncodings) do
  6.     FStandardEncodings[E] := nil;
  7. {$ifdef FPC_HAS_FEATURE_THREADING}
  8.   InitCriticalSection(FLock);
  9. {$endif}
  10. end;

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #34 on: January 05, 2018, 05:36:51 am »
Most likely TEncoding.FreeEncodings needs a little change:
Code: Pascal  [Select][+][-]
  1. class procedure TEncoding.FreeEncodings;
  2. var
  3.   E: TStandardEncoding;
  4. begin
  5. {$ifdef FPC_HAS_FEATURE_THREADING}
  6.   EnterCriticalSection(FLock);
  7.   try
  8. {$endif}
  9.     for E := Low(FStandardEncodings) to High(FStandardEncodings) do
  10.       if assigned(FStandardEncodings[E]) then
  11.       begin
  12.         FStandardEncodings[E].Free;
  13.         FStandardEncodings[E] := nil;
  14.       end;
  15. {$ifdef FPC_HAS_FEATURE_THREADING}
  16.   finally
  17.     LeaveCriticalSection(FLock);
  18.   end;
  19. {$endif}
  20. end;

Ok, enough talking to myself.

RedOctober

  • Sr. Member
  • ****
  • Posts: 452
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #35 on: January 05, 2018, 06:09:09 am »
Hi engkin.  The encoding/decoding works now.  Thank you very much for that.

I noticed that I cannot e.Free.  If I do, my app will throw a memory access error.  I suspect that the .Create line assigned ownership to an object that is already freeing it somewhere, so I should not .Free it on my one.  Just create it and leave it alone.

Is that true?  Or should I e.Free somewhere? If so, where?  I keep getting memory access errors if I e.free.

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #36 on: January 07, 2018, 05:10:43 pm »
I noticed that I cannot e.Free.  If I do, my app will throw a memory access error.  I suspect that the .Create line assigned ownership to an object that is already freeing it somewhere, so I should not .Free it on my one.  Just create it and leave it alone.
This was meant to prove that the bug is related to TEncoding and how its Default property is different from DefaultSystemCodePage. In my opinion this bug is related to Lazarus, because it changes the value of DefaultSystemCodePage and does not correct TEncoding in accordance.

If Thaddy and Molly agree with me then a bug report should be filed.

Is that true?  Or should I e.Free somewhere? If so, where?  I keep getting memory access errors if I e.free.
I am puzzled by this as well, but for now instead of:
Code: Pascal  [Select][+][-]
  1.   TEncoding.FreeEncodings;
  2.   e := TMBCSEncoding.Create(DefaultSystemCodePage);

I suggest you do:
Code: Pascal  [Select][+][-]
  1.   TEncoding.FreeEncodings;
  2.   e := TEncoding.GetEncoding(DefaultSystemCodePage);

and to free it:
Code: Pascal  [Select][+][-]
  1.   if not TEncoding.IsStandardEncoding(e) then
  2.     e.Free;

Thaddy

  • Hero Member
  • *****
  • Posts: 14205
  • Probably until I exterminate Putin.
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #37 on: January 07, 2018, 05:57:59 pm »
I actually answered on a related bug report, but in the end encryption should be performed byte (bit) wise and a programmer should be discouraged to use encryption on the basis of "string type xxx".
That is usually only a valid concept for AnsiString or UCS2: fixed length stringtypes.
It would be much clearer if all those "string" interfaces would be replaced by streams.
It is much easier to teach such a proper concept than explaining every time that a char is not always a byte size.

Actually: all those string types are legacy and assume AnsiString. Suggest to mark all of them as deprecated or rename to AnsiString or RawByteString. And even then deprecate them.... :P :o >:D
Proper encryption is on the binary level. Bits and bytes... O:-) Not "strings" whatever that may mean.

Just an honest opinion. "Strings" are too often affected by code rot (what is it? Ansi?, UTF8 (Laz only), WideString? UnicodeString?...)  because you can not immediately identify which string type it is.
Which implies that Object Pascal is a strongly typed language ... just not for "strings".... >:D >:D >:( >:(
And what you want is byte level, not char level.

It is not too difficult to explain SizeOf() and a Buffer cast.
« Last Edit: January 07, 2018, 07:16:07 pm by Thaddy »
Specialize a type, not a var.

engkin

  • Hero Member
  • *****
  • Posts: 3112
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #38 on: January 07, 2018, 07:21:35 pm »
I actually answered on a related bug report, but in the end encryption should be performed byte (bit) wise and a programmer should be discouraged to use encryption on the basis of "string type xxx".
That is usually only a valid concept for AnsiString or UCS2: fixed length stringtypes.
I agree with you.

It would be much clearer if all those "string" interfaces would be replaced by streams.
That is the case we have here.

It is much easier to teach such a proper concept than explaining every time that a char is not always a byte size.
Yes.

Actually: all those string types are legacy and assume AnsiString. Suggest to mark all of them as deprecated or rename to AnsiString or RawByteString. And even then deprecate them.... :P :o >:D
Understandable.


Proper encryption is on the binary level. Bits and bytes... O:-) Not "strings" whatever that may mean.
Yes, the encryption/decryption is on binary level.


Just an honest opinion. "Strings" are too often affected by code rot (what is it? Ansi?, UTF8 (Laz only), WideString? UnicodeString?...)  and what you want is byte level, not char level.
True.


It is not too difficult to explain SizeOf() and a Buffer cast.
Correct.

Now, to do the encryption on a binary level we need to use streams. Michael had changed TStringSteam1 to support encoding, Delphi compatibility, you know (Thank you Michael, wonderful job).

After decryption we need the data in its original format (image, text, file, etc.) which in our example here is a UTF8 encoded string. So again we use TStringSteam2.

TStringSteam1,2:
Thaddy, see? we are using streams, right? That is binary, for encryption and decryption.


THE PROBLEM:
TStringSteam uses TEncoding.Default which, under Windows where Lazarus changes DefaultSystemCodePage from ANSI to UTF8, has a different encoding than UTF8. That is expected because the change to using TEncoding in TStringStream happened fairly recent.

I am suggesting that Lazarus should correct TEncoding.Default when it changes DefaultSystemCodePage to UTF8. What do you think?

Maybe in the initialization section of $(Lazarus)\components\lazutils\fpcadds.pas
or more probable in SetMultiByteConversionCodePage in that unit.

What do you think Thaddy? as you can see, the real problem has nothing to do with encryption. It is disagreement between TEncoding.Default and DefaultSystemCodePage. Both should represent the same encoding. That is not the case in the trunk under Windows when using LazUtils.

Juha are you reading this?

Thaddy

  • Hero Member
  • *****
  • Posts: 14205
  • Probably until I exterminate Putin.
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #39 on: January 07, 2018, 08:55:31 pm »
Well, yes and no:
Yes there is an issue with TEncoding. (We were out of that for years, great feature! <cynical>...In the mid to late 90's I was writing software in TP and Delphi that also catered a.o. the Japanese and Korean markets. Who needs TEncoding? <more cynical>.)
TStringstream? What string type? same argument. (get rid of string and string aliases, name them for what they are:Ansi, UTF8, UnicodeString. Clean up the language)

As for encryption your argument does not hold true, because by inference the string type representation of a binary encrypted buffer *must* be known to the programmer in order to decrypt to the correct string type.

Shortstring can stay btw.... as string... :P :D :D {$MACRO ON}{$Define  H+ := H-}  8-) - wish that actually works... untested  :-X

Just to let you know: my knowledge of Japanese is extremely limited and Korean is completely lacking in my grey matter. Doesn't mean you can not write software for it.(in a team)
« Last Edit: January 07, 2018, 09:10:52 pm by Thaddy »
Specialize a type, not a var.

JuhaManninen

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 4459
  • I like bugs.
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #40 on: January 07, 2018, 10:05:02 pm »
Juha are you reading this?
Yes. The TEncoding changes sound valid. Please write about it in Lazarus mailing list or open a bug report. In both cases a patch would be welcome for testing it.
Mattias is the main architect of our UTF-8 system although I have been louder in this forum. It will be interesting to see his comment on this issue.
Mostly Lazarus trunk and FPC 3.2 on Manjaro Linux 64-bit.

BeniBela

  • Hero Member
  • *****
  • Posts: 905
    • homepage
Re: BlowFish busted in Laz 1.9 / Fpc 3.1.1
« Reply #41 on: January 14, 2019, 12:44:20 am »
Well, yes and no:
Yes there is an issue with TEncoding. (We were out of that for years, great feature! <cynical>...In the mid to late 90's I was writing software in TP and Delphi that also catered a.o. the Japanese and Korean markets. Who needs TEncoding? <more cynical>.)
TStringstream? What string type? same argument. (get rid of string and string aliases, name them for what they are:Ansi, UTF8, UnicodeString. Clean up the language)

Seriously, no one needs TEncoding. Just convert everything to utf-8 and you never need any other encodings. Utf-8 was the future, and now it is here.

And a default encoding that differs from the default codepage is the height of pointlessness.


Yes. The TEncoding changes sound valid. Please write about it in Lazarus mailing list or open a bug report.

Here it is: https://bugs.freepascal.org/view.php?id=32961

It is been there for a year and nearly nothing happened. Has anyone just accepted that they can't use utf-8 in streams? And the new fpc has  brought the same mess to stringlists. No more utf-8 in stringlists

 

TinyPortal © 2005-2018