Crazy encrypting algo

[Zaap]

I made a server/client network program with encrypted communication.

I generate a key (ex: 49523851).
(Actually it's four keys that I concat in one).

To encrypt I split the key like this :
49523851 -> 49,52,38,51.
And I do a ceasar encryption using these numbers, so "a,b,c,d,e" becomes "a+49,b+52,c+38,d+51,e+49" (modulo 255).

To decrypt I substract the key and add 255 if [char]-[number]<-127 (extended ASCII table).

It works pretty fine, except when the message's length is big (like 6000-7000 char).

Passing this point, the decrypted message becomes weird, so when I expect something like "lorem ipsum" i got "lore-/d§jb¨".

Here's the code of my reception function :

/** Receive a message */
std::string msgr(SOCKET s, const bool& show=true){
    diffhell dh;
    char buff[bufsize];
    std::string str="";
    int n(bufsize);
    bool noResponse(false);
    while(n!=SOCKET_ERROR && n>=bufsize && !noResponse){
        if(n=recv(s,buff,bufsize,0)) for(int i(0);i<n;++i) str+=buff[i];
        else noResponse=true;
    }
    if(::CRYPTED) str=dh.uncrypt(::MY_KEY,::MY_KEY2,::MY_KEY3,::MY_KEY4,str);
    bool write(false);
    while(!write){
        fstream file("log.txt");
        if(file.is_open()){
            file.seekp(0,ios::end);
            file<<str<<"\n";
            write=true;
        } else{
            system("title>>log.txt");
        }
    }
    if(str.size()<5 || !(str.substr(0,2)=="__" && str.substr(str.size()-2,2)=="__")) std::cout<<"<from>"<<str<<std::endl;
    return str;
}

Here's my decrypt function :

std::string uncrypt(const uint64_t& key, const uint64_t& key2, const uint64_t& key3, const uint64_t& key4, std::string msg){
        std::string tk(to_string(key)+to_string(key2)+to_string(key3)+to_string(key4)); //text key
        vector<int> tmp_tk;
        if(tk.size()%2!=0) tk+=tk[0];
        while(tk.size()>0){
            tmp_tk.push_back(atoi(tk.substr(0,2).c_str()));
            tk=tk.substr(2);
        }
        for(int i(0);i<tmp_tk.size();++i) tk+=(char)tmp_tk[i];
        while(tk.size()<msg.size()){
            tk+=tk;
        }

        if(msg.size()<=tk.size()){
            std::string tmp="";
            for(int i(0);i<msg.size();++i){
                char tmp_val=msg[i]-tk[i];
                if(tmp_val<-127) tmp_val+=255;
                tmp+=tmp_val;
            }
            return tmp;
        }
    }

Thanks for help.

[lastchance]

How big is bufsize?

What happens with those long messages if the key is 00000000 (i.e. no encryption)?

Also, are your chars signed or unsigned?

[Zaap]

#define bufsize 4096

The case 0000000 can't happen with my generation algo.

My chars are signed.

[jonnin]

(like 6000-7000 char).
#define bufsize 4096

^^^ This looks like a problem...

[Zaap]

Not really (r if it's the case there is something I missunderstand).

The buffer is only here to read the stream from recv(), but the string used to decrypt is "str".

If there is some part of the code that you're not sure to fully understand don't hesitate to ask me so I comment it.

Edit : after some verifications the problem come from the bufsize, but I don't understand why.

[coder777]

Try removing && n>=bufsize from line 8 in msgr(...). It is actually redundant. Maybe you get less than 4096 even though more data will arrive. But it shouldn't be garbage...

[Zaap]

I tried, but nothing works without this condition.

[lastchance]

Zaap wrote:

Edit : after some verifications the problem come from the bufsize, but I don't understand why.

Are you sure the problem lies in the fragments of code that you have provided? What happens in the SENDING and ENCRYPTION routines? Or anywhere else that you might conceivably have used the item bufsize ...

Can you interrogate the value of n after that recv() operation?

[coder777]

I tried, but nothing works without this condition.

Why not? What doesn't work?

[Zaap]

Ok I don't understand what is happenning.

After interrogating my N, i saw that the "decrypt bug" doesn't appear always at the same location.

Another weird thing is that when I re-ask the request, the full text is clear.

And it does this continuously, switching between bug/clear text.

However the N is correctly filled by the recv() function. (A request to the server return something like 4096->4096->3250).

I'm pretty the problem is coming from the recept function, but here's my send code :

/** Send a message */
void msgs(SOCKET s, std::string msg, const bool& show=true){
    diffhell dh;
    if(::CRYPTED) msg=dh.crypt(::MY_KEY,::MY_KEY2,::MY_KEY3,::MY_KEY4,msg);
    send(s,msg.c_str(),msg.size(),0);
    std::cout<<"Send:"<<msg<<std::endl;
    msg="";
}

[salem c]

Is your text ASCII only?

Is there any possibility that any of so "a,b,c,d,e" becomes "a+49,b+52,c+38,d+51,e+49" could result in a '\0' character being generated?

> if(::CRYPTED) str=dh.uncrypt(::MY_KEY,::MY_KEY2,::MY_KEY3,::MY_KEY4,str);
Maybe do

if(::CRYPTED) decoded_str=dh.uncrypt(::MY_KEY,::MY_KEY2,::MY_KEY3,::MY_KEY4,str);

So that you can write both the 'before' and 'after' results to your log file.

It will help you figure out whether your recv() is broken, or your uncrypt() is broken.

[Emil Enchev]

It works pretty fine, except when the message's length is big (like 6000-7000 char).

After interrogating my N, i saw that the "decrypt bug" doesn't appear always at the same location.

Another weird thing is that when I re-ask the request, the full text is clear.

And it does this continuously, switching between bug/clear text.

?! I don't understand shit about "server/client network programing" but are you sure that you receive encrypted message without errors in it before you start to decrypted it?!

1.Can you check the number of bytes in the message you receive, and that of original encrypted message and see if they are equal?

2. Is the bug's behavior like this one

"lorem ipsum" i got "lore-/d§jb¨"

- from a certain point to the end of the message everything is wrong or only some middle part of it?

[lastchance]

Zaap wrote:

The case 0000000 can't happen with my generation algo.

Override it, so that it CAN happen. Then you can test the sending of an unencrypted message first,

[dutch]

Your algorithm can be simplified quite a bit. This example uses "hex" keys, i.e., the uint64_t key units are used one-byte-at-a-time for displacement of the letters.

#include <iostream>
#include <iomanip>
#include <string>
#include <cctype>

void crypt(std::string& msg, const uint64_t key[4], bool decrypt = false) {
    unsigned k = 0, shft = 64;
    for (char& ch: msg) {
        unsigned n = (unsigned char)ch;
        shft -= 8;
        unsigned x = (key[k] >> shft) & 0xff;
        if (decrypt) x = 0x100 - x;
        n = (n + x) % 0x100;
        ch = (char)n;
        if (shft == 0) {
            k = (k + 1) % 4;
            shft = 64;
        }
    }
}

void dump(const std::string& str) {
    for (char ch: str)
        std::cout << std::setw(5) << (int)(unsigned char)ch << ": "
                  << (isgraph(ch) ? ch : '.') << '\n';
    std::cout << '\n';
}

int main() {
    uint64_t key[4] = {
        0x0102030405060708,
        0x090A0B0C0D0E0F10,
        0x1112131415161718,
        0x191A1B1C1D1E1F20,
    };
    std::string msg = "one two three four five six seven eight";
    std::cout << msg << '\n';
    crypt(msg, key);
    dump(msg);
    crypt(msg, key, true);
    std::cout << msg << '\n';
}

As for all zeroes not being possible, is that because all of the individual offsets are non-zero? If so, that's a weakness in the algo since we know for sure that no character stands for itself (this weakness was in the famous Enigma machine).

In the code above, we deal with the characters explicitly as unsigned so that the signedness of char doesn't matter. Also, instead of subtracting the value to decrypt, we add the inverse (i.e., for "subtracting" 0xDD we add 0x100 - 0xDD = 0x23). This keeps the modulus simple.

Note that to print the char numeric value properly as unsigned you need to cast it to unsigned char and then cast that to unsigned (or even int).

#include <iostream>

int main() {
    signed char ch = (signed char)0xdd;
    std::cout <<                     (int)ch << '\n';  // -35
    std::cout <<                (unsigned)ch << '\n';  // 4294967261 (i.e., 2**32 - 35)
    std::cout <<           (unsigned char)ch << '\n';  // (prints symbol for char 221, if anything)
    std::cout << (unsigned)(unsigned char)ch << '\n';  // 221
}

[jonnin]

what I saw was this:

while(n!=SOCKET_ERROR && n>=bufsize && !noResponse){
if(n=recv(s,buff,bufsize,0)) for(int i(0);i<n;++i) str+=buff[i];

which, I admit, I did not fully study, and only read in a drive-by. But my drive-by read of it flipped a red flag in my head that said "this looks like a constant is being used to mean 2 different things". Or, I think someone said it, n>=bufsize just looks weird. It could be right; but if you have a bug, this is where I would dig. Salem could be onto something as well, but overflow to zero *looked* unlikely to me (again with the drive by).

[Zaap]

Wow I've got a ton of responses, so let me answer to this.

Salem c:
1) Yes it's ASCII only.
2) You're right my algo might generate the '\0' char. I don't know if this is the cause of the bug, but I'll fix it anyway.
3) I don't see the goal of printing the before/after string. I'm sure that my uncrypt algo try to decrypt because the chars at the end aren't the same than the original message.

Emil Enchev:
1) I'll check the size of send/received messages, but I'm pretty sure this is ok.
2) The bug happen from a certain point until the end of the message.

Lastchance:
1) I'll try it.

dutch:
1) Yes I know, it's just that I recently added the 3 other keys so I didn't optimized/cleared the thing.
2) I generate my key with the Diffie-Hellman protocol, so it can't be 0000000, however it is possible for a letter to appear non-crypted. (ex: 21002685 -> +21,+00,+26,+85).
3) Yup, good idea.

jonnin:
1) All the globals I'm using (essentially for thread communication) are preceded by the :: operator and wrote in caps (ex: ::MY_GLOBAL).
2) About the n>=bufsize, this mean that I maybe still have something to read if the size is equal to the bufsize. If it's not, it means that it was the last packet.

Thank you for all the answers guys. I'll test everything tomorrow and then post the result of the tests here.

[jonnin]

I get the intent.

while(n!=SOCKET_ERROR && n>=bufsize && !noResponse)

what if n== 1234 and bufsize == 4096?
is it your desire to not loop anymore?

[Zaap]

jonnin:
Yes it is. n==1234 means it was the end of the recv() function's buffer, so I must stop reading.

I tried to comment the crypt/decrypt and so, the full text is clear, so the problem comes from my crypt/decrypt algo.

I checked the size of the message when I send it, when I recveive it, and when I uncrypt it, and this is weird.

Send : 12044
Recv : 4380+7664 = 12044 (I don't know why I receive it in two part).
Uncrypt : 4380+7664 = 12044

This is possibly due to the travel time of the full message, but I'm not sure.

I tried to set my keys to 0, and so it works in this case.

I tried to avoid the '\0' by adding one to my char if it was null. But still not working.

[Emil Enchev]

I tried to comment the crypt/decrypt and so, the full text is clear, so the problem comes from my crypt/decrypt algo.

1. So you don't have bugs when you send and receive plain BIG text, only when send text crypt/decrypt?

I don't know why I receive it in two part

2. This fragmentation is not matter when you can correctly restore the entire message from these fragments. As I know(but as I said, I don't know even the shit about socket programming) the stream protocol can fragment messages any way it likes.

3. If you sure that plain text messages are OK, tell me here and I will tell you how we will proceed. I'll find your bug if I know that it is not something happen in "socket" during send-recv process.

P.s. I can tell almost sure what the nature of bug is, but this almost is not 100%

First, you receive message on two parts with different sizes depending from sending protocol.

Second, you combine them together to make messageOriginal

Third, if the message1 have "\0" end on it when you combine message1+message2 = messageOriginal - if the text is PLAIN, nothing will happen, simply messageOriginal will be with one byte bigger(or two byte, if and message2 have null character end too). (You don't see Null character in the middle if look messageOriginal with plain text). But when you decrypt this messageOriginal (with one byte bigger) your decrypt will be correct exactly to the end of message1 where "\0" is added. Next decrypt will not work because first KEY will act on "\0", not on begin symbol from message2. Rest of text will be mess because you "shift" decrypt keys on second half of the messageOriginal so it will act more like crypt algo. This not mean that your decrypt algorithm don't work.

Fourth, sometimes you have correct decrypt because transfer protocol DON'T split send message on two fragment, but send it in one.

But as I said it is only theory.

P.p.s. Use test keys only from 00000000 instead 49523851. "a,b,c,d,e" becomes "a+0,b+0,c+0,d+0,e+0". In this way you will see if there is problems in your crypt/decrypt code, because as I suggest, the problem probably is not in them. They will not do nothing, and text will stay plain (am I worng?).

I tried to set my keys to 0, and so it works in this case.

So it is not decrypt part.

P.p.p.s

I tried to avoid the '\0' by adding one to my char if it was null. But still not working.

I don't understand where you add one char - but this is not the solution - you must extract char instead if such is added because if it is in the middle it will shift decrypt keys for second part of the message.

P.p.p.p.s Many developers first they try to look for the problem in the code, before understand the bug itself. Don't act in this way! First analyses the bug, next look in the code. So far there has not been a bug that has occurred and I can not find it :-) I have not even looked at your code yet. If you are cooperative, things will get faster.

[salem c]

There is NO guarantee that send() and recv() are synchronised in any way. send() can (soft)fail to send the whole buffer in one go, and recv() can (soft)fail to fill the buffer in one go.
You have to build your own retry loops to keep the connection going.

Sending:

const char *p = str.c_str();
size_t len = str.length();
ssize_t n;
while ( len > 0 && (n=send(sock,p,len,0)) > 0 ) {
    len -= n;
    p += n;
}

Receiving:

string result;
char buff[bufsize];
ssize_t n;
while ( (n=recv(sock,buff,bufsize,0)) > 0 ) {
    result += string(buff,n);
}