CODEWITHSUNDEEP
c++winapisocks
user9414954
user9414954

Reputation:

Authentication - SOCKS 5

I am writing a SOCKS 5 server which talks with a SOCKS 5 client (maxthon browser). I got it to work without authentication. However, when I added authentication,

client sends user/pass to server server validates user/pass

server sends response to client

+----+--------+
|VER | STATUS |
+----+--------+
| 1  |   1    |
+----+--------+

VER = 0x01 STATUS = 0x00 (success)

  1. Am I right to assume that this is the end of the authentication?

After this I try to read data from the client,

+----+-----+-------+------+----------+----------+
|VER | CMD |  RSV  | ATYP | DST.ADDR | DST.PORT |
+----+-----+-------+------+----------+----------+
| 1  |  1  | X'00' |  1   | Variable |    2     |
+----+-----+-------+------+----------+----------+

However the bytes read at this point don't equal to the bytes expected by the sizeof this structure. The bytes read == 0.

  1. What is the issue?

If I take out the authentication my server works fine. However I wanted to get it working with it.

EDIT

Here is the code after the server authenticates the client:

    socks5_login SOCKS5_Login;
    if (Str::Compare(UserName, PUCHAR("test")) == 0 && Str::Compare(Password, PUCHAR("test")) == 0) {

        SOCKS5_Login.Version = 0x01;
        SOCKS5_Login.Status  = 0x00;
        if (Http::SendProxyData(Settings->Sock, PCHAR(&SOCKS5_Login), sizeof(socks5_login), Settings->Func) != TRUE)
            return FALSE;

        UCHAR Status;
        Settings->Func.Recv(Settings->Sock, PCHAR(Status), sizeof(UCHAR), 0);

        if (Status != NO_ERROR)
            return FALSE;

        /*
            The SOCKS request is formed as follows:
                +----+-----+-------+------+----------+----------+
                |VER | CMD |  RSV  | ATYP | DST.ADDR | DST.PORT |
                +----+-----+-------+------+----------+----------+
                | 1  |  1  | X'00' |  1   | Variable |    2     |
                +----+-----+-------+------+----------+----------+
        */

        socks5_request Request;
        if (Http::ReadProxyData(Settings->Sock, PCHAR(&Request.SOCKS5_Header), sizeof(socks5_header), Settings->Func) != TRUE)
            return FALSE;

        if (Request.SOCKS5_Header.Command != 1/*CONNECT*/ && 
            Request.SOCKS5_Header.AddressType != 1 /*IPv4*/ &&
            Request.SOCKS5_Header.Version != 5 /*SOCKS Version 5*/)
            return FALSE;

        // ...

When reading Request.SOCKS5_Header, Http::ReadProxyData() function returns FALSE ( recv() returned 0 ).

Upvotes: 0

Views: 750

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 596176

You are reading a UCHAR after sending your authentication reply (and you are type-casting its undefined value to PCHAR, because you are not passing it to Recv() using the & address operator. So you are likely to crash your code!). The client does not send a reply to your reply. By reading that UCHAR, you are reading the VER field of the next request, which means reading the full Request afterwards is likely to fail.

You should NOT be reading that UCHAR at all.

More importantly, you are setting the VER field of your authentication reply to 0x01 instead of to 0x05. You say the number of bytes being read for the next request is 0. That means the client is gracefully closing the connection on its end, which makes sense if you send back a malformed authentication reply.

Re-read RFC 1928 and RFC 1929 more carefully. The diagrams show you the byte sizes of each field, not the values you should set the fields to (those values are described in lists following the diagrams). For instance:

+----+--------+
|VER | STATUS |
+----+--------+
| 1  |   1    |
+----+--------+

Means the VER field is 1 byte in size, and STATUS is 1 byte in size. Not that you should be setting the VER field to 1.

Also, the if at the end of the code you showed should be using the || operator instead of the && operator. Though, you really shouldn't be validating the request fields the way you are, since different conditions require you to send a different error code back to the client so it knows WHY the request failed.

Try something more like this:

socks5_login LoginReply;
LoginReply.Version = 5;

if (Str::Compare(UserName, PUCHAR("test")) == 0 && Str::Compare(Password, PUCHAR("test")) == 0)
    LoginReply.Status = 0; /*success*/
else
    LoginReply.Status = 1; /*failure*/

if (Http::SendProxyData(Settings->Sock, PCHAR(&LoginReply), sizeof(LoginReply), Settings->Func) != TRUE)
{
    // close the connection...
    return FALSE;
}

if (LoginReply.Status != 0)
{
    // close the connection...
    return FALSE;
}

/*
The SOCKS request is formed as follows:
    +----+-----+-------+------+----------+----------+
    |VER | CMD |  RSV  | ATYP | DST.ADDR | DST.PORT |
    +----+-----+-------+------+----------+----------+
    | 1  |  1  | X'00' |  1   | Variable |    2     |
    +----+-----+-------+------+----------+----------+
*/

socks5_request Request;

// I'm assuming that SOCKS5_Header is only the 1st 4 bytes of the request...
if (Http::ReadProxyData(Settings->Sock, PCHAR(&Request.SOCKS5_Header), sizeof(socks5_header), Settings->Func) != TRUE)
{
    // close the connection...
    return FALSE;
}

if (Request.SOCKS5_Header.Version != 5) /*SOCKS Version 5*/
{
    // I'm assuming you have something like this defined.
    // Adjust this code as needed. What is important is that
    // you need to send back a Reply code before disconnecting...
    socks5_reply Reply;
    UCHAR dst[6] = {0}; // <-- if socks5_reply doesn't have DST.ADDR/DST.PORT fields of its own...

    Reply.SOCKS5_Header.Version = 5;
    Reply.SOCKS5_Header.Status = 1; /*general SOCKS server failure*/
    Reply.SOCKS5_Header.Reserved = 0;
    Reply.SOCKS5_Header.AddressType = 1;

    Http::SendProxyData(Settings->Sock, PCHAR(&Reply.SOCKS5_Header), sizeof(socks5_header), Settings->Func);
    Http::SendProxyData(Settings->Sock, PCHAR(dst), sizeof(dst), Settings->Func);

    // close the connection...
    return FALSE;
}

if (Request.SOCKS5_Header.Command != 1) /*CONNECT*/
{
    // same as above...
    Reply.SOCKS5_Header.Status = 7; /*Command not supported*/
    ...
    return FALSE;
}

if (Request.SOCKS5_Header.AddressType != 1) /*IPv4*/
{
    // same as above...
    Reply.SOCKS5_Header.Status = 8; /*Address type not supported*/
    ...
    return FALSE;
}

// finish reading DST.ADDR and DST.PORT fields into Request...
// use Request as needed...
// send a Reply accordingly...

Upvotes: 1

Related Questions