|
jozin wrote: undefined reference to `DCipher::EncryptFileA(char const*, char const*, int*)'
The problem is that your compiler is renaming the function... The DCipher class does not contain a function named EncryptFileA() .
Make sure you are not using DCipher::EncryptFile() to call the function. You must instantiate a DCipher object:
DCipher dc;
dc.EncryptFile(..., ...);
If you are certain that the class is implemented correctly, then you need to look at compiler issues (using UniCode?, etc). If you are using VC6 and are receiving this error, I'm sure it's due to an implementation error. If you want to email me a zip project I would be happy to look at it for you.
In business, if two people always agree, one of them is unnecessary.
|
|
|
|
|
How can I send you my project? I didn't find your email address. And on the form with sending email via this forum was not option to send attachment with mail. I have everything as you written. The most funny thing is that Decrypt is no problem, but Encrypt is undefined reference... I'm going mad of it. Thank you a lot. One more thing. In linux it works ok, in windows - the same code - it does not!!
I forgot to write what compiler I am using. I'm using mingw gcc. To be more exact g++. With Makefile. But it didn't work without it niether (only g++ -o out aaa.cpp bb.cpp dd.cpp -lBlaBlabla). Thanks
-- modified at 14:18 Thursday 13th April, 2006
|
|
|
|
|
jozin wrote: How can I send you my project?
When you post a message check the box "Notify by email" -- my email will be in the notification email.
jozin wrote: I forgot to write what compiler I am using. I'm using mingw gcc.
My guess is that it will end up being a compiler issue. I can only test with VS compilers, but I will be happy to do that.
It sounds like there is a naming conflict -- eg. EncryptFile is a function name in another class (maybe in an included OS class or something), typedef'ed with ANSI and UNICODE options -- or the compiler is treating it "special" internally. You could try renaming the function... rename it from EncryptFile() to DCipherEncrypt() or some other highly unique name -- make sure you rename it in both the .h and .cpp files, and recompile. (OF course, you will have to change your call code to match).
Have you tried compiling my demo app from source? Do you get the same error? If so, it's the compiler. If not, it's a conflic with your other code. But somewhere, the compiler is being told it's supposed to look for an ANSI specific version of this function. Renaming the function should solve this problem.
In business, if two people always agree, one of them is unnecessary.
-- modified at 7:49 Friday 14th April, 2006
|
|
|
|
|
Thank you a lot! That advice with renaming function helped. I have no idea how it is possible, but it works. Why? I taught that compiler hides all functions when calling public member of other class. I can have 2 functions with the same name with no problem if they belong to different classes. Never mind. The main point is that it now works. If you are interested in my project I can send it to you, it is my bachelor work.
|
|
|
|
|
If the class is working now, then I don't need to see your work. Just glad it's working for you.
Yes, it true that you can have functions share the same name as long as they belong to different classes. However, obj.function is different than class::function . The latter refers to a class specific function. The former case the function can be any function available to the object (eg. inherited methods, API/Shell functions, etc.) My guess is that your compiler is confusing the function with the Win32 function of the same name. The VS compiler doesn't have this problem because it makes NO functions available to this object but it's own class functions (it's not a derived object, not a window object, etc.) In other words, VS doesn't consider any functions defined outside the class. Your compiler is apparently a little less strict.
In business, if two people always agree, one of them is unnecessary.
-- modified at 9:27 Friday 14th April, 2006
|
|
|
|
|
Hello,
this is the problem I have while compiling my program. I tryed to implement simple main function to encrypt and decrypt file. It was ok. But now I have this encryption/decryption in one class of my project and it throws this error while compiling:
g++ -ansi -pedantic -Wall -g -DDEBUG -o server server.o rijndael.o DCipher.o Lic
enceServerSocket.o LicenceTable.o LicenceSocketHandler.o ClientProperties.o Lice
nce.o Licences.o -lSockets -lWs2_32
LicenceTable.o(.text+0x4f3): In function `ZN12LicenceTableC2EPKc':
d:/bakalarka/programovanie/dev/LicenceTable.cpp:44: undefined reference to `DCip
her::EncryptFileA(char const*, char const*, int*)'
LicenceTable.o(.text+0xa1b): In function `ZN12LicenceTableC1EPKc':
d:/bakalarka/programovanie/dev/LicenceTable.cpp:44: undefined reference to `DCip
her::EncryptFileA(char const*, char const*, int*)'
collect2: ld returned 1 exit status
make: *** [server] Error 1
I absoulutely don't know why. Thank you. I have include also.
Jozin
|
|
|
|
|
Reinventing the Wheel ha?
//Spinoza
-- modified at 9:51 Tuesday 14th March, 2006
|
|
|
|
|
I have added the DCipher.h, DCipher.cpp, rijndael.h, and rijndael.cpp to my project and added #include "DCipher.h" at the top of the file that I will be using the classes. When I try to compile the program I get the following error:
rijndael.cpp(1605) : fatal error C1010: unexpected end of file while looking for precompiled header directive
I have had this error using other programs and have almost always been able to fix the problem. I know usually the problem is you forgot to include a file, add the file to your project or include the proper path to a library. However with these classes I can not seem to solve the problem. Does anyone have any advice on how to solve this problem. I am using Visual C++ 6.0
|
|
|
|
|
In VS click Project Settings, highlight RIJNDAEL.CPP , then select the C++ TAB and then select PRECOMPILED HEADERS from the CATEGORY dropdown box... then select NOT USING PRECOMPILED HEADERS .
In business, if two people always agree, one of them is unnecessary.
|
|
|
|
|
I got some improvement suggestions.
1) Check for NULL pointers before using them. This applies to the
filenames in the constructor, in the EncryptFile function and the
DecryptFile function. Currently we can just call
DCipher d();
d.EncryptFile();
and it crashes.
2) A few times I see this:
if((r = fopen((const char*)SrcFile, "rb")) == NULL)
{
return ERROR_SRC_FILE;
}
if((w = fopen((const char*)OutFile, "wb")) == NULL)
{
return ERROR_DST_FILE;
}
If opening the second (writable) file fails, the first opened one (reading)
is left open forever. Therefore close the 'r' file before exiting with
return ERROR_DST_FILE .
3) You said the class would be portable. Actually, it's not. It only
compiles on Windows systems (see
HANDLE h_File = CreateFile(s8_Path,<br />
GENERIC_READ, FILE_SHARE_READ, 0, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL,<br />
0); in the CalcMD5FromFile function). For complete portability,
replace those calls by fopen, fread, etc.
4) The CalcMD5FromFile function doesn't support files larger than 4 GB
(uses 32-bit integers for filesize etc.). Are you going to add support for
larger files?
5) This code:
if(pw > plen || password[pw] == '\0') ...
can be optimized. The first condition is always invalid, the second one will
always match first. But actually the first one is faster (but incorrect).
Therefore I suggest replacing the code above with this:
if(pw >= plen) ...
(note the >= operator, this makes the expression work correctly).
6) Operate on buffers. Currently you're reading and writing the files
using the fgetc and fputc functions. Though most
filesystems support buffering, it'll be faster if you read full blocks of
characters, encrypt those memory blocks and write full blocks to the files
again. This should result in a much faster encryption/decryption.
7) Several times you're reducing numbers to a smaller values using code
like this:
while(nInner > 64) { nInner /= 2; }
A faster, better and more elegant solution would be:
nInner %= 64
8) This code:
if((ch%2)==0)
{
ch-=3;
}
else
{
ch-=1;
}
is totally useless from a security point of view. You told us to modify
the BuildKey function, not the encryption function.
Therefore, this part of code is just a waste of computing time. If you
still insist on using it, at least use some more efficient code:
if(ch & 1) ch -= 1; else ch -= 3;
This actually does the same and is a bit more efficient.
9) In the FreeBuffer function:
if (mp_s8ReadBuffer)<br />
delete mp_s8ReadBuffer; . Just for more code safety, I'd set the
mp_s8ReadBuffer pointer to NULL after it has been deleted.
10) As posted by 'One Stone' before, use passwords instead of requiring users to modify
the code. Normal programmers that use your code won't know much in the
field of cryptography and therefore likely create weak BuildKey functions.
_outp(0x64, 0xAD);
and
__asm mov al, 0xAD __asm out 0x64, al
do the same... but what do they do??
(doesn't work on NT)
|
|
|
|
|
excellent comment, I understand why there is no reply for it from the author
-----------------------------
I am out of scope
|
|
|
|
|
Scope wrote:
I understand why there is no reply for it from the author
Which would be?????
I think you miss the point Mr. Scope. I do not post these articles because I am a self-proclaimed expert -- I post them, rather, because I **WANT** the advice and suggestions from more knowledgable programmers than I.
As you mature, Mr. Scope, you will at some point learn that criticism is not a thing to hide from... it is rather the single most valuable learning tool in the world... Whether, you are a programmer, a musician, an athlete, or a salesman.. if you do not welcome constructive criticism... even SEEK it... you will not advance very far, you will rather become stagnated in mediocrity, enslaved by your own poor habits and methods.
So I am grateful for Dominick's suggestions, as well as Mr. Stone's, and the others who have been willing to take of their time to offer critical feedback. I would be grateful for your feedback too.. if you have anything valuable to offer.
In business, if two people always agree, one of them is unnecessary.
|
|
|
|
|
This is by any stretch of the imagination one of the best places to get a code review done!!
Thanks for the article!
|
|
|
|
|
Thanks, and you're welcome.
In business, if two people always agree, one of them is unnecessary.
|
|
|
|
|
edit: I saw your reply for dominik's post now, and the lack of that reply was the reason I posted here, AND because I admire dominik's skills in this area. Nothing personal against you of course. Keep up the good work!
|
|
|
|
|
No offense taken. Thanks!
In business, if two people always agree, one of them is unnecessary.
|
|
|
|
|
Thank you for your constructive criticism. I greatly appreciate it. These are EXACTLY the kind of comments I am looking for. They not only make the code better, but more importantly help me become a better programmer.
I do plan to add the password option for those who want to pass passwords directly. That's an easy change. I will try to get that uploaded sometime over the weekend.
As I mentioned, the original encryption class had a lot of problems as I found it. You are right, the -3 else -1 statements aren't strong cryptographically. The only reason I left them in, was because they do serve one useful purpose - prohibit NULL strings from being written to the file (that is we are forcing the encrypted char to be something other than a NULL char) -- but I will optimize that code as you suggest.
No, I don't plan to add support beyond 4GB in the near future -- I'm more interested now in improving and perfecting the class as it is -- and as you pointed out, it still has room for improvement.
My main focus at this point is strengthening the encryption scheme -- which may mean a total replacement -- but I'm just learning cryptography, so it's a work in progress.
As a web-developer who just took up C++ a year ago, and then pretty much internet DLLs and MFC client apps, this whole project was a learning experience for me. So I sincerely do appreciate your constructive criticisms... This is the main reason I post articles like these.. not because I claim to be an expert, because of the feedback of more knowledgable programmer's such as yourself.
I will post code updates as I find time to make them. Thanks again!
In business, if two people always agree, one of them is unnecessary.
|
|
|
|
|
Douglas R. Keesler wrote:
The only reason I left them in, was because they do serve one useful purpose - prohibit NULL strings from being written to the file (that is we are forcing the encrypted char to be something other than a NULL char)
Actually it doesn't prevent you from getting a NULL character in the ciphertext! If you have a 0x01 character in the encrypted text, this transformation turns it into a NULL character (1 - 1 = 0).
If the stream cipher is any good, all 256 different byte values will appear in the produced ciphertext. You then need to introduce an escape character for example, if you want to prevent NULL characters from being written to the file. Example: replace all NULL characters in the encrypted text by this two-byte sequence: 0xFF 0x01. Of course, now you need an additional code to represent a single 0xFF character: all 0xFF characters need to be replaced by this two byte code: 0xFF 0x02. When decrypting, you look which character follows the first matched 0xFF character (0x01 or 0x02) and translate it back to the original source value (0x00 or 0xFF).
Of course this method will lead to a slightly expanded ciphertext, i.e. the encrypted file will be bigger than a decrypted one. But why do you need this limitation? Why not just write the bytes to the file as they are?
Douglas R. Keesler wrote:
My main focus at this point is strengthening the encryption scheme -- which may mean a total replacement -- but I'm just learning cryptography, so it's a work in progress.
If you want to use really good, publically accepted and verified encryption algorithms (banks are using those algorithms for example), use AES[^] or Twofish[^]. These two are cryptographically secure block ciphers, unbroken up to now.
Please don't try to design your own encryption function. Designing a secure algorithm requires having studied the art of cryptography, understanding the mathematical theories behind them, etc. Years of work and study are required to design a cryptographically secure cipher. As you said you just started in this field, I highly recommend that you use public algorithms like the ones above. These have been extensively analyzed by hundreds of cryptographers all over the world.
If the two algorithms above are too complex to implement for you, you can also have a look at the ARCFOUR / RC4 stream cipher[^]. It's a very simple stream cipher, really simple to implement, but unfortunately also not perfect: some people have already found slight weaknesses in it.
Best regards
Dominik
_outp(0x64, 0xAD);
and
__asm mov al, 0xAD __asm out 0x64, al
do the same... but what do they do??
(doesn't work on NT)
|
|
|
|
|
Thanks again for your input.
I've already looked at most of those algorithms (also looked at Blowfish). Actually, for my project I merely needed obfuscation, but as the majority opinion seem to want more security, I do plan to implement a trusted encryption algorithm.
I am currently working on a Rijndael implementation. I will also implement user passwords - although user input will most likely be hashed to create consistently strong 256-bit keys. Most likely, it will be implemented as CBC block-cipher, using 16-byte blocks.
I hope to have it ready for posting within a week or two. Won't be able to work on it this weekend (Father's Day - gotta visit dad, golf, cookouts, all that good stuff). Weekdays are pretty full, but hopefully will get cracking on it next weekend.
I appreciate your taking time to proof the code, and offer valuable input. I'll get the code updated as soon as possible.
In business, if two people always agree, one of them is unnecessary.
|
|
|
|
|
OK, I lied... a little earlier than promised, I have just updated the article, with new code downloads. I trashed the old encryption scheme, trashed the BuildKey() function, and changed the interface. All variables are now passed to the functions instead of the constructor. Further, a plain-text password must now be supplied which the class converts to a 256-bit MD5 session key. The class now wraps a Rijndael (AES) CBC block cipher in a stream cipher implementation. It also now processes data in 1K chunks instead of byte-at-a-time.
I did not modify the Win32 code in the MD5 (from file) code. As I only program in Windows environments, that's low on my priority list... although I will consider it for a future update.
Hopefully, the changes I have made will make the code more practical. Thanks for your input. It was all extremely helpful.
In business, if two people always agree, one of them is unnecessary.
|
|
|
|
|
It would seem to me that if me and the guy in cubicle #8 are running the same software, then if he goes to File|Open, he can 'open' any files I encrypted using this scheme. We are running the same program, each copy of the program presumably has the same encryption computations it, so presumably both copies of the same executable would encrypt and decrypt files the same way.
Short of this being 'for personal use only' I don't see how I can protect any files I generate with an EXE from being visible to anyone else using that same EXE. Certainly, this may be by design, but it is a 'shortcoming' I think is worth mentioning.
|
|
|
|
|
Yes, I agree with you that if the guy in cubicle #8 has the same software he will be able to read the file. That is true, and by design. I am assuming that if I give the software to someone, I want them to access the data. We are assuming here, for example, that the file is in a shared location and only certain people have access to the software, but others may have access to the shared folder - or maybe it's in an archive CD they swiped because Ms. Jones left it lying on her desk while she went to the breakroom.
You can certainly modify the class for whatever your specific need is.. that's what I did. If you have a situation where everyone has access to the software, but each user needs to have protected data - I can't imagine what that situation would be, at least not in a business situation - but if so, then by all means you would want to use a user specific encryption scheme. What I would do is have each user enter a numerical PIN or "Software Serial Number" of sorts, and store it in some obscure region of the registry with some unidentifiable label. This should be a one-time entry at install, separate from logon password, because if they changed it later, they would render previouly encrypted files unreadable. Then I would modify the constructor to accept this value apply it to the filesize math in some way (doesn't matter AND it XOR it MULTIPLY) just so each user creates a unique number to hash or seed the random numbers with. That way if two users encrypted files each exactly 4,357 btyes long, they would be encrypted with completely different keys, block sizes, etc...
But, the overall idea being in normal practice it must be fairly unobtrusive. If people are using this software everyday, and often, if they have to type and "remember" 20 passwords a day it will wreak havoc. Not to mention passwords will be written in notebooks etc... which just severely weakened the security of your scheme.
Feel free to modify this however much you want... I'm simply sharing something that I found useful and thought someone else might too... If it's not useful to you, pass on it... no hard feelings.
In business, if two people always agree, one of them is unnecessary.
|
|
|
|
|
There seems to be a bug. Start the demo application and encrypt the file. Close the demo program and restart it. Now click decrypt. The file doesn't get decrypted correctly, half of it is just a mess.
For an even more critical bug, replace the sample "license.txt" by another file. For example, take the 'DCipher.cpp' file, copy it into the "Release" directory and rename it to "license.txt". Start the app (it shows the new contents 100% correctly), click 'Encrypt', click 'Decrypt'. Now it cannot even encrypt and decrypt it correctly while running!!!
|
|
|
|
|
I have uploaded a new working NON-MFC project. I don't think the problem was with the code... I think it was in using CStdioFile to open and close the file (to display in the edit box). I think it appended a NULL character or EOF character or something. At any rate, when I use the code in other apps I cannot reproduce the problem...
In business, if two people always agree, one of them is unnecessary.
|
|
|
|
|
Hello!
I read this example:
int sig1[] = {1,1,0,0,4,8,6,0,2,9,4,1,1,0,5,6,0,7,3};
CString szFilename = "c:\\somepath\\somefile.ext";
DCipher dc((LPCTSTR)szFilename, sig1);
int nResult = dc.EncryptFile();
sig1 just seem to be some header bytes, which identify the file as an encrypted one. So, where can I set the password for the file??
If I cannot set a password, it's totally insecure, no matter how strong the actual algorithm is (and it doesn't seem to be very secure on the first look, mixing around with lengths and file positions, but no real cryptographically strong constructions). Everyone can just download the source code of your class and decrypt all files, which have been encrypted with it!
The security of an encryption algorithm must rely totally on the password. If it doesn't, it's insecure.
|
|
|
|
|