TCP socket server using OpenSSL

Question

I've written this basic TCP socket server that uses SSL. This is my first experience with sockets in C++. The client is a Qt desktop application.

Code overview:

1. Receive incoming connection.
2. Check if connection is from an existing socket, if so reply via that socket, otherwise, create, bind and send response via new socket.
3. Close socket connections when response has been made.

Does this code look safe and scalable? Any other comments or improvements I could make?

I've removed the #include statements for simplicity, but I'm using OpenSSL bindings along with sys/socket.h.

class TCP
{
  private:
    fd_set active_fd_set, read_fd_set;
    int i;
    struct sockaddr_in addr;
    uint len;
    SSL *ssl;
    int sock;
    SSL_CTX *ctx;
    const SSL_METHOD *method;
    auto CreateSocket(int port)->int;
    auto InitialiseOpenSSL()->void;
    auto CreateContext()->SSL_CTX*;
    auto ConfigureContext(SSL_CTX *ctx)->void;
    auto Listen()->int;
 public:
    TCP();
    ~TCP();
    auto INI()->void;
};

TCP::TCP():len(sizeof(addr)),ssl(new SSL),ctx(new SSL_CTX){};
TCP::~TCP()
{
   delete ssl;
   delete ctx;
   SSL_free(ssl);
   SSL_CTX_free(ctx);
   EVP_cleanup();
};

auto TCP::INI()->void
{
   this->InitialiseOpenSSL();
   this->CreateContext();
   this->ConfigureContext(ctx);
   sock=CreateSocket(3000);
   ssl=SSL_new(ctx);
   FD_ZERO(&active_fd_set);
   FD_SET(sock,&active_fd_set);
   this->Listen();
}

auto TCP::Listen()->int
{
   while(1)
   {
     read_fd_set=active_fd_set;
     if(select(FD_SETSIZE, &read_fd_set, NULL,NULL,NULL)<0) //Check sockets exist
     {
        std::cout<<"FD ERROR"<<std::endl;
     }
     for(i=0;i<FD_SETSIZE;i++) //Iterate through maximum avaliable socket spaces
     {
        if(FD_ISSET(i,&read_fd_set)) //Check if socket(i) is set in read_fd_set descriptor
        {
           if(i==sock) //If i == new socket, keep new, otherwise, get its existing socket connection
           {
              int new_conn=accept(sock,(struct sockaddr*)&addr,&len); //Create new connection
              if(new_conn<0){std::cout<<"New Connection failure"<<std::endl; return 0;} //ensure accept has worked
              FD_SET(new_conn,&active_fd_set); //Add a fd
              SSL_set_fd(ssl,new_conn); //Add ssl fd
              if(SSL_accept(ssl)<=0) //Wait for handshake inialistation
              {
                 std::cout<<"SSL_accept ERROR"<<std::endl;
                 return 0;
              }
              else
              {
                 std::cout<<":--> SSL HANDSHAKE COMPLETED"<<std::endl;
                 char buff[1024];
                 SSL_read(ssl,buff,sizeof(buff));
                 std::cout<<":--> NEW SOCKET CREATED & MESSAGE FROM CLIENT: "<<new_conn<<" : "<<buff<< std::endl;

                  char buff_response[]="Thanks from the server";
                  SSL_write(ssl,buff_response,1024);
                  std::cout<<":--> RESPONSE SENT TO CLIENT: "<<new_conn<<std::endl;
              }
            }
            else
            {
               char buff[1024];
               SSL_read(ssl,buff,sizeof(buff));
               std::cout<<":--> ORIGINAL SOCKET USED & RESPONSE FROM CLIENT: "<<i<<" : "<<buff<<std::endl;
               close(i);
               std::cout<<":--> SOCKET CLOSED: "<<i<<std::endl;
               FD_CLR(i,&active_fd_set); //remove sock from active_fd
             }
          }
        }
    }
  return 0;
}

auto TCP::CreateSocket(int port)->int
{
   int s;
   struct sockaddr_in addr;
   addr.sin_family=AF_INET;
   addr.sin_port=htons(port);
   addr.sin_addr.s_addr=htonl(INADDR_ANY);
   s=socket(AF_INET,SOCK_STREAM,0);
   if(s<0){perror("Unable to create socket"); exit(EXIT_FAILURE);}; //Check sock creation
   if(bind(s,(struct sockaddr*)&addr,sizeof(addr))<0){perror("Unable to bind"); exit(EXIT_FAILURE);}; //Bind
   if(listen(s,1)<0){perror("Unable to listen"); exit(EXIT_FAILURE);}; //Listen
 return s;
}

auto TCP::InitialiseOpenSSL()->void
{
   SSL_load_error_strings(); OpenSSL_add_ssl_algorithms();
}

auto TCP::CreateContext()->SSL_CTX*
{
   method=SSLv23_server_method(); //Protocol set
   ctx=SSL_CTX_new(method);
   if(!ctx){perror("Unable to create context"); ERR_print_errors_fp(stderr); exit(EXIT_FAILURE);};
   return ctx;
}

auto TCP::ConfigureContext(SSL_CTX *ctx)->void
{  if(SSL_CTX_use_certificate_file(ctx,"/root/myCA/server_crt.pem",SSL_FILETYPE_PEM)>0)
  {
    std::cout<<"CERTIFICATE INITIALISED"<<std::endl;
  }
if(SSL_CTX_use_PrivateKey_file(ctx,"/root/myCA/server_key.pem",SSL_FILETYPE_PEM)>0)
  {
    std::cout<<"KEY INITIALISED"<<std::endl;
  }
if(SSL_CTX_check_private_key(ctx)>0)
  {
    std::cout<<"KEY VALIDATED"<<std::endl;
  }
}


int main()
{
  TCP O; O.INI();
}

Answer 1

The worst problem with this code (IMO) is the readability, please take 10 minutes to have a look at other people's code on this site. I suspect you won't get much input because of the layout of your code.

1. Name variables properly, you might know what I is for now, but in 6 months...
2. You are deleting ssl and ctx without checking they have been allocated.
3. You aren't initialising all variables.
4. One statement, one line. Its possible to change an earlier statement and screw up later ones. (Your constructor and if statements)
5. while(1) ?? You should have a flag that can be controlled from outside the class, giving your code time to shutdown properly.
6. Your code is difficult to read because of lack of spaces.
7. Your code is difficult to follow because of lack of comments.
8. Hard coding 1024 is limiting it should be a constant or possibly a parameter.
9. Use const reference for parameters unless you are changing them.
10. Try C++ exceptions rather than exit statements, it will be easier to perform a controlled shutdown.

To answer all you questions:

1. Does it look safe? No - See point 2 above.
2. Is it scalable? I can't tell.