1

I am having a partial failure with my code in that I can establish non-secure connections to places such as www.hp.com and www.google.com, but when adding SSL to the socket I can no longer connect to www.google.com even though I can still connect to www.hp.com

In response to this I did a fair bit of research and came up with mixed answers. I tried testing different sites, and the results were spotty. So I feel like I am missing a crucial portion of the SSL handshake here that is not readily apparent (at least to me).

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <resolv.h>
#include <netdb.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <openssl/bio.h>
#include <openssl/ssl.h>
#include <openssl/err.h>
#include <openssl/pem.h>
#include <openssl/x509.h>
#include <openssl/x509_vfy.h>
#include <errno.h>

#define SW_SUCCESS  1
#define SW_ERROR    0

typedef struct socket_info
{
    int SocketHandle;
    unsigned long AddressLong;
    unsigned short Port;
    char Host[256];
    char Address[16];
    char Request[256];
    char Agent[128];
    char Headers[512];
    SSL *SecureHandle;
    SSL_CTX *SecureContext;
    const SSL_METHOD *SecureMethod;
    BIO *SecureCertificate;
} SOCKETINFO, *PSOCKETINFO, *LPSOCKETINFO;


int secinit()
{
    OpenSSL_add_all_algorithms();
    ERR_load_BIO_strings();
    ERR_load_crypto_strings();
    SSL_load_error_strings();
    return SW_SUCCESS;
}


int rawclose(struct socket_info *psi)
{
    return (!close(psi->SocketHandle) ? SW_SUCCESS : SW_ERROR);
}

int secclose(struct socket_info *psi)
{
    SSL_free(psi->SecureHandle);
    SSL_CTX_free(psi->SecureContext);
    return (rawclose(psi) ? SW_SUCCESS : SW_ERROR);
}


int rawconnect(struct socket_info *psi)
{
    struct hostent *host;
    struct sockaddr_in addr;

    if ( (host = gethostbyname(psi->Host)) == NULL ) return SW_ERROR;

    if ( (psi->SocketHandle = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP)) < 0 )
    {
        psi->SocketHandle = 0;
        return SW_ERROR;
    }

    memset(&addr, 0, sizeof(addr));
    addr.sin_family = AF_INET;
    addr.sin_port = htons(psi->Port);
    addr.sin_addr.s_addr = *(long *)host->h_addr; 

    if ( connect(psi->SocketHandle, (struct sockaddr *)&addr, sizeof(struct sockaddr)) == -1 )
    {
        psi->SocketHandle = 0;
        return SW_ERROR;
    }

    return SW_SUCCESS;
}



int secconnect(struct socket_info *psi)
{
    // Initialise certificate BIO.
    psi->SecureCertificate = BIO_new(BIO_s_file());
    if ( SSL_library_init() < 0 ) return SW_ERROR;

    // Initialise SSL method and deprecate SSLv2.
    psi->SecureMethod = SSLv23_client_method();
    if ( (psi->SecureContext = SSL_CTX_new(psi->SecureMethod)) == NULL ) return SW_ERROR;
    SSL_CTX_set_options(psi->SecureContext, SSL_OP_NO_SSLv2);
    psi->SecureHandle = SSL_new(psi->SecureContext);

    // Connect to host with raw socket.
    if ( !rawconnect(psi) )
    {
        SSL_free(psi->SecureHandle);
        SSL_CTX_free(psi->SecureContext);
        return SW_ERROR;
    }

    // Upgrade socket to SSL enabled.
    SSL_set_fd(psi->SecureHandle, psi->SocketHandle);
    if ( SSL_connect(psi->SecureHandle) != 1 )
    {
        SSL_free(psi->SecureHandle);
        close(psi->SocketHandle);
        SSL_CTX_free(psi->SecureContext);
        return SW_ERROR;
    }

    return SW_SUCCESS;
}


int main(int argc, char **argv)
{
    SOCKETINFO si;
    int ret;

    // SSLWrap module initialisation.
    if ( secinit() != SW_SUCCESS ) return SW_ERROR;

    memset(&si, 0, sizeof(si));
    strncpy(si.Host, "www.google.com", 10);
    si.Port = 80;
    if ( (ret = rawconnect(&si)) != SW_SUCCESS ) return SW_ERROR;
    if ( (ret = rawclose(&si)) != SW_SUCCESS ) return SW_ERROR;

    memset(&si, 0, sizeof(si));
    strncpy(si.Host, "www.google.com", 10);
    si.Port = 443;
    if ( (ret = secconnect(&si)) != SW_SUCCESS ) return SW_ERROR;
    if ( (ret = secclose(&si)) != SW_SUCCESS ) return SW_ERROR;

    return 0;
}
jww
  • 97,681
  • 90
  • 411
  • 885
  • 3
    What is the failure? Are you getting any response? – Eugene Sh. Jun 26 '17 at 17:38
  • `connect(psi->SocketHandle, (struct sockaddr *)&addr, sizeof(struct sockaddr))` fails with error -1 when attempting to call it through the use of `if ( !rawconnect(psi) )` within the secure socket code. To clarify, the secure socket function is `int secconnect(SOCKETINFO *psi)` and the non-secure connect is `int rawconnect(SOCKETINFO *psi)` – Dana Vaughne Jun 26 '17 at 17:41
  • 4
    Check the `errno`. – Eugene Sh. Jun 26 '17 at 17:42
  • Failed with ERRNO[111]: Connection refused – Dana Vaughne Jun 26 '17 at 17:46
  • You are trying to connect to port `psi->Port`. It's unclear how you're choosing which port to use, but it is no longer safe to assume that HTTPS runs on a specific port (e.g. port 443), and other SSL/TLS applications have long run on various other ports. Trying to connect to a port that is not open would very likely produce a "connection refused" error such as you observe. – John Bollinger Jun 26 '17 at 17:50
  • Currently I am manually selecting the ports in the initial test calls, so psi->Port is just filled by me right now depending on whether I plan to call the raw (80) or sec (443) versions. However, I guess I had not really considered that Google, and some of these other tested sites that are failing, would have been using SSL on a port other than 443. I will have to look into that aspect further, but do you have any suggestions on how better to handle that issue? – Dana Vaughne Jun 26 '17 at 17:56
  • @DanaVaughne: Most services including google use the default port 443 for HTTPS. If the connection is refused to this port like in your case then there might be a firewall somewhere in the path blocking the connections or there are problems at the DNS level where the wrong IP address is returned for the server. Check if you can reach the host on the same port using another client on the same machine. – Steffen Ullrich Jun 26 '17 at 18:01
  • @Steffen ping and whois command return no unexpected information, and my web browsers seem to connect through HTTPS just fine. Considering the limited scope of the failure, I am fairly certain the error lies within my SSL implementation as detailed within the first chunk of posted code, or perhaps some port negotiation that I am currently unaware of as suggested by John. – Dana Vaughne Jun 26 '17 at 18:06
  • 2
    @DanaVaughne, you say your code is failing in `connect()` with `ECONNREFUSED`. This, then, has nothing to do with SSL in particular -- it is TCP-level failure occurring before any SSL handshake is initiated. That's why I suggested the possibility of a wrong port. – John Bollinger Jun 26 '17 at 18:08
  • 2
    @JohnBollinger is right in that the problem has nothing to do with SSL according to your error report. But, google.com is definitely running at port 443. And if the web browsers on the same machine connect fine to this site I could only imagine that they succeed by using a different IP address (maybe IPv6? Your code only uses IPv4). BTW, whois is not relevant in testing connectivity to a site and ping does not test connectivity at the TCP level to a specific port. You might try `openssl s_client -connect www.google.com:443` instead. – Steffen Ullrich Jun 26 '17 at 18:13
  • @John, I just tried this `nc -zv www.google.com 443` and got `Connection to www.google.com 443 port [tcp/https] succeeded!`. Is there perhaps some other reason I would receive an ECONNREFUSED ? @Steffen, the openssl s_client command confirmed that the SSL connection is operating properly. – Dana Vaughne Jun 26 '17 at 18:16
  • 1
    @DanaVaughne, If you can connect via `nc` but not via your own program then it follows that your program is doing something differently. If it's not trying to connect to a different *port*, then the only other significant variable is the *address*. How are you choosing that? – John Bollinger Jun 26 '17 at 18:21
  • @John, The address is derived from `if ( (host = gethostbyname(psi->Host)) == NULL ) return SW_ERROR;` after I personally set `psi->Host` to a value of my choosing (e.g. www.google.comm). The ping and whois on the address stored in the `addr` structure directly before the connect() call shows that the address data matches that of the prior successful rawconnect() call, and the data returned from whois/ping. Which is why I did the whois/ping initially. – Dana Vaughne Jun 26 '17 at 18:25
  • 1
    Actually, that was a question I should not have to ask. We ordinarily expect a question of this general kind to include a [mcve], and so far, yours does not. Please provide one. – John Bollinger Jun 26 '17 at 18:27
  • The dump of `addr` for rawconnect() to google.com is: `02 00 00 50 C6 69 F4 E4 00 00 00 00 00 00 00 00` whereas the same dump method used during secconnect() results in `02 00 01 BB C6 69 F4 E4 00 00 00 00 00 00 00 00` showing that the only information difference being the addr.sin_port being equal to either 80 or 443 respectively. – Dana Vaughne Jun 26 '17 at 18:28
  • _We ordinarily expect a question of this general kind to include a Minimal, Complete, and Verifiable example, and so far, yours does not. Please provide one._ please clarify, @John Bollinger the source code for the gethostbyname() is included, I manually set the psi->Host during the rawconnect() or secconnect() calls. The prototypes for rawconnect() and secconnect() are both included in the comments. The addresses were verified through three sources (addr dump, whois, ping), and the ports were verified through two more sources (openssl s_client, nc -zv). This is why I'm at a loss. – Dana Vaughne Jun 26 '17 at 18:33
  • 1
    @DanaVaughne, follow the link if you are unfamiliar with the idea of an MCVE and need details. The gist is the smallest code and input needed (Minimal) that is sufficient (Complete) for some one else to reproduce the same error (Verifiable), even if it is not the actual code in which you discovered the problem (Example). Generally, it *isn't* your original code -- that rarely passes the "minimal" criterion. What you have provided so far does not satisfy the "complete" or "minimal" criteria. – John Bollinger Jun 26 '17 at 18:38
  • `strncpy(si.Host, "www.google.com", 10);` 10 is too short; just use `strcpy()` here. [also a strong advice: **never** use `strncpy()` , at least until you know what it does ...] – wildplasser Jun 26 '17 at 19:01
  • @wildplasser thanks, I believe I just noticed that at about the same time you did as I was updating the original code posting to remove comments and printf() lines. – Dana Vaughne Jun 26 '17 at 19:07
  • 1
    It cost me just one minute of reading to find it ... – wildplasser Jun 26 '17 at 19:09
  • 1
    It should have been `strncpy(si.Host, "www.google.com", sizeof si.Host);` – user207421 Jun 27 '17 at 00:31

1 Answers1

1

strncpy(si.Host, "www.google.com", 10); 10 is too short; just use strcpy() here.

[also a strong advice: never use strncpy() , at least until you know what it does ...]


why is strncpy() considered evil? Mostly because it is confusing, at least for novice users. How?

using strncpy(dst, src, siz);:

  • If there is sufficient space in the destination buffer (strlen(src) < siz) , the restof the buffer will befilled with nullbytes
  • if there is not enough space (strlen(src) >= siz) , no more than sizbytes will be written to*dst, but the copied string wil not be nul-terminated.

The first case is not that important, a few cycles are wasted for writing the extra nulls. The second case can be disastrous if you expect the resulting string to be null-terminated. (most people do) This is why strncpy() raises red flags.


How to avoid strncpy() ? There are a few ways. one of the simplest:

len = strlen(src);
if (len >=siz) len=siz-1;
memcpy(dst, src, len);
dst[len]= 0;
wildplasser
  • 43,142
  • 8
  • 66
  • 109
  • The answer really should go to John as it was his advice to update my post to MCVE standards which revealed the error to me initially. Thanks for the look though. :) – Dana Vaughne Jun 26 '17 at 19:15
  • I'm sorry for John,then. I just read the question, skimmed the comments, they seemed to focus mainly on network stuff. – wildplasser Jun 26 '17 at 19:18
  • *"... just use strcpy() here... strong advice: never use strncpy()..."* - Awful advice. Isn't 40 or 50 years of security bugs due to `strcpy` enough? If backfilling with `0` is too unpalatable, they can look for alternate methods, like one of the safer string functions. Also see [Safe String Functions In Mac OS X and Linux](https://stackoverflow.com/q/4570147/608639). – jww Jun 26 '17 at 22:58
  • (You omitted: *at least until you know what it does*) How about false trust in `strncpy()` ? ... Cargo cult going terribly wrong... – wildplasser Jun 26 '17 at 23:04