Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSL → TLS #1160

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/coverity-scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
--form [email protected] \
--form [email protected] \
--form version=coverity_scan \
--form description="Client for PPP+SSL VPN tunnel services" \
--form description="Client for PPP+TLS VPN tunnel services" \
https://scan.coverity.com/builds?project=adrienverge%2Fopenfortivpn
env:
TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ On the master branch there may be changes that are not (yet) described here.
* [-] improve tunnel speed on macOS
* [-] modify memory allocation in the tunnel configuration structure
* [+] openfortivpn returns the PPP exit status
* [+] print SSL socket options in log
* [+] print TLS socket options in log

### 1.15.0

Expand Down Expand Up @@ -362,7 +362,7 @@ On the master branch there may be changes that are not (yet) described here.
* [+] Export the configuration of routes and gateway to environment
* [~] Several improvements around establishing the tunnel connection and http traffic
* [+] Allow using a custom CA
* [-] Turn on SSL verification, check the hostname at least for the CN
* [-] Turn on TLS verification, check the hostname at least for the CN
* [+] Add --plugin option
* [-] Fix a format string warning in do_log_packet
* [~] Improved debugging output
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
openfortivpn
============

openfortivpn is a client for PPP+SSL VPN tunnel services.
openfortivpn is a client for PPP+TLS VPN tunnel services.
It spawns a pppd process and operates the communication between the gateway and
this process.

Expand Down
6 changes: 3 additions & 3 deletions doc/openfortivpn.1.in
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.TH OPENFORTIVPN 1 "May 4, 2020" ""

.SH NAME
openfortivpn \- Client for PPP+SSL VPN tunnel services
openfortivpn \- Client for PPP+TLS VPN tunnel services

.SH SYNOPSIS
.B openfortivpn
Expand Down Expand Up @@ -167,13 +167,13 @@ Pass phrase for the PEM-encoded key.
Log to syslog instead of terminal.
.TP
\fB\-\-trusted\-cert=\fI<digest>\fR
Trust a given gateway. If classical SSL certificate validation fails, the
Trust a given gateway. If classical TLS certificate validation fails, the
gateway certificate will be matched against this value. \fI<digest>\fR is the
X509 certificate's sha256 sum. The certificate has to be encoded in DER form.
This option can be used multiple times to trust several certificates.
.TP
\fB\-\-insecure\-ssl\fR
Do not disable insecure SSL protocols/ciphers.
Do not disable insecure TLS protocols/ciphers.
If your server requires a specific cipher, consider using \fB\-\-cipher\-list\fR
instead.
.TP
Expand Down
12 changes: 6 additions & 6 deletions src/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ int http_send(struct tunnel *tunnel, const char *request, ...)
n = safe_ssl_write(tunnel->ssl_handle, (uint8_t *) buffer,
length);
if (n < 0) {
log_debug("Error writing to SSL connection (%s).\n",
log_debug("Error writing to TLS connection (%s).\n",
err_ssl_str(n));
return ERR_HTTP_SSL;
return ERR_HTTP_TLS;
}

return 1;
Expand Down Expand Up @@ -169,13 +169,13 @@ int http_receive(struct tunnel *tunnel,

while ((n = safe_ssl_read(tunnel->ssl_handle,
(uint8_t *) buffer + bytes_read,
capacity - bytes_read)) == ERR_SSL_AGAIN)
capacity - bytes_read)) == ERR_TLS_AGAIN)
;
if (n < 0) {
log_debug("Error reading from SSL connection (%s).\n",
log_debug("Error reading from TLS connection (%s).\n",
err_ssl_str(n));
free(buffer);
return ERR_HTTP_SSL;
return ERR_HTTP_TLS;
}
bytes_read += n;

Expand Down Expand Up @@ -315,7 +315,7 @@ static int http_request(struct tunnel *tunnel, const char *method,

ret = do_http_request(tunnel, method, uri, data,
response, response_size);
if (ret == ERR_HTTP_SSL) {
if (ret == ERR_HTTP_TLS) {
ssl_connect(tunnel);
ret = do_http_request(tunnel, method, uri, data,
response, response_size);
Expand Down
7 changes: 4 additions & 3 deletions src/http.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
#define ERR_HTTP_INVALID -1
#define ERR_HTTP_TOO_LONG -2
#define ERR_HTTP_NO_MEM -3
#define ERR_HTTP_SSL -4
#define ERR_HTTP_SSL -4 // deprecated
#define ERR_HTTP_TLS -4
#define ERR_HTTP_BAD_RES_CODE -5
#define ERR_HTTP_PERMISSION -6
#define ERR_HTTP_NO_COOKIE -7
Expand All @@ -40,8 +41,8 @@ static inline const char *err_http_str(int code)
return "Request too long";
else if (code == ERR_HTTP_NO_MEM)
return "Not enough memory";
else if (code == ERR_HTTP_SSL)
return "SSL error";
else if (code == ERR_HTTP_TLS)
return "TLS error";
else if (code == ERR_HTTP_BAD_RES_CODE)
return "Bad HTTP response code";
else if (code == ERR_HTTP_PERMISSION)
Expand Down
14 changes: 7 additions & 7 deletions src/io.c
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ static void debug_bad_packet(struct tunnel *tunnel, uint8_t *header)
}

/*
* Thread to read bytes from the SSL socket, convert them to ppp packets and add
* Thread to read bytes from the TLS socket, convert them to ppp packets and add
* them to the 'ssl_to_pty' pool.
*/
static void *ssl_read(void *arg)
Expand All @@ -446,14 +446,14 @@ static void *ssl_read(void *arg)

ret = safe_ssl_read_all(tunnel->ssl_handle, header, 6);
if (ret < 0) {
log_debug("Error reading from SSL connection (%s).\n",
log_debug("Error reading from TLS connection (%s).\n",
err_ssl_str(ret));
break;
}

if (memcmp(header, http_header, 6) == 0) {
/*
* When the SSL-VPN portal has not been set up to allow
* When the TLS-VPN portal has not been set up to allow
* tunnel mode for VPN clients, while it allows web mode
* for web browsers, it returns an HTTP error instead of
* a PPP packet:
Expand Down Expand Up @@ -483,7 +483,7 @@ static void *ssl_read(void *arg)
ret = safe_ssl_read_all(tunnel->ssl_handle, pkt_data(packet),
size);
if (ret < 0) {
log_debug("Error reading from SSL connection (%s).\n",
log_debug("Error reading from TLS connection (%s).\n",
err_ssl_str(ret));
free(packet);
break;
Expand Down Expand Up @@ -525,7 +525,7 @@ static void *ssl_read(void *arg)
}

/*
* Thread to pop packets from the 'pty_to_ssl' pool, and write them to the SSL
* Thread to pop packets from the 'pty_to_ssl' pool, and write them to the TLS
* socket.
*/
static void *ssl_write(void *arg)
Expand Down Expand Up @@ -553,7 +553,7 @@ static void *ssl_write(void *arg)
packet->content, 6 + packet->len);
} while (ret == 0);
if (ret < 0) {
log_debug("Error writing to SSL connection (%s).\n",
log_debug("Error writing to TLS connection (%s).\n",
err_ssl_str(ret));
free(packet);
break;
Expand All @@ -567,7 +567,7 @@ static void *ssl_write(void *arg)
}

/*
* Thread to pop packets from the 'pty_to_ssl' pool, and write them to the SSL
* Thread to pop packets from the 'pty_to_ssl' pool, and write them to the TLS
* socket.
*/
static void *if_config(void *arg)
Expand Down
2 changes: 1 addition & 1 deletion src/io.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
#include <stdint.h>

/*
* For performance reasons, we store the 6-byte header used by the SSL
* For performance reasons, we store the 6-byte header used by the TLS
* communication right in front of the real PPP packet data. This way,
* SSL_write can be called directly on packet->content, instead of memcpy'ing
* the header + data to a temporary buffer.
Expand Down
6 changes: 3 additions & 3 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ PPPD_USAGE \
"\n"

#define summary \
"Client for PPP+SSL VPN tunnel services.\n" \
"Client for PPP+TLS VPN tunnel services.\n" \
"openfortivpn connects to a VPN by setting up a tunnel to the gateway at\n" \
"<host>:<port>. It spawns a pppd process and operates the communication between\n" \
"the gateway and this process.\n" \
Expand Down Expand Up @@ -143,15 +143,15 @@ PPPD_USAGE \
" authentication with a certificate.\n" \
" --pem-passphrase=<pass> Pass phrase for the PEM-encoded key.\n" \
" --use-syslog Log to syslog instead of terminal.\n" \
" --trusted-cert=<digest> Trust a given gateway. If classical SSL\n" \
" --trusted-cert=<digest> Trust a given gateway. If classical TLS\n" \
" certificate validation fails, the gateway\n" \
" certificate will be matched against this value.\n" \
" <digest> is the X509 certificate's sha256 sum.\n" \
" This option can be used multiple times to trust\n" \
" several certificates.\n"

#define help_options_part2 \
" --insecure-ssl Do not disable insecure SSL protocols/ciphers.\n" \
" --insecure-ssl Do not disable insecure TLS protocols/ciphers.\n" \
" Also enable TLS v1.0 if applicable.\n" \
" If your server requires a specific cipher or protocol,\n" \
" consider using --cipher-list and/or --min-tls instead.\n" \
Expand Down
72 changes: 40 additions & 32 deletions src/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,30 +54,38 @@
#define ERESTART -1
#endif

#define ERR_SSL_AGAIN 0
#define ERR_SSL_CLOSED -1
#define ERR_SSL_CERT -2
#define ERR_SSL_EOF -3
#define ERR_SSL_PROTOCOL -4
#define ERR_SSL_SEE_ERRNO -5
#define ERR_SSL_SEE_SSLERR -6
#define ERR_SSL_UNKNOWN -7
#define ERR_SSL_AGAIN 0 // deprecated
#define ERR_TLS_AGAIN 0
#define ERR_SSL_CLOSED -1 // deprecated
#define ERR_TLS_CLOSED -1
#define ERR_SSL_CERT -2 // deprecated
#define ERR_TLS_CERT -2
#define ERR_SSL_EOF -3 // deprecated
#define ERR_TLS_EOF -3
#define ERR_SSL_PROTOCOL -4 // deprecated
#define ERR_TLS_PROTOCOL -4
#define ERR_SSL_SEE_ERRNO -5 // deprecated
#define ERR_TLS_SEE_ERRNO -5
#define ERR_SSL_SEE_TLSERR -6 // deprecated
#define ERR_TLS_SEE_TLSERR -6
#define ERR_SSL_UNKNOWN -7 // deprecated
#define ERR_TLS_UNKNOWN -7

static inline const char *err_ssl_str(int code)
{
if (code == ERR_SSL_AGAIN)
if (code == ERR_TLS_AGAIN)
return "Try again";
else if (code == ERR_SSL_CLOSED)
else if (code == ERR_TLS_CLOSED)
return "Connection closed";
else if (code == ERR_SSL_CERT)
else if (code == ERR_TLS_CERT)
return "Want X509 lookup";
else if (code == ERR_SSL_EOF)
else if (code == ERR_TLS_EOF)
return "Protocol violation with EOF";
else if (code == ERR_SSL_PROTOCOL)
else if (code == ERR_TLS_PROTOCOL)
return "Protocol error";
else if (code == ERR_SSL_SEE_ERRNO)
else if (code == ERR_TLS_SEE_ERRNO)
return strerror(errno);
else if (code == ERR_SSL_SEE_SSLERR)
else if (code == ERR_TLS_SEE_TLSERR)
return ERR_reason_error_string(ERR_peek_last_error());
return "unknown";
}
Expand All @@ -87,37 +95,37 @@ static inline int handle_ssl_error(SSL *ssl, int ret)
int code;

if (SSL_get_shutdown(ssl) & SSL_RECEIVED_SHUTDOWN)
return ERR_SSL_CLOSED;
return ERR_TLS_CLOSED;

code = SSL_get_error(ssl, ret);
if (code == SSL_ERROR_WANT_READ || code == SSL_ERROR_WANT_WRITE)
return ERR_SSL_AGAIN; // The caller should try again
return ERR_TLS_AGAIN; // The caller should try again

if (code == SSL_ERROR_ZERO_RETURN)
return ERR_SSL_CLOSED;
return ERR_TLS_CLOSED;
if (code == SSL_ERROR_WANT_X509_LOOKUP)
return ERR_SSL_CERT;
return ERR_TLS_CERT;
if (code == SSL_ERROR_SYSCALL) {
if (ERR_peek_last_error() != 0)
return ERR_SSL_SEE_SSLERR;
return ERR_TLS_SEE_TLSERR;
if (ret == 0)
return ERR_SSL_EOF;
return ERR_TLS_EOF;
if (errno == EAGAIN || errno == ERESTART || errno == EINTR)
return ERR_SSL_AGAIN; // The caller should try again
return ERR_TLS_AGAIN; // The caller should try again
if (errno == EPIPE)
return ERR_SSL_CLOSED;
return ERR_SSL_SEE_ERRNO;
return ERR_TLS_CLOSED;
return ERR_TLS_SEE_ERRNO;
}
if (code == SSL_ERROR_SSL)
return ERR_SSL_PROTOCOL;
return ERR_SSL_UNKNOWN;
return ERR_TLS_PROTOCOL;
return ERR_TLS_UNKNOWN;
}

/*
* Reads data from the SSL connection.
* Reads data from the TLS connection.
*
* @return > 0 in case of success (number of bytes transferred)
* ERR_SSL_AGAIN if the caller should try again
* ERR_TLS_AGAIN if the caller should try again
* < 0 in case of error
*/
static inline int safe_ssl_read(SSL *ssl, uint8_t *buf, int bufsize)
Expand All @@ -132,7 +140,7 @@ static inline int safe_ssl_read(SSL *ssl, uint8_t *buf, int bufsize)
}

/*
* Reads all data from the SSL connection.
* Reads all data from the TLS connection.
*
* @return 1 in case of success
* < 0 in case of error
Expand All @@ -145,7 +153,7 @@ static inline int safe_ssl_read_all(SSL *ssl, uint8_t *buf, int bufsize)
int ret;

ret = safe_ssl_read(ssl, &buf[n], bufsize - n);
if (ret == ERR_SSL_AGAIN)
if (ret == ERR_TLS_AGAIN)
continue;
else if (ret < 0)
return ret;
Expand All @@ -155,14 +163,14 @@ static inline int safe_ssl_read_all(SSL *ssl, uint8_t *buf, int bufsize)
}

/*
* Writes data to the SSL connection.
* Writes data to the TLS connection.
*
* Since SSL_MODE_ENABLE_PARTIAL_WRITE is not set by default (see man
* SSL_get_mode), SSL_write() will only report success once the complete chunk
* has been written.
*
* @return > 0 in case of success (number of bytes transferred)
* ERR_SSL_AGAIN if the caller should try again
* ERR_TLS_AGAIN if the caller should try again
* < 0 in case of error
*/
static inline int safe_ssl_write(SSL *ssl, const uint8_t *buf, int n)
Expand Down
Loading