From ba44ce12b50b9caa3966b38df1a17be67ef59975 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos Orfanos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 28 Jan 2021 11:33:22 +0100 Subject: [PATCH 01/76] Assign an IPv4 address from the link-local subnet (#833) While the 192.0.2.0/24 TEST-NET-1 block would avoid collisions, the 169.254.0.0/16 subnet described in RFC3927 has been specified precisely for such cases. We should probably select "an address using a pseudo-random number generator with a uniform distribution in the range from 169.254.1.0 to 169.254.254.255 inclusive" as described in RFC3927. However, that would of course require more code and I'm lazy, but more importantly it had been reported that FortiClient uses 169.254.2.1. I'd rather use the same address as FortiClient. --- src/tunnel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tunnel.c b/src/tunnel.c index 96e9d511..8516adc6 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -232,7 +232,7 @@ static int pppd_run(struct tunnel *tunnel) static const char *const v[] = { ppp_path, "230400", // speed - ":192.0.2.1", // : + ":169.254.2.1", // : "noipdefault", "noaccomp", "noauth", From 7be6fed21e6cb2be0f270930b234a64c0bbf6ebe Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 5 Feb 2021 11:29:48 +0100 Subject: [PATCH 02/76] Add a space at the end of the OTP prompt Other prompts do have that final space: "VPN account password: " "Two-factor authentication token: " --- src/http.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http.c b/src/http.c index 97e96783..58e8fc7a 100644 --- a/src/http.c +++ b/src/http.c @@ -515,7 +515,7 @@ static int try_otp_auth(struct tunnel *tunnel, const char *buffer, } } if (p == NULL) - p = "Please enter one-time password:"; + p = "Please enter one-time password: "; /* Search for all inputs */ while ((s = strcasestr(s, " Date: Sun, 7 Feb 2021 19:19:35 +0100 Subject: [PATCH 03/76] Query for the PEM pass phrase only once OpenSSL queries for the PEM pass phrase whenever we call ssl_connect(). Instead openfortivpn will query for the PEM password only once, if needed, from within an OpenSSL callback. --- doc/openfortivpn.1.in | 5 +++ src/config.c | 10 ++++++ src/config.h | 3 ++ src/main.c | 81 +++++++++++++++++++++++++------------------ src/tunnel.c | 40 +++++++++++++++++++++ 5 files changed, 105 insertions(+), 34 deletions(-) diff --git a/doc/openfortivpn.1.in b/doc/openfortivpn.1.in index cadbcb4b..00058caa 100644 --- a/doc/openfortivpn.1.in +++ b/doc/openfortivpn.1.in @@ -145,6 +145,9 @@ or a partial PKCS11-URI (p11tool --list-token-urls) Use specified PEM-encoded key if the server requires authentication with a certificate. .TP +\fB\-\-pem-passphrase=\fI\fR +Pass phrase for the PEM-encoded key. +.TP \fB\-\-use\-syslog\fR Log to syslog instead of terminal. .TP @@ -309,6 +312,8 @@ user\-cert = @SYSCONFDIR@/openfortivpn/user\-cert.pem .br user\-key = @SYSCONFDIR@/openfortivpn/user\-key.pem .br +pem\-passphrase = baz +.br # the sha256 digest of the trusted host certs obtained by .br # openssl dgst -sha256 server\-cert.crt: diff --git a/src/config.c b/src/config.c index a9d2bdb2..d30e7a63 100644 --- a/src/config.c +++ b/src/config.c @@ -75,6 +75,8 @@ const struct vpn_config invalid_cfg = { .ca_file = NULL, .user_cert = NULL, .user_key = NULL, + .pem_passphrase = {'\0'}, + .pem_passphrase_set = 0, .insecure_ssl = -1, .cipher_list = NULL, .min_tls = -1, @@ -410,6 +412,10 @@ int load_config(struct vpn_config *cfg, const char *filename) } else if (strcmp(key, "user-key") == 0) { free(cfg->user_key); cfg->user_key = strdup(val); + } else if (strcmp(key, "pem-passphrase") == 0) { + strncpy(cfg->pem_passphrase, val, PEM_PASSPHRASE_SIZE); + cfg->pem_passphrase[PEM_PASSPHRASE_SIZE] = '\0'; + cfg->pem_passphrase_set = 1; } else if (strcmp(key, "insecure-ssl") == 0) { int insecure_ssl = strtob(val); @@ -579,6 +585,10 @@ void merge_config(struct vpn_config *dst, struct vpn_config *src) free(dst->user_key); dst->user_key = src->user_key; } + if (src->pem_passphrase_set) { + strcpy(dst->pem_passphrase, src->pem_passphrase); + dst->pem_passphrase_set = src->pem_passphrase_set; + } if (src->insecure_ssl != invalid_cfg.insecure_ssl) dst->insecure_ssl = src->insecure_ssl; if (src->cipher_list) { diff --git a/src/config.h b/src/config.h index 6c317eed..f7783e1f 100644 --- a/src/config.h +++ b/src/config.h @@ -63,6 +63,7 @@ struct x509_digest { #define PASSWORD_SIZE 256 #define OTP_SIZE 64 #define REALM_SIZE 63 +#define PEM_PASSPHRASE_SIZE 31 /* * RFC 6265 does not limit the size of cookies: @@ -120,6 +121,8 @@ struct vpn_config { char *ca_file; char *user_cert; char *user_key; + char pem_passphrase[PEM_PASSPHRASE_SIZE + 1]; + int pem_passphrase_set; int insecure_ssl; int min_tls; int seclevel_1; diff --git a/src/main.c b/src/main.c index f299bd77..1ba130a2 100644 --- a/src/main.c +++ b/src/main.c @@ -136,6 +136,7 @@ PPPD_USAGE \ " --user-cert=pkcs11: Use smartcard. Takes also partial or full PKCS11-URI.\n" \ " --user-key= Use specified PEM-encoded key if the server requires\n" \ " authentication with a certificate.\n" \ +" --pem-passphrase= Pass phrase for the PEM-encoded key.\n" \ " --use-syslog Log to syslog instead of terminal.\n" \ " --trusted-cert= Trust a given gateway. If classical SSL\n" \ " certificate validation fails, the gateway\n" \ @@ -194,6 +195,7 @@ int main(int argc, char **argv) .gateway_port = 443, .username = {'\0'}, .password = {'\0'}, + .password_set = 0, .otp = {'\0'}, .otp_prompt = NULL, .otp_delay = 0, @@ -223,6 +225,8 @@ int main(int argc, char **argv) .ca_file = NULL, .user_cert = NULL, .user_key = NULL, + .pem_passphrase = {'\0'}, + .pem_passphrase_set = 0, .insecure_ssl = 0, #ifdef TLS1_2_VERSION .min_tls = TLS1_2_VERSION, @@ -238,42 +242,43 @@ int main(int argc, char **argv) struct vpn_config cli_cfg = invalid_cfg; const struct option long_options[] = { - {"help", no_argument, NULL, 'h'}, - {"version", no_argument, NULL, 0}, - {"config", required_argument, NULL, 'c'}, - {"pinentry", required_argument, NULL, 0}, - {"realm", required_argument, NULL, 0}, - {"username", required_argument, NULL, 'u'}, - {"password", required_argument, NULL, 'p'}, - {"otp", required_argument, NULL, 'o'}, - {"otp-prompt", required_argument, NULL, 0}, - {"otp-delay", required_argument, NULL, 0}, - {"no-ftm-push", no_argument, &cli_cfg.no_ftm_push, 1}, - {"ifname", required_argument, NULL, 0}, - {"set-routes", required_argument, NULL, 0}, - {"no-routes", no_argument, &cli_cfg.set_routes, 0}, + {"help", no_argument, NULL, 'h'}, + {"version", no_argument, NULL, 0}, + {"config", required_argument, NULL, 'c'}, + {"pinentry", required_argument, NULL, 0}, + {"realm", required_argument, NULL, 0}, + {"username", required_argument, NULL, 'u'}, + {"password", required_argument, NULL, 'p'}, + {"otp", required_argument, NULL, 'o'}, + {"otp-prompt", required_argument, NULL, 0}, + {"otp-delay", required_argument, NULL, 0}, + {"no-ftm-push", no_argument, &cli_cfg.no_ftm_push, 1}, + {"ifname", required_argument, NULL, 0}, + {"set-routes", required_argument, NULL, 0}, + {"no-routes", no_argument, &cli_cfg.set_routes, 0}, {"half-internet-routes", required_argument, NULL, 0}, - {"set-dns", required_argument, NULL, 0}, - {"no-dns", no_argument, &cli_cfg.set_dns, 0}, - {"use-syslog", no_argument, &cli_cfg.use_syslog, 1}, - {"persistent", required_argument, NULL, 0}, - {"ca-file", required_argument, NULL, 0}, - {"user-cert", required_argument, NULL, 0}, - {"user-key", required_argument, NULL, 0}, - {"trusted-cert", required_argument, NULL, 0}, - {"insecure-ssl", no_argument, &cli_cfg.insecure_ssl, 1}, - {"cipher-list", required_argument, NULL, 0}, - {"min-tls", required_argument, NULL, 0}, - {"seclevel-1", no_argument, &cli_cfg.seclevel_1, 1}, + {"set-dns", required_argument, NULL, 0}, + {"no-dns", no_argument, &cli_cfg.set_dns, 0}, + {"use-syslog", no_argument, &cli_cfg.use_syslog, 1}, + {"persistent", required_argument, NULL, 0}, + {"ca-file", required_argument, NULL, 0}, + {"user-cert", required_argument, NULL, 0}, + {"user-key", required_argument, NULL, 0}, + {"pem-passphrase", required_argument, NULL, 0}, + {"trusted-cert", required_argument, NULL, 0}, + {"insecure-ssl", no_argument, &cli_cfg.insecure_ssl, 1}, + {"cipher-list", required_argument, NULL, 0}, + {"min-tls", required_argument, NULL, 0}, + {"seclevel-1", no_argument, &cli_cfg.seclevel_1, 1}, #if HAVE_USR_SBIN_PPPD - {"pppd-use-peerdns", required_argument, NULL, 0}, - {"pppd-no-peerdns", no_argument, &cli_cfg.pppd_use_peerdns, 0}, - {"pppd-log", required_argument, NULL, 0}, - {"pppd-plugin", required_argument, NULL, 0}, - {"pppd-ipparam", required_argument, NULL, 0}, - {"pppd-ifname", required_argument, NULL, 0}, - {"pppd-call", required_argument, NULL, 0}, - {"plugin", required_argument, NULL, 0}, // deprecated + {"pppd-use-peerdns", required_argument, NULL, 0}, + {"pppd-no-peerdns", no_argument, &cli_cfg.pppd_use_peerdns, 0}, + {"pppd-log", required_argument, NULL, 0}, + {"pppd-plugin", required_argument, NULL, 0}, + {"pppd-ipparam", required_argument, NULL, 0}, + {"pppd-ifname", required_argument, NULL, 0}, + {"pppd-call", required_argument, NULL, 0}, + {"plugin", required_argument, NULL, 0}, // deprecated #endif #if HAVE_USR_SBIN_PPP {"ppp-system", required_argument, NULL, 0}, @@ -392,6 +397,14 @@ int main(int argc, char **argv) cli_cfg.user_key = strdup(optarg); break; } + if (strcmp(long_options[option_index].name, + "pem-passphrase") == 0) { + strncpy(cli_cfg.pem_passphrase, optarg, + PEM_PASSPHRASE_SIZE); + cli_cfg.pem_passphrase[PEM_PASSPHRASE_SIZE] = '\0'; + cli_cfg.pem_passphrase_set = 1; + break; + } if (strcmp(long_options[option_index].name, "pinentry") == 0) { cli_cfg.pinentry = strdup(optarg); diff --git a/src/tunnel.c b/src/tunnel.c index 8516adc6..95dafc7c 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -29,6 +29,7 @@ #include "tunnel.h" #include "http.h" #include "log.h" +#include "userinput.h" #ifndef HAVE_X509_CHECK_HOST #include "openssl_hostname_validation.h" #endif @@ -960,6 +961,41 @@ static void ssl_disconnect(struct tunnel *tunnel) tunnel->ssl_context = NULL; } +/* + * Query for the pass phrase used for encrypted PEM structures + * (normally only private keys). + */ +static int pem_passphrase_cb(char *buf, int size, int rwflag, void *u) +{ + struct vpn_config *cfg = (struct vpn_config *)u; + + /* We expect to only read PEM pass phrases, not write them. */ + if (rwflag == 0) { + if (!cfg->pem_passphrase_set) { + if (size > PEM_PASSPHRASE_SIZE) { + read_password(NULL, NULL, + "Enter PEM pass phrase: ", + cfg->pem_passphrase, + PEM_PASSPHRASE_SIZE + 1); + cfg->pem_passphrase_set = 1; + } else { + log_error("Buffer too small for PEM pass phrase: %d.", + size); + } + } + if (cfg->pem_passphrase_set) { + assert(strlen(cfg->pem_passphrase) < size); + strncpy(buf, cfg->pem_passphrase, size); + buf[size - 1] = '\0'; + return strlen(buf); + } + } else { + log_error("We refuse to write PEM pass phrases!"); + } + + return -1; +} + /* * Connects to the gateway and initiate an SSL session. */ @@ -1016,6 +1052,10 @@ int ssl_connect(struct tunnel *tunnel) } } + /* Set the password callback for PEM certificates with encryption. */ + SSL_CTX_set_default_passwd_cb(tunnel->ssl_context, &pem_passphrase_cb); + SSL_CTX_set_default_passwd_cb_userdata(tunnel->ssl_context, tunnel->config); + if (tunnel->config->cipher_list) { log_debug("Setting cipher list to: %s\n", tunnel->config->cipher_list); if (!SSL_CTX_set_cipher_list(tunnel->ssl_context, From cde9d69dd44f83f00de38fe5bdb163fb3ee07913 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 11 Feb 2021 13:04:08 +0100 Subject: [PATCH 04/76] openfortivpn version 1.16.0 --- CHANGELOG.md | 9 +++++++++ configure.ac | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 515a688b..8472788f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,15 @@ Releases This high level changelog is usually updated when a release is tagged. On the master branch there may be changes that are not (yet) described here. +### 1.16.0 + +* [+] support for user key pass phrase +* [~] add a space at the end of the OTP prompt +* [-] 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 + ### 1.15.0 * [-] fix issue sending pin codes diff --git a/configure.ac b/configure.ac index 97ae3860..9c0f7d09 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ([2.63]) -AC_INIT([openfortivpn], [1.15.0]) +AC_INIT([openfortivpn], [1.16.0]) AC_CONFIG_SRCDIR([src/main.c]) AM_INIT_AUTOMAKE([foreign subdir-objects]) From f461630725507ee667ac59caa8d5cd9f37f28143 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sat, 13 Feb 2021 22:07:43 +0100 Subject: [PATCH 05/76] Update checkpatch.pl from Linux kernel --- tests/ci/checkpatch/checkpatch.pl | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/ci/checkpatch/checkpatch.pl b/tests/ci/checkpatch/checkpatch.pl index 00085308..1afe3af1 100755 --- a/tests/ci/checkpatch/checkpatch.pl +++ b/tests/ci/checkpatch/checkpatch.pl @@ -3390,13 +3390,6 @@ sub process { } } -# discourage the use of boolean for type definition attributes of Kconfig options - if ($realfile =~ /Kconfig/ && - $line =~ /^\+\s*\bboolean\b/) { - WARN("CONFIG_TYPE_BOOLEAN", - "Use of boolean is deprecated, please use bool instead.\n" . $herecurr); - } - if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && ($line =~ /\+(EXTRA_[A-Z]+FLAGS).*/)) { my $flag = $1; @@ -6646,6 +6639,12 @@ sub process { # } # } +# strlcpy uses that should likely be strscpy + if ($line =~ /\bstrlcpy\s*\(/) { + WARN("STRLCPY", + "Prefer strscpy over strlcpy - see: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw\@mail.gmail.com/\n" . $herecurr); + } + # typecasts on min/max could be min_t/max_t if ($perl_version_ok && defined $stat && From f93cd858d68596797f538cc5cfb3b2311f5a0f11 Mon Sep 17 00:00:00 2001 From: Alexander Ryzhov Date: Sun, 14 Feb 2021 10:52:28 +0300 Subject: [PATCH 06/76] Optional OpenSSL engines usage helps to compile against openssl with `no-engines` option. Original commit author: The-BB from @Entware --- src/tunnel.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/tunnel.c b/src/tunnel.c index 95dafc7c..9bd86a54 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -35,7 +35,9 @@ #endif #include +#ifdef OPENSSL_ENGINE #include +#endif #include #include #if HAVE_SYSTEMD @@ -1106,6 +1108,7 @@ int ssl_connect(struct tunnel *tunnel) #endif /* Use engine for PIV if user-cert config starts with pkcs11 URI: */ +#ifdef OPENSSL_ENGINE if (tunnel->config->use_engine > 0) { ENGINE *e; @@ -1166,6 +1169,7 @@ int ssl_connect(struct tunnel *tunnel) } } else { /* end PKCS11-engine */ +#endif if (tunnel->config->user_cert) { if (!SSL_CTX_use_certificate_file( @@ -1194,7 +1198,9 @@ int ssl_connect(struct tunnel *tunnel) return 1; } } +#ifdef OPENSSL_ENGINE } +#endif /* PKCS11-engine */ tunnel->ssl_handle = SSL_new(tunnel->ssl_context); if (tunnel->ssl_handle == NULL) { From 5e88cd90270e4b819a4d3249dd670b4e284b3986 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Tue, 16 Feb 2021 09:29:20 +0100 Subject: [PATCH 07/76] Avoid spurious warning on macOS On macOS, SO_SNDBUF and SO_RCVBUF are defined in but getsockopt() fails for these values with: Protocol not available I guess this is expected on macOS, although the GETSOCKOPT(2) man page does not document that. In any case the behaviour is common enough to avoid the warning in the log. Fixes #822. --- src/tunnel.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tunnel.c b/src/tunnel.c index 9bd86a54..d4434bad 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -637,15 +637,19 @@ static int tcp_connect(struct tunnel *tunnel) #ifdef SO_SNDBUF ret = tcp_getsockopt(handle, SO_SNDBUF); if (ret < 0) +#ifndef __APPLE__ log_warn("getsockopt: %s: %s\n", "SO_SNDBUF", strerror(errno)); else +#endif log_debug("SO_SNDBUF: %d\n", ret); #endif #ifdef SO_RCVBUF ret = tcp_getsockopt(handle, SO_RCVBUF); if (ret < 0) +#ifndef __APPLE__ log_warn("getsockopt: %s: %s\n", "SO_RCVBUF", strerror(errno)); else +#endif log_debug("SO_RCVBUF: %d\n", ret); #endif From fb319b78d4234e14feac135e1fbedefdb81f9e2a Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 17 Feb 2021 20:52:58 +0100 Subject: [PATCH 08/76] Adapt to latest changes in checkpatch.pl Fixes regression introduced by #844, it had gone unnoticed because of current Travis CI failures. --- tests/lint/checkpatch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lint/checkpatch.sh b/tests/lint/checkpatch.sh index fd4ff30a..d9a6f748 100755 --- a/tests/lint/checkpatch.sh +++ b/tests/lint/checkpatch.sh @@ -11,7 +11,7 @@ for file in "$@"; do tmp=$(mktemp) "$checkpatch_path" --no-tree --terse \ - --ignore LEADING_SPACE,SPDX_LICENSE_TAG,CODE_INDENT,NAKED_SSCANF,VOLATILE,NEW_TYPEDEFS,LONG_LINE,LONG_LINE_STRING,QUOTED_WHITESPACE_BEFORE_NEWLINE \ + --ignore LEADING_SPACE,SPDX_LICENSE_TAG,CODE_INDENT,NAKED_SSCANF,VOLATILE,NEW_TYPEDEFS,LONG_LINE,LONG_LINE_STRING,QUOTED_WHITESPACE_BEFORE_NEWLINE,STRLCPY \ -f "$file" | tee "$tmp" if [ -s "$tmp" ]; then From 79bf3d0f41294f88d8ac45a22e1947564cf83e67 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 18 Feb 2021 11:04:14 +0100 Subject: [PATCH 09/76] =?UTF-8?q?Travis=20CI=20=E2=86=92=20GitHub=20Action?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/openfortivpn.yml | 51 ++++++++++++++++++++++++++++++ .travis.yml | 30 ------------------ tests/ci/install_astyle.sh | 2 +- 3 files changed, 52 insertions(+), 31 deletions(-) create mode 100644 .github/workflows/openfortivpn.yml diff --git a/.github/workflows/openfortivpn.yml b/.github/workflows/openfortivpn.yml new file mode 100644 index 00000000..25030553 --- /dev/null +++ b/.github/workflows/openfortivpn.yml @@ -0,0 +1,51 @@ +--- +name: Tests + +on: + push: + branches-ignore: [master] + # Remove the line above to run when pushing to master + pull_request: + branches: [master] + +jobs: + astyle: + name: Style + runs-on: ubuntu-latest + + steps: + - name: Checkout Code + uses: actions/checkout@v2 + + - name: Install Dependencies + run: ./tests/ci/install_astyle.sh $HOME/.openfortivpn-deps + + - name: Artistic Style + run: ./tests/lint/astyle.sh $(git ls-files '*.[ch]' | grep -v openssl_hostname_validation) + + - name: Linux Kernel Coding Style + run: ./tests/lint/checkpatch.sh $(git ls-files '*.[ch]' | grep -v openssl_hostname_validation) + + - name: EOL at EOF + run: ./tests/lint/eol-at-eof.sh $(git ls-files | grep -v openssl_hostname_validation) + + - name: Line Length + run: ./tests/lint/line_length.py $(git ls-files '*.[ch]' | grep -v openssl_hostname_validation) + + build: + name: Build + runs-on: ubuntu-latest + + steps: + - name: Checkout Code + uses: actions/checkout@v2 + + - name: Install Dependencies + run: ./tests/ci/install_openssl.sh $HOME/.openfortivpn-deps + + - name: Build + run: | + ./autogen.sh + export PKG_CONFIG_PATH="$HOME/.openfortivpn-deps/lib/pkgconfig" + ./configure --prefix=/usr --sysconfdir=/etc + make diff --git a/.travis.yml b/.travis.yml index 103e7ebc..7875fdfb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,42 +9,12 @@ cache: directories: - $HOME/.openfortivpn-deps -before_install: - - | - if [ "$TRAVIS_OS_NAME" = "linux" ]; then - tests/ci/install_astyle.sh $HOME/.openfortivpn-deps - fi - - | - if [ "$TRAVIS_OS_NAME" != "osx" ]; then - tests/ci/install_openssl.sh $HOME/.openfortivpn-deps - fi - -install: - - ./autogen.sh - - | - if [ "$TRAVIS_OS_NAME" = "osx" ]; then - export PKG_CONFIG_PATH="/usr/local/opt/openssl/lib/pkgconfig:$PKG_CONFIG_PATH" - else - export PKG_CONFIG_PATH="$HOME/.openfortivpn-deps/lib/pkgconfig" - fi - - ./configure --prefix=/usr --sysconfdir=/etc - - make - -script: - - | - if [ "$TRAVIS_OS_NAME" = "linux" ]; then - ./tests/lint/run.sh - fi - env: global: # COVERITY_SCAN_TOKEN - secure: "HuYAk43itUiq0YJjmyvCyOM3mnRo8bFqSwksOmQ5fjvQscVWqZg6M9h8Abb+lKAc9L9avrwObjhu3EmOqoZ9Cjtp4WS4IdESK/UNe/jHpODFknsKefPgP67Z/yEFNVU1+5pS0+r+gTWQcZMyJrWrDde0lWpWHR/W7rTtqIbZH8Bp/T7oUN5q10x8PBcOZi/yVqNNicyuERIvUkobro5k+e4rY1N1HkptKs4wVY8M4WZUHwokNtuVfs0yoGQtrpi1KVRWq8sDkb8DNTeNv0MMEoQYUmNlEWxPaKJ9DBr65ajGLHzwUkWaNJGYw7mXwXoBwn5uKOyoytRA6Tj1wbVj4ggqqq6P6tbIcZ4YUW6N72gRaqvCnm6mRnbDHQtvzpqqxtr585mD11mh0DNmB15rvjNcZU/4wHHNQBsFcYEr+fN2/2ef/T3n0dymnnv2uZ05d11V5uSC6XYt3h0VxMDwarOVvOcWKYIQqzs4Bhigd2982v3OlIi5tbOSzHkbiCSP1msOKd6/XpCiCFpqed2UtSGZukisp3wQNfPnUOWoguOJ38nNe2o2G/l6HNOLd6u3sFlhzzthnJ6LPFD1CD9LPaOVYIVrRr90l0B2j1vPQM03eiiw7P6XvI6hPdqAM3VSEyjyrzUJRCUXAA8tovRwKTFlc506FWiFlGBFXFFI3zs=" addons: - homebrew: - packages: - - openssl@1.0 coverity_scan: project: name: adrienverge/openfortivpn diff --git a/tests/ci/install_astyle.sh b/tests/ci/install_astyle.sh index 2bbfedcd..de4ed365 100755 --- a/tests/ci/install_astyle.sh +++ b/tests/ci/install_astyle.sh @@ -4,7 +4,7 @@ set -e PREFIX="$1" -ln -fs "${PREFIX}/bin/astyle" "${HOME}/bin/astyle" +ln -fs "${PREFIX}/bin/astyle" "${HOME}/.local/bin/astyle" [ -x "${PREFIX}/bin/astyle" ] && exit 0 VERSION=3.1 From 622f38680370fefdc64de0dc582c13ebb143dacc Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 18 Feb 2021 11:04:58 +0100 Subject: [PATCH 10/76] Install astyle as an Ubuntu package GitHub Actions currently run on Ubuntu 18.04 and 20.04. Artistic Style 3.1 has been available since at least Ubutnu 18.04. I feel it's not worth the pain installing our own version of Artistic Style 3.1 and caching it. Let's use the current Ubuntu astyle package, whether 3.1 or newer. --- .github/workflows/openfortivpn.yml | 2 +- tests/ci/install_astyle.sh | 17 ----------------- 2 files changed, 1 insertion(+), 18 deletions(-) delete mode 100755 tests/ci/install_astyle.sh diff --git a/.github/workflows/openfortivpn.yml b/.github/workflows/openfortivpn.yml index 25030553..5a2ac2ce 100644 --- a/.github/workflows/openfortivpn.yml +++ b/.github/workflows/openfortivpn.yml @@ -18,7 +18,7 @@ jobs: uses: actions/checkout@v2 - name: Install Dependencies - run: ./tests/ci/install_astyle.sh $HOME/.openfortivpn-deps + run: sudo apt-get install -y astyle - name: Artistic Style run: ./tests/lint/astyle.sh $(git ls-files '*.[ch]' | grep -v openssl_hostname_validation) diff --git a/tests/ci/install_astyle.sh b/tests/ci/install_astyle.sh deleted file mode 100755 index de4ed365..00000000 --- a/tests/ci/install_astyle.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/sh - -set -e - -PREFIX="$1" - -ln -fs "${PREFIX}/bin/astyle" "${HOME}/.local/bin/astyle" -[ -x "${PREFIX}/bin/astyle" ] && exit 0 - -VERSION=3.1 -SRC="https://sourceforge.net/projects/astyle/files/astyle/astyle%20${VERSION}/astyle_${VERSION}_linux.tar.gz/download" - -wget -O astyle.tar.gz "$SRC" -tar -xf astyle.tar.gz -C "$HOME" -cd "${HOME}/astyle/build/gcc" -make -make prefix="$PREFIX" install From 47a754dbddf625433f4af81d80b3ce06df854e32 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 18 Feb 2021 11:15:15 +0100 Subject: [PATCH 11/76] Run checks when pushing to master It happens on exceptional circumstances only, but checks remain useful. --- .github/workflows/openfortivpn.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/openfortivpn.yml b/.github/workflows/openfortivpn.yml index 5a2ac2ce..c636bbd1 100644 --- a/.github/workflows/openfortivpn.yml +++ b/.github/workflows/openfortivpn.yml @@ -3,8 +3,7 @@ name: Tests on: push: - branches-ignore: [master] - # Remove the line above to run when pushing to master + pull_request: branches: [master] From bbf034e5ed4c730475b7a9c38f49e12b3bb2a228 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 18 Feb 2021 18:30:56 +0100 Subject: [PATCH 12/76] Get rid of Travis CI At some point I shall reimplement Coverity Scan support within GitHub Actions. --- .travis.yml | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 7875fdfb..00000000 --- a/.travis.yml +++ /dev/null @@ -1,25 +0,0 @@ -os: -- linux - -language: c - -sudo: false - -cache: - directories: - - $HOME/.openfortivpn-deps - -env: - global: - # COVERITY_SCAN_TOKEN - - secure: "HuYAk43itUiq0YJjmyvCyOM3mnRo8bFqSwksOmQ5fjvQscVWqZg6M9h8Abb+lKAc9L9avrwObjhu3EmOqoZ9Cjtp4WS4IdESK/UNe/jHpODFknsKefPgP67Z/yEFNVU1+5pS0+r+gTWQcZMyJrWrDde0lWpWHR/W7rTtqIbZH8Bp/T7oUN5q10x8PBcOZi/yVqNNicyuERIvUkobro5k+e4rY1N1HkptKs4wVY8M4WZUHwokNtuVfs0yoGQtrpi1KVRWq8sDkb8DNTeNv0MMEoQYUmNlEWxPaKJ9DBr65ajGLHzwUkWaNJGYw7mXwXoBwn5uKOyoytRA6Tj1wbVj4ggqqq6P6tbIcZ4YUW6N72gRaqvCnm6mRnbDHQtvzpqqxtr585mD11mh0DNmB15rvjNcZU/4wHHNQBsFcYEr+fN2/2ef/T3n0dymnnv2uZ05d11V5uSC6XYt3h0VxMDwarOVvOcWKYIQqzs4Bhigd2982v3OlIi5tbOSzHkbiCSP1msOKd6/XpCiCFpqed2UtSGZukisp3wQNfPnUOWoguOJ38nNe2o2G/l6HNOLd6u3sFlhzzthnJ6LPFD1CD9LPaOVYIVrRr90l0B2j1vPQM03eiiw7P6XvI6hPdqAM3VSEyjyrzUJRCUXAA8tovRwKTFlc506FWiFlGBFXFFI3zs=" - -addons: - coverity_scan: - project: - name: adrienverge/openfortivpn - description: Client for PPP+SSL VPN tunnel services - notification_email: DimitriPapadopoulos@users.noreply.github.com - build_command_prepend: ./configure --prefix=/usr --sysconfdir=/etc; make clean - build_command: make - branch_pattern: coverity_scan From 920142502fc26fac0bddd82ccb6c0aed1645c3df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillaume=20L=C3=A9croart?= Date: Wed, 17 Feb 2021 23:59:35 +0100 Subject: [PATCH 13/76] Prefer SSL_CTX_use_certificate_chain_file To honor bundle of chained client certificates --- src/tunnel.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/tunnel.c b/src/tunnel.c index d4434bad..c1bd91c5 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -1176,10 +1176,9 @@ int ssl_connect(struct tunnel *tunnel) #endif if (tunnel->config->user_cert) { - if (!SSL_CTX_use_certificate_file( - tunnel->ssl_context, tunnel->config->user_cert, - SSL_FILETYPE_PEM)) { - log_error("SSL_CTX_use_certificate_file: %s\n", + if (!SSL_CTX_use_certificate_chain_file( + tunnel->ssl_context, tunnel->config->user_cert)) { + log_error("SSL_CTX_use_certificate_chain_file: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); return 1; } From 14f6929aa6a4b16d2cc9a156ca737f758681a030 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 19 Feb 2021 08:25:48 +0100 Subject: [PATCH 14/76] Upgrade to the latest OpenSSL 1.0.2 release OpenSSL 1.0.2u is the latest publicly available release. OpenSSL 1.0.2y is currently the latest security fix for version 1.0.2, available under extended support contracts. We do not have access to it, so we stay with 1.0.2u. --- tests/ci/install_openssl.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ci/install_openssl.sh b/tests/ci/install_openssl.sh index e15a48a1..2cd1f7ee 100755 --- a/tests/ci/install_openssl.sh +++ b/tests/ci/install_openssl.sh @@ -6,7 +6,7 @@ PREFIX="$1" [ -x "${PREFIX}/bin/openssl" ] && exit 0 -VERSION=1.0.2o +VERSION=1.0.2u SRC="https://www.openssl.org/source/old/1.0.2/openssl-${VERSION}.tar.gz" wget -O openssl.tar.gz "$SRC" From e01a3eea9ecb702a7b9de15fb262686be42e0deb Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 19 Feb 2021 08:59:04 +0100 Subject: [PATCH 15/76] =?UTF-8?q?1=20=E2=86=92=20EXIT=5FFAILURE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/userinput.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/userinput.c b/src/userinput.c index 01e223a4..a0dc4b87 100644 --- a/src/userinput.c +++ b/src/userinput.c @@ -213,20 +213,20 @@ static void pinentry_read_password(const char *pinentry, const char *hint, close(to_pinentry[1]); if (dup2(to_pinentry[0], STDIN_FILENO) == -1) { perror("dup2"); - exit(1); + exit(EXIT_FAILURE); } close(to_pinentry[0]); close(from_pinentry[0]); if (dup2(from_pinentry[1], STDOUT_FILENO) == -1) { perror("dup2"); - exit(1); + exit(EXIT_FAILURE); } close(from_pinentry[1]); execlp(pinentry, pinentry, NULL); perror(pinentry); - exit(1); + exit(EXIT_FAILURE); } close(to_pinentry[0]); From 68ddf5ccdbfde512f6b31225ba09c2ba7f233df5 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 19 Feb 2021 09:13:11 +0100 Subject: [PATCH 16/76] Ignore pushes to master branch Otherwise we end up running tests twice with each pull request, first when submitting the pull request, then when pushing the pull request into master. --- .github/workflows/openfortivpn.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/openfortivpn.yml b/.github/workflows/openfortivpn.yml index c636bbd1..718414e7 100644 --- a/.github/workflows/openfortivpn.yml +++ b/.github/workflows/openfortivpn.yml @@ -3,6 +3,8 @@ name: Tests on: push: + branches-ignore: [master] + # Remove the line above to run when pushing to master pull_request: branches: [master] From a1059afed15d3e67e3acc39834ea877e9794999d Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 19 Feb 2021 13:11:30 +0100 Subject: [PATCH 17/76] Better debug output --- src/tunnel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tunnel.c b/src/tunnel.c index c1bd91c5..445e85ac 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -1271,7 +1271,7 @@ int run_tunnel(struct vpn_config *config) ret = auth_log_in(&tunnel); if (ret != 1) { log_error("Could not authenticate to gateway. Please check the password, client certificate, etc.\n"); - log_debug("%s %d\n", err_http_str(ret), ret); + log_debug("%s (%d)\n", err_http_str(ret), ret); ret = 1; goto err_tunnel; } From 89615fcb1c82cd4b3887a50976c71ebdd4c255a9 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 19 Feb 2021 15:34:46 +0100 Subject: [PATCH 18/76] Consistency in CFLAGS / CPPFLAGS --- Makefile.am | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index 78319800..810d007e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -8,14 +8,12 @@ openfortivpn_SOURCES = src/config.c src/config.h src/hdlc.c src/hdlc.h \ src/xml.h src/userinput.c src/userinput.h \ src/openssl_hostname_validation.c \ src/openssl_hostname_validation.h -openfortivpn_CFLAGS = -Wall -pedantic openfortivpn_CPPFLAGS = -DSYSCONFDIR=\"$(sysconfdir)\" \ -DPPP_PATH=\"@PPP_PATH@\" \ -DNETSTAT_PATH=\"@NETSTAT_PATH@\" \ -DRESOLVCONF_PATH=\"@RESOLVCONF_PATH@\" \ -DREVISION=\"@REVISION@\" - -openfortivpn_CPPFLAGS += $(OPENSSL_CFLAGS) $(LIBSYSTEMD_CFLAGS) +openfortivpn_CFLAGS = -Wall -pedantic $(OPENSSL_CFLAGS) $(LIBSYSTEMD_CFLAGS) openfortivpn_LDADD = $(OPENSSL_LIBS) $(LIBSYSTEMD_LIBS) PATHFILES = From 15036a6e2055826acd587f7d10815f562ab5fcca Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 22 Feb 2021 12:01:35 +0100 Subject: [PATCH 19/76] Support Coverity Scan from GitHub Actions Inspired by: https://github.com/ruby/actions-coverity-scan --- .github/workflows/coverity-scan.yml | 43 +++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 .github/workflows/coverity-scan.yml diff --git a/.github/workflows/coverity-scan.yml b/.github/workflows/coverity-scan.yml new file mode 100644 index 00000000..ef426a1e --- /dev/null +++ b/.github/workflows/coverity-scan.yml @@ -0,0 +1,43 @@ +--- +name: Synopsys + +on: + push: + branches: [coverity_scan] + +jobs: + coverity-scan: + name: Coverity Scan + runs-on: ubuntu-latest + + steps: + - name: Checkout Code + uses: actions/checkout@v2 + + - name: Download the Coverity Scan Build Tool + run: | + wget -q https://scan.coverity.com/download/cxx/linux64 --post-data "token=$TOKEN&project=adrienverge%2Fopenfortivpn" -O cov-analysis-linux64.tar.gz + mkdir cov-analysis-linux64 + tar xzf cov-analysis-linux64.tar.gz --strip 1 -C cov-analysis-linux64 + env: + TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }} + + - name: Build + run: | + ./autogen.sh + ./configure --prefix=/usr --sysconfdir=/etc + export PATH=`pwd`/cov-analysis-linux64/bin:$PATH + cov-build --dir cov-int make + + - name: Upload the Project Build + run: | + tar caf openfortivpn.xz cov-int + curl \ + --form token=$TOKEN \ + --form email=DimitriPapadopoulos@users.noreply.github.com \ + --form file=@openfortivpn.xz \ + --form version=coverity_scan \ + --form description="Client for PPP+SSL VPN tunnel services" \ + https://scan.coverity.com/builds?project=adrienverge%2Fopenfortivpn + env: + TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }} From 763618d6c284ad9f2ed123dca0562f3bb8c6bee6 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 22 Feb 2021 11:47:05 +0100 Subject: [PATCH 20/76] Revert 68ddf5c There are more benefits than drawbacks to checking both pull requests and direct pushes to the master branch. --- .github/workflows/openfortivpn.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/openfortivpn.yml b/.github/workflows/openfortivpn.yml index 718414e7..de4afb16 100644 --- a/.github/workflows/openfortivpn.yml +++ b/.github/workflows/openfortivpn.yml @@ -3,11 +3,10 @@ name: Tests on: push: - branches-ignore: [master] - # Remove the line above to run when pushing to master pull_request: - branches: [master] + branches: + - master jobs: astyle: From 47ae7b72c73dcd2a7d4bd22dd80d70e3b8e42f25 Mon Sep 17 00:00:00 2001 From: Emanuel Haupt Date: Wed, 3 Mar 2021 08:38:02 +0100 Subject: [PATCH 21/76] Add repology badges Add repology badges to display the packaging status in all known repositories. It's nice to compare versions and provides easy browseable access to each repository. --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index a7286e5f..dd693aa4 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,10 @@ or [install MacPorts](https://www.macports.org/install.php) then install openfor sudo port install openfortivpn ``` +A more complete overview can be obtained from repology + +[![Packaging status](https://repology.org/badge/vertical-allrepos/openfortivpn.svg)](https://repology.org/project/openfortivpn/versions) + ### Building and installing from source For other distros, you'll need to build and install from source: From 4010b4dcec78272a6bf36bd89e93b7c14fc1fcb0 Mon Sep 17 00:00:00 2001 From: Emanuel Haupt Date: Wed, 3 Mar 2021 09:32:29 +0100 Subject: [PATCH 22/76] Add colon after new title --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index dd693aa4..c03623b0 100644 --- a/README.md +++ b/README.md @@ -109,7 +109,7 @@ or [install MacPorts](https://www.macports.org/install.php) then install openfor sudo port install openfortivpn ``` -A more complete overview can be obtained from repology +A more complete overview can be obtained from repology: [![Packaging status](https://repology.org/badge/vertical-allrepos/openfortivpn.svg)](https://repology.org/project/openfortivpn/versions) From 31e696e95c8a10df46fdfc95f45e1ae776e57113 Mon Sep 17 00:00:00 2001 From: Emanuel Haupt Date: Wed, 3 Mar 2021 14:50:45 +0100 Subject: [PATCH 23/76] Provide a link for packaging status By feedback from @mrbaseman, provide a repology link to get a packaging overview. --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index c03623b0..bbc569a0 100644 --- a/README.md +++ b/README.md @@ -109,9 +109,7 @@ or [install MacPorts](https://www.macports.org/install.php) then install openfor sudo port install openfortivpn ``` -A more complete overview can be obtained from repology: - -[![Packaging status](https://repology.org/badge/vertical-allrepos/openfortivpn.svg)](https://repology.org/project/openfortivpn/versions) +A more complete overview can be obtained from [repology](https://repology.org/project/openfortivpn/versions). ### Building and installing from source From 72f17c0d62f8d6dad65a003d5d6a6c9b328185d8 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 11 Mar 2021 07:42:33 +0100 Subject: [PATCH 24/76] =?UTF-8?q?SSLv23=5Fclient=5Fmethod()=20=E2=86=92=20?= =?UTF-8?q?TLS=5Fclient=5Fmethod()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the man page: SSLv23_method(), SSLv23_server_method(), SSLv23_client_method() Use of these functions is deprecated. They have been replaced with the above TLS_method(), TLS_server_method() and TLS_client_method() respectively. New code should use those functions instead. --- src/tunnel.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tunnel.c b/src/tunnel.c index 445e85ac..a07d76a1 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -1019,9 +1019,11 @@ int ssl_connect(struct tunnel *tunnel) SSL_load_error_strings(); // Register the available ciphers and digests SSL_library_init(); -#endif tunnel->ssl_context = SSL_CTX_new(SSLv23_client_method()); +#else + tunnel->ssl_context = SSL_CTX_new(TLS_client_method()); +#endif if (tunnel->ssl_context == NULL) { log_error("SSL_CTX_new: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); From 0b51c6f9ce0273d084b8241c72f19677d0c2300d Mon Sep 17 00:00:00 2001 From: Juho Autio Date: Mon, 29 Mar 2021 10:18:41 +0300 Subject: [PATCH 25/76] Document secure password storage with pinentry (#872) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Document secure password storage with pinentry - Man page was missing the `--pinentry` option → add it - Improve the man page on plain text password vs. pinentry - README to mention man page as the main source of documentation - README to favor pinentry over plain text passwords - Fix format of headers in README --- README.md | 27 ++++++++++++++++----------- doc/openfortivpn.1.in | 20 ++++++++++++++++---- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index bbc569a0..849c23d7 100644 --- a/README.md +++ b/README.md @@ -7,8 +7,13 @@ this process. It is compatible with Fortinet VPNs. - +Usage -------- + +``` +man openfortivpn +``` + Examples -------- @@ -22,9 +27,14 @@ Examples openfortivpn vpn-gateway:8443 --username=foo --realm=bar ``` +* Store password securely with a pinentry program: + ``` + openfortivpn vpn-gateway:8443 --username=foo --pinentry=pinentry-mac + ``` + * Don't set IP routes and don't add VPN nameservers to `/etc/resolv.conf`: ``` - openfortivpn vpn-gateway:8443 -u foo -p bar --no-routes --no-dns --pppd-no-peerdns + openfortivpn vpn-gateway:8443 -u foo --no-routes --no-dns --pppd-no-peerdns ``` * Using a config file: ``` @@ -36,16 +46,17 @@ Examples host = vpn-gateway port = 8443 username = foo - password = bar - set-routes = 0 set-dns = 0 pppd-use-peerdns = 0 # X509 certificate sha256 sum, trust only this one! trusted-cert = e46d4aff08ba6914e64daa85bc6112a422fa7ce16631bff0b592a28556f993db ``` +* For the full list of config options, see the `CONFIG FILE` section of + ``` + man openfortivpn + ``` ---------- Smartcard --------- @@ -73,9 +84,6 @@ Multiple readers are currently not supported. Smartcard support has been tested with Yubikey under Linux, but other PIV enabled smartcards may work too. On Mac OS X Mojave it is known that the pkcs engine-by-id is not found. - - ----------- Installing ---------- @@ -160,7 +168,6 @@ For other distros, you'll need to build and install from source: Finally, install runtime dependency `ppp` or `pppd`. ----------------- Running as root? ---------------- @@ -189,8 +196,6 @@ As described in [#54](https://github.com/adrienverge/openfortivpn/issues/54), a malicious user could use `--pppd-plugin` and `--pppd-log` options to divert the program's behaviour. - ------------- Contributing ------------ diff --git a/doc/openfortivpn.1.in b/doc/openfortivpn.1.in index 00058caa..d2c0ae10 100644 --- a/doc/openfortivpn.1.in +++ b/doc/openfortivpn.1.in @@ -8,6 +8,7 @@ openfortivpn \- Client for PPP+SSL VPN tunnel services [\fI\fR[:\fI\fR]] [\fB\-u\fR \fI\fR] [\fB\-p\fR \fI\fR] +[\fB\-\-pinentry=\fI\fR] [\fB\-\-otp=\fI\fR] [\fB\-\-otp\-prompt=\fI\fR] [\fB\-\-otp\-delay=\fI\fR] @@ -68,7 +69,12 @@ Specify a custom config file (default: @SYSCONFDIR@/openfortivpn/config). VPN account username. .TP \fB\-p \fI\fR, \fB\-\-password=\fI\fR -VPN account password. +VPN account password in plain text. +For a secure alternative, use pinentry or let openfortivpn prompt for the password. +.TP +\fB\-\-pinentry=\fI\fR +The pinentry program to use. Allows supplying the password in a secure manner. +For example: pinentry-gnome3 on Linux, or pinentry-mac on macOS. .TP \fB\-o \fI\fR, \fB\-\-otp=\fI\fR One-Time-Password. @@ -288,7 +294,15 @@ port = 443 .br username = foo .br -password = bar +# Password in plain text. +.br +# For a secure alternative, use pinentry or let openfortivpn prompt for the password. +.br +# password = bar +.br +# The pinentry program to use. Allows supplying the password in a secure manner. +.br +# pinentry = pinentry-mac .br # realm = some-realm .br @@ -304,8 +318,6 @@ password = bar .br # no\-ftm\-push = 1 .br -# pinentry = pinentry program -.br user\-cert = @SYSCONFDIR@/openfortivpn/user\-cert.pem .br # user\-cert = pkcs1: # use smartcard as client certificate From c5a4be03a3f3a7090ba14017480281c93fddafb7 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 26 Mar 2021 10:14:53 +0100 Subject: [PATCH 26/76] =?UTF-8?q?CONFIG=20FILES=20=E2=86=92=20CONFIGURATIO?= =?UTF-8?q?N?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to man-pages(7), conventional man page section names include CONFIGURATION but not CONFIG FILES. On the other hand CONFIGURATION is Normally only in Section 4 Special files (devices). Still, I prefer standard names. --- doc/openfortivpn.1.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/openfortivpn.1.in b/doc/openfortivpn.1.in index d2c0ae10..98c358fa 100644 --- a/doc/openfortivpn.1.in +++ b/doc/openfortivpn.1.in @@ -277,7 +277,7 @@ VPN_ROUTE_GATEWAY_... the gateway for the current route entry If not compiled for pppd the pppd options and features that rely on them are not available. On FreeBSD \fB\-\-ppp\-system\fR is available instead. -.SH CONFIG FILE +.SH CONFIGURATION Options can be taken from a configuration file. Options passed in the command line will override those from the config file, though. The default config file is @SYSCONFDIR@/openfortivpn/config, but this can be set using the \fB\-c\fR option. From 9783ac3086bb8fdcde3a6bf2588cb025dc123b62 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sat, 13 Feb 2021 22:27:27 +0100 Subject: [PATCH 27/76] CodeQL Analysis Security analysis from GitHub for C, C++, C#, Java, JavaScript, TypeScript, Python, and Go developers. --- .github/workflows/codeql-analysis.yml | 67 +++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 .github/workflows/codeql-analysis.yml diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml new file mode 100644 index 00000000..f4e0b48f --- /dev/null +++ b/.github/workflows/codeql-analysis.yml @@ -0,0 +1,67 @@ +# For most projects, this workflow file will not need changing; you simply need +# to commit it to your repository. +# +# You may wish to alter this file to override the set of languages analyzed, +# or to provide custom queries or build logic. +# +# ******** NOTE ******** +# We have attempted to detect the languages in your repository. Please check +# the `language` matrix defined below to confirm you have the correct set of +# supported CodeQL languages. +# +name: "CodeQL" + +on: + push: + branches: [ master ] + pull_request: + # The branches below must be a subset of the branches above + branches: [ master ] + schedule: + - cron: '26 6 * * 5' + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + language: [ 'cpp' ] + # CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python' ] + # Learn more: + # https://docs.github.com/en/free-pro-team@latest/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-code-scanning#changing-the-languages-that-are-analyzed + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: ${{ matrix.language }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. + # queries: ./path/to/local/query, your-org/your-repo/queries@main + + # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). + # If this step fails, then you should remove it and run the build manually (see below) + - name: Autobuild + uses: github/codeql-action/autobuild@v1 + + # ℹ️ Command-line programs to run using the OS shell. + # 📚 https://git.io/JvXDl + + # ✏️ If the Autobuild fails above, remove it and uncomment the following three lines + # and modify them (or add more) to build your code if your project + # uses a compiled language + + #- run: | + # make bootstrap + # make release + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 From d8520e280e0eb346c8205ac2dd5a872a567f934d Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 29 Mar 2021 11:00:23 +0200 Subject: [PATCH 28/76] =?UTF-8?q?index=20=E2=86=92=20strchr?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Was missing from #570. --- configure.ac | 1 - 1 file changed, 1 deletion(-) diff --git a/configure.ac b/configure.ac index 9c0f7d09..977b2c1d 100644 --- a/configure.ac +++ b/configure.ac @@ -136,7 +136,6 @@ geteuid \ getifaddrs \ getopt_long \ htons \ -index \ inet_addr \ inet_ntoa \ ioctl \ From 4bcc4da1be2dcfc662089a5f22eb7c9e4274d239 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 29 Mar 2021 11:24:47 +0200 Subject: [PATCH 29/76] =?UTF-8?q?config=20file=20=E2=86=92=20configuration?= =?UTF-8?q?=20file?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 22 +++++++++---------- README.md | 2 +- doc/openfortivpn.1.in | 15 ++++++------- etc/openfortivpn/config.template | 2 +- src/config.c | 32 ++++++++++++++-------------- src/main.c | 36 ++++++++++++++++---------------- src/tunnel.c | 4 ++-- 7 files changed, 57 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8472788f..a87dd14b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,7 +82,7 @@ On the master branch there may be changes that are not (yet) described here. * [-] fix CVE-2020-7041: incorrect use of X509_check_host (regarding return value). * [-] always hide cleartest password in -vv output * [+] add a clear warning about sensitive information in the debug output -* [+] add a hint in debug output when password is read from config file +* [+] add a hint in debug output when password is read from configuration file * [-] fix segfault when connecting with empty password * [+] use resolvconf if available to update resolv.conf file * [~] replace semicolon by space in dns-suffix string @@ -146,7 +146,7 @@ On the master branch there may be changes that are not (yet) described here. * [~] Invert order of ssl libraries (this may help linking on some platforms) * [+] Add FreeBSD support and redesigned the autoconf mechanism * [+] Support building with gcc 8 -* [-] Prioritize command line arguments over config file parameters +* [-] Prioritize command line arguments over configuration file parameters * [~] Dynamically allocate routing buffer and therefore allow larger routing table * [+] Support systemd notification upon tunnel up * [+] Support building in a separate directory @@ -156,7 +156,7 @@ On the master branch there may be changes that are not (yet) described here. ### 1.7.1 -* [~] Be more tolerant about white space in config file +* [~] Be more tolerant about white space in configuration file * [~] Make better usage of pkg-config * [~] Rework linking against OpenSSL * [-] Build again on Mac OSX where pthread_mutexattr_setrobust is not available @@ -164,7 +164,7 @@ On the master branch there may be changes that are not (yet) described here. ### 1.7.0 * [~] Correctly set up route to vpn gateway (add support for some particular situations) -* [+] Support two factor authentication with config file (for NM-plugin) +* [+] Support two factor authentication with configuration file (for NM-plugin) * [~] Change the ip address in the pppd call parameters by a rfc3330 test-net address * [-] Correctly report the exit status codes of pppd * [+] Add --pppd-call option @@ -181,7 +181,7 @@ On the master branch there may be changes that are not (yet) described here. * [+] Support pppd ifname option * [+] Print a clear error message at runtime if pppd does not exist * [+] Print clear text error messages of pppd upon failure -* [~] Existing config file is not overwritten anymore at installation time +* [~] Existing configuration file is not overwritten anymore at installation time * [~] Increase the accepted cookie size and align the error behavior according to RFCs * [-] More gracefully handle unexcpected content of resolv.conf * [~] Dynamically allocate memory for split routes and thus support larger numbers of routes @@ -190,10 +190,10 @@ On the master branch there may be changes that are not (yet) described here. * [~] Improve error handling around the call of pppd * [+] Add half-internet-routes option -* [-] realm was not recognized in the config file +* [-] realm was not recognized in the configuration file * [~] Switch from no-routes and no-dns to set-routes and set-dns option * [+] Add pppd-no-peerdns and pppd-log option -* [~] Allow passing the otp via the config file for use with NetworkManager plugin +* [~] Allow passing the otp via the configuration file for use with NetworkManager plugin * [-] Fix issues initializing memory and with build system * [+] Support building against Openssl 1.1 * [~] use pkg-config for configuration of openssl instead of configure option @@ -236,7 +236,7 @@ On the master branch there may be changes that are not (yet) described here. ### 1.2.0 -* [+] Support login with client certificate, key, and ca-file specified in config file +* [+] Support login with client certificate, key, and ca-file specified in configuration file * [~] Use more meaningful error codes when loading config fails * [-] Correctly report errors of hostname lookup * [+] Add an option not to ask ppp peer for dns servers @@ -255,7 +255,7 @@ On the master branch there may be changes that are not (yet) described here. ### 1.1.3 -* [~] Support set-dns and set-routes flag from config file as well +* [~] Support set-dns and set-routes flag from configuration file as well * [-] Properly URL-encode values sent in http requests * [+] Add support for realm authentication * [+] Add support for two factor authentication @@ -295,13 +295,13 @@ On the master branch there may be changes that are not (yet) described here. * [~] Better error messages in /etc/resolv.conf helpers * [~] Use better colors for warnings and error messages and only if output is a tty -* [-] Fix parsing of "trusted-cert" in config file +* [-] Fix parsing of "trusted-cert" in configuration file * [~] Add --pedantic to CFLAGS * [+] Add ability to type password interactively * [+] Verify gateway's X509 certificate * [-] Don't delete nameservers at tear down if they were here before * [~] Set /etc/openfortivpn/config not readable by other users -* [+] Add ability to use a config file +* [+] Add ability to use a configuration file ### 1.0.0 diff --git a/README.md b/README.md index 849c23d7..6342cdce 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ Examples ``` openfortivpn vpn-gateway:8443 -u foo --no-routes --no-dns --pppd-no-peerdns ``` -* Using a config file: +* Using a configuration file: ``` openfortivpn -c /etc/openfortivpn/my-config ``` diff --git a/doc/openfortivpn.1.in b/doc/openfortivpn.1.in index 98c358fa..23b9f8ad 100644 --- a/doc/openfortivpn.1.in +++ b/doc/openfortivpn.1.in @@ -63,7 +63,7 @@ Show the help message and exit. Show version and exit. .TP \fB\-c \fI\fR, \fB\-\-config=\fI\fR -Specify a custom config file (default: @SYSCONFDIR@/openfortivpn/config). +Specify a custom configuration file (default: @SYSCONFDIR@/openfortivpn/config). .TP \fB\-u \fI\fR, \fB\-\-username=\fI\fR VPN account username. @@ -252,7 +252,7 @@ incoming connections. If one of these variables is defined, .B openfortivpn tries to first establish a TCP connection to this proxy (plain HTTP, not encrypted), and then makes a request to connect to the VPN host as given on the command line -or in the config file. The proxy is supposed to forward any subsequent packets +or in the configuration file. The proxy is supposed to forward any subsequent packets transparently to the VPN host, so that the TLS layer of the connection effectively is established between the client and the VPN host, and the proxy just acts as a forwarding instance on the lower level of the TCP connection. @@ -279,13 +279,14 @@ available. On FreeBSD \fB\-\-ppp\-system\fR is available instead. .SH CONFIGURATION Options can be taken from a configuration file. Options passed in the command -line will override those from the config file, though. The default config file -is @SYSCONFDIR@/openfortivpn/config, but this can be set using the \fB\-c\fR option. -An empty template for the config file is installed to +line will override those from the configuration file, though. The default +configuration file is @SYSCONFDIR@/openfortivpn/config, but this can be set +using the \fB\-c\fR option. +An empty template for the configuration file is installed to @DATADIR@/config.template .TP -A config file looks like: +A configuration file looks like: # this is a comment .br host = vpn\-gateway @@ -306,7 +307,7 @@ username = foo .br # realm = some-realm .br -# useful for a gui that passes a config file to openfortivpn +# useful for a gui that passes a configuration file to openfortivpn .br # otp = 123456 .br diff --git a/etc/openfortivpn/config.template b/etc/openfortivpn/config.template index ac2be583..0c7cd349 100644 --- a/etc/openfortivpn/config.template +++ b/etc/openfortivpn/config.template @@ -1,4 +1,4 @@ -### config file for openfortivpn, see man openfortivpn(1) ### +### configuration file for openfortivpn, see man openfortivpn(1) ### host = vpn.example.org port = 443 diff --git a/src/config.c b/src/config.c index d30e7a63..098ea933 100644 --- a/src/config.c +++ b/src/config.c @@ -224,7 +224,7 @@ int load_config(struct vpn_config *cfg, const char *filename) // Expect something like: "key = value" equals = strchr(line, '='); if (equals == NULL) { - log_warn("Bad line in config file: \"%s\".\n", line); + log_warn("Bad line in configuration file: \"%s\".\n", line); continue; } equals[0] = '\0'; @@ -257,7 +257,7 @@ int load_config(struct vpn_config *cfg, const char *filename) unsigned long port = strtoul(val, NULL, 0); if (port == 0 || port > 65535) { - log_warn("Bad port in config file: \"%lu\".\n", + log_warn("Bad port in configuration file: \"%lu\".\n", port); continue; } @@ -279,7 +279,7 @@ int load_config(struct vpn_config *cfg, const char *filename) long otp_delay = strtol(val, NULL, 0); if (otp_delay < 0 || otp_delay > UINT_MAX) { - log_warn("Bad value for otp-delay in config file: \"%s\".\n", + log_warn("Bad value for otp-delay in configuration file: \"%s\".\n", val); continue; } @@ -288,7 +288,7 @@ int load_config(struct vpn_config *cfg, const char *filename) int no_ftm_push = strtob(val); if (no_ftm_push < 0) { - log_warn("Bad no-ftm-push in config file: \"%s\".\n", + log_warn("Bad no-ftm-push in configuration file: \"%s\".\n", val); continue; } @@ -303,7 +303,7 @@ int load_config(struct vpn_config *cfg, const char *filename) int set_dns = strtob(val); if (set_dns < 0) { - log_warn("Bad set-dns in config file: \"%s\".\n", + log_warn("Bad set-dns in configuration file: \"%s\".\n", val); continue; } @@ -312,7 +312,7 @@ int load_config(struct vpn_config *cfg, const char *filename) int set_routes = strtob(val); if (set_routes < 0) { - log_warn("Bad set-routes in config file: \"%s\".\n", + log_warn("Bad set-routes in configuration file: \"%s\".\n", val); continue; } @@ -321,7 +321,7 @@ int load_config(struct vpn_config *cfg, const char *filename) int half_internet_routes = strtob(val); if (half_internet_routes < 0) { - log_warn("Bad half-internet-routes in config file: \"%s\".\n", + log_warn("Bad half-internet-routes in configuration file: \"%s\".\n", val); continue; } @@ -330,7 +330,7 @@ int load_config(struct vpn_config *cfg, const char *filename) unsigned long persistent = strtoul(val, NULL, 0); if (persistent > UINT_MAX) { - log_warn("Bad value for persistent in config file: \"%s\".\n", + log_warn("Bad value for persistent in configuration file: \"%s\".\n", val); continue; } @@ -340,7 +340,7 @@ int load_config(struct vpn_config *cfg, const char *filename) int pppd_use_peerdns = strtob(val); if (pppd_use_peerdns < 0) { - log_warn("Bad pppd-use-peerdns in config file: \"%s\".\n", + log_warn("Bad pppd-use-peerdns in configuration file: \"%s\".\n", val); continue; } @@ -375,7 +375,7 @@ int load_config(struct vpn_config *cfg, const char *filename) int use_resolvconf = strtob(val); if (use_resolvconf < 0) { - log_warn("Bad use-resolvconf value in config file: \"%s\".\n", + log_warn("Bad use-resolvconf value in configuration file: \"%s\".\n", val); continue; } @@ -387,14 +387,14 @@ int load_config(struct vpn_config *cfg, const char *filename) int use_syslog = strtob(val); if (use_syslog < 0) { - log_warn("Bad use-syslog in config file: \"%s\".\n", + log_warn("Bad use-syslog in configuration file: \"%s\".\n", val); continue; } cfg->use_syslog = use_syslog; } else if (strcmp(key, "trusted-cert") == 0) { if (strlen(val) != SHA256STRLEN - 1) { - log_warn("Bad certificate sha256 digest in config file: \"%s\".\n", + log_warn("Bad certificate sha256 digest in configuration file: \"%s\".\n", val); continue; } @@ -420,7 +420,7 @@ int load_config(struct vpn_config *cfg, const char *filename) int insecure_ssl = strtob(val); if (insecure_ssl < 0) { - log_warn("Bad insecure-ssl in config file: \"%s\".\n", + log_warn("Bad insecure-ssl in configuration file: \"%s\".\n", val); continue; } @@ -433,7 +433,7 @@ int load_config(struct vpn_config *cfg, const char *filename) int min_tls = parse_min_tls(val); if (min_tls == -1) { - log_warn("Bad min-tls in config file: \"%s\".\n", + log_warn("Bad min-tls in configuration file: \"%s\".\n", val); continue; } else { @@ -444,7 +444,7 @@ int load_config(struct vpn_config *cfg, const char *filename) int seclevel_1 = strtob(val); if (seclevel_1 < 0) { - log_warn("Bad seclevel-1 in config file: \"%s\".\n", + log_warn("Bad seclevel-1 in configuration file: \"%s\".\n", val); continue; } @@ -459,7 +459,7 @@ int load_config(struct vpn_config *cfg, const char *filename) free(cfg->check_virtual_desktop); cfg->check_virtual_desktop = strdup(val); } else { - log_warn("Bad key in config file: \"%s\".\n", key); + log_warn("Bad key in configuration file: \"%s\".\n", key); goto err_free; } } diff --git a/src/main.c b/src/main.c index 1ba130a2..9adfa018 100644 --- a/src/main.c +++ b/src/main.c @@ -108,7 +108,7 @@ PPPD_USAGE \ "Options:\n" \ " -h --help Show this help message and exit.\n" \ " --version Show version and exit.\n" \ -" -c , --config= Specify a custom config file (default:\n" \ +" -c , --config= Specify a custom configuration file (default:\n" \ " " SYSCONFDIR "/openfortivpn/config).\n" \ " -u , --username= VPN account username.\n" \ " -p , --password= VPN account password.\n" \ @@ -168,12 +168,12 @@ PPPD_USAGE \ #define help_config \ "\n" \ -"Config file:\n" \ +"Configuration file:\n" \ " Options can be taken from a configuration file. Options passed in the\n" \ -" command line will override those from the config file, though. The default\n" \ -" config file is " SYSCONFDIR "/openfortivpn/config,\n" \ +" command line will override those from the configuration file, though. The\n" \ +" default configuration file is " SYSCONFDIR "/openfortivpn/config,\n" \ " but this can be set using the -c option.\n" \ -" A simple config file example looks like:\n" \ +" A simple configuration file example looks like:\n" \ " # this is a comment\n" \ " host = vpn-gateway\n" \ " port = 8443\n" \ @@ -181,7 +181,7 @@ PPPD_USAGE \ " password = bar\n" \ " trusted-cert = certificatedigest4daa8c5fe6c...\n" \ " trusted-cert = othercertificatedigest6631bf...\n" \ -" For a full-featured config see man openfortivpn(1).\n" +" For a full-featured configuration see man openfortivpn(1).\n" int main(int argc, char **argv) { @@ -549,7 +549,7 @@ int main(int argc, char **argv) goto user_error; if (cli_cfg.password[0] != '\0') - log_warn("You should not pass the password on the command line. Type it interactively or use a config file instead.\n"); + log_warn("You should not pass the password on the command line. Type it interactively or use a configuration file instead.\n"); log_debug_all("ATTENTION: the output contains sensitive information such as the THE CLEAR TEXT PASSWORD.\n"); @@ -557,13 +557,13 @@ int main(int argc, char **argv) if (strcmp(&REVISION[1], VERSION)) log_debug("revision " REVISION "\n"); - // Load config file + // Load configuration file if (config_file[0] != '\0') { ret = load_config(&cfg, config_file); if (ret == 0) - log_debug("Loaded config file \"%s\".\n", config_file); + log_debug("Loaded configuration file \"%s\".\n", config_file); else - log_warn("Could not load config file \"%s\" (%s).\n", + log_warn("Could not load configuration file \"%s\" (%s).\n", config_file, err_cfg_str(ret)); } if (cli_cfg.password_set) { @@ -571,14 +571,14 @@ int main(int argc, char **argv) log_debug("Disabled password due to empty command-line option\n"); } else if (cfg.password_set) { if (cfg.password[0] == '\0') - log_debug("Disabled password due to empty entry in config file \"%s\"\n", + log_debug("Disabled password due to empty entry in configuration file \"%s\"\n", config_file); else - log_debug("Loaded password from config file \"%s\"\n", + log_debug("Loaded password from configuration file \"%s\"\n", config_file); } - // Then apply CLI config + // Then apply CLI configuration merge_config(&cfg, &cli_cfg); set_syslog(cfg.use_syslog); @@ -617,12 +617,12 @@ int main(int argc, char **argv) "password", "VPN account password: ", cfg.password, PASSWORD_SIZE); } - log_debug("Config host = \"%s\"\n", cfg.gateway_host); - log_debug("Config realm = \"%s\"\n", cfg.realm); - log_debug("Config port = \"%d\"\n", cfg.gateway_port); + log_debug("Configuration host = \"%s\"\n", cfg.gateway_host); + log_debug("Configuration realm = \"%s\"\n", cfg.realm); + log_debug("Configuration port = \"%d\"\n", cfg.gateway_port); if (cfg.username[0] != '\0') - log_debug("Config username = \"%s\"\n", cfg.username); - log_debug_all("Config password = \"%s\"\n", cfg.password); + log_debug("Configuration username = \"%s\"\n", cfg.username); + log_debug_all("Configuration password = \"%s\"\n", cfg.password); if (cfg.otp[0] != '\0') log_debug("One-time password = \"%s\"\n", cfg.otp); diff --git a/src/tunnel.c b/src/tunnel.c index a07d76a1..9c061097 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -922,7 +922,7 @@ static int ssl_verify_cert(struct tunnel *tunnel) log_error("Gateway certificate validation failed, and the certificate digest is not in the local whitelist. If you trust it, rerun with:\n"); log_error(" --trusted-cert %s\n", digest_str); - log_error("or add this line to your config file:\n"); + log_error("or add this line to your configuration file:\n"); log_error(" trusted-cert = %s\n", digest_str); log_error("Gateway certificate:\n"); log_error(" subject:\n"); @@ -1348,7 +1348,7 @@ int run_tunnel(struct vpn_config *config) log_info("Logged out.\n"); } - // explicitly free the buffer allocated for split routes of the ipv4 config + // explicitly free the buffer allocated for split routes of the ipv4 configuration if (tunnel.ipv4.split_rt != NULL) { free(tunnel.ipv4.split_rt); tunnel.ipv4.split_rt = NULL; From 9af3ba30f606d1861734c1cc2346b2a07f393f85 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 26 Mar 2021 09:33:41 +0100 Subject: [PATCH 30/76] Additional sources of information --- doc/openfortivpn.1.in | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/openfortivpn.1.in b/doc/openfortivpn.1.in index 23b9f8ad..83f0faa9 100644 --- a/doc/openfortivpn.1.in +++ b/doc/openfortivpn.1.in @@ -378,3 +378,10 @@ cipher\-list = HIGH:!aNULL:!kRSA:!PSK:!SRP:!MD5:!RC4 persistent = 0 .br seclevel-1 = 0 + +.SH SEE ALSO + +The \fBopenfortivpn\fR home page +(\fIhttps://github.com/adrienverge/openfortivpn\fR) +provides a short introduction in the \fBREADME\fR file and additional +information under the \fBWiki\fR tab. From 0ab2e1c19ea59a9c1f5904c6186c45a5c664cb98 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Tue, 30 Mar 2021 10:36:02 +0200 Subject: [PATCH 31/76] Remove unused functions from AC_CHECK_FUNCS * While the malloc() and realloc() functions are being called, the AC_FUNC_MALLOC and AC_FUNC_REALLOC macros have already been checking if these functions are available and compatible with the GNU C library functions. * Also, optarg and optind are not functions - they are documented to be external variables. --- configure.ac | 6 ------ 1 file changed, 6 deletions(-) diff --git a/configure.ac b/configure.ac index 977b2c1d..ad9c368b 100644 --- a/configure.ac +++ b/configure.ac @@ -120,7 +120,6 @@ fileno \ fopen \ forkpty \ fprintf \ -fputc \ fputs \ fread \ free \ @@ -142,7 +141,6 @@ ioctl \ isatty \ isdigit \ isspace \ -malloc \ memcmp \ memcpy \ memmem \ @@ -151,8 +149,6 @@ memset \ ntohs \ open \ openlog \ -optarg \ -optind \ pclose \ popen \ printf \ @@ -169,10 +165,8 @@ pthread_mutex_lock \ pthread_mutex_unlock \ pthread_self \ pthread_sigmask \ -putchar \ puts \ read \ -realloc \ rewind \ select \ sem_destroy \ From 0f712fa9c1d08b045b11e5db9a839322c8859c22 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sun, 11 Apr 2021 10:42:16 +0200 Subject: [PATCH 32/76] No need to check for C standard library functions I have removed these C89 functions: isdigit isspace signal fclose fflush fopen fprintf fputs fread freopen fwrite getchar printf puts rewind sprintf sscanf vprintf atoi exit free getenv strtol strtoul system memcmp memcpy memmove memset strcasestr strcat strchr strcmp strcpy strerror strlen strncat strncmp strncpy strstr I have also removed these C99 functions: snprintf vsnprintf The rationale is that it is hard to keep track of standard C library functions used in our code. Hare are examples of C89 functions that were not checked: isalnum isxdigit strerror perror abort --- configure.ac | 42 ------------------------------------------ 1 file changed, 42 deletions(-) diff --git a/configure.ac b/configure.ac index ad9c368b..fb232c7c 100644 --- a/configure.ac +++ b/configure.ac @@ -107,30 +107,17 @@ AC_FUNC_MALLOC AC_FUNC_REALLOC AC_CHECK_FUNCS([ \ access \ -atoi \ close \ connect \ execv \ -exit \ _exit \ -fclose \ fcntl \ -fflush \ fileno \ -fopen \ forkpty \ -fprintf \ -fputs \ -fread \ -free \ freeaddrinfo \ freeifaddrs \ -freopen \ -fwrite \ gai_strerror \ getaddrinfo \ -getchar \ -getenv \ geteuid \ getifaddrs \ getopt_long \ @@ -139,19 +126,12 @@ inet_addr \ inet_ntoa \ ioctl \ isatty \ -isdigit \ -isspace \ -memcmp \ -memcpy \ memmem \ -memmove \ -memset \ ntohs \ open \ openlog \ pclose \ popen \ -printf \ pthread_cancel \ pthread_cond_init \ pthread_cond_signal \ @@ -165,9 +145,7 @@ pthread_mutex_lock \ pthread_mutex_unlock \ pthread_self \ pthread_sigmask \ -puts \ read \ -rewind \ select \ sem_destroy \ sem_init \ @@ -177,37 +155,17 @@ setenv \ setsockopt \ sigaddset \ sigemptyset \ -signal \ sleep \ -snprintf \ socket \ -sprintf \ -sscanf \ strcasecmp \ -strcasestr \ -strcat \ -strchr \ -strcmp \ -strcpy \ strdup \ -strerror \ -strlen \ strncasecmp \ -strncat \ -strncmp \ -strncpy \ strsignal \ -strstr \ strtok_r \ -strtol \ -strtoul \ syslog \ -system \ tcgetattr \ tcsetattr \ usleep \ -vprintf \ -vsnprintf \ vsyslog \ waitpid \ write \ From 289354ded07b98d615164bc0e1308d081f5f205e Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sun, 11 Apr 2021 11:00:36 +0200 Subject: [PATCH 33/76] =?UTF-8?q?atoi=20=E2=86=92=20strtol?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #882. --- src/http.c | 2 +- src/userinput.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/http.c b/src/http.c index 58e8fc7a..f9f2fc3b 100644 --- a/src/http.c +++ b/src/http.c @@ -197,7 +197,7 @@ int http_receive(struct tunnel *tunnel, header_size); if (header) - content_size = atoi(header); + content_size = strtol(header, NULL, 10); if (find_header(buffer, "Transfer-Encoding: chunked", diff --git a/src/userinput.c b/src/userinput.c index a0dc4b87..121e740d 100644 --- a/src/userinput.c +++ b/src/userinput.c @@ -143,7 +143,7 @@ static int pinentry_read(int from, char **retstr) } if (strncmp(buf, "ERR ", 4) == 0 || strncmp(buf, "S ERROR", 7) == 0) { - ret = atoi(&buf[4]); + ret = strtol(&buf[4], NULL, 10); if (!ret) ret = -1; if (retstr) { From 64dda9b47e06693c02a6b514e1a464a7143dec0c Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 26 Apr 2021 09:50:41 +0200 Subject: [PATCH 34/76] Properly escape non-ASCII characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now UTF-8 encoded character 'à' was escaped to: %FFFFFFC3%FFFFFFA9 but it should be escaped to: %C3%A9 The nice thing with the current uri_unescape() function is that it works with both mappings,so we should be able to handle both mappings transparently and avoid compatibility issues. Note that we should make sure strings are encoded to UTF-8 before escaping them. However, I won't enforce that here because: 1. UTF-8 should already be the default encoding on supported platforms, 2. changing the mapping might cause compatibility issues, 3. it's OK if escaping and unescaping are consistently wrong. --- src/userinput.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/userinput.c b/src/userinput.c index 121e740d..8eac79ba 100644 --- a/src/userinput.c +++ b/src/userinput.c @@ -55,7 +55,8 @@ static char *uri_escape(const char *string) if (isalnum(string[i])) escaped[real_len++] = string[i]; else - real_len += sprintf(&escaped[real_len], "%%%02X", string[i]); + real_len += sprintf(&escaped[real_len], "%%%02X", + (unsigned char)string[i]); } if (escaped) escaped[real_len] = '\0'; From f96cddbaab91740fae0c28b85082ffc5bde0b814 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 26 Apr 2021 10:02:04 +0200 Subject: [PATCH 35/76] Properly escape non-ASCII characters Unreserved characters need not be escaped: - _ . ~ --- src/userinput.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/userinput.c b/src/userinput.c index 8eac79ba..d928a4d1 100644 --- a/src/userinput.c +++ b/src/userinput.c @@ -52,7 +52,8 @@ static char *uri_escape(const char *string) } escaped = tmp; } - if (isalnum(string[i])) + if (isalnum(string[i]) || string[i] == '-' || string[i] == '_' || + string[i] == '.' || string[i] == '~') escaped[real_len++] = string[i]; else real_len += sprintf(&escaped[real_len], "%%%02X", From f307912d8c8f3d4b6248a0eebc91b205fc2a671f Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 26 Apr 2021 10:05:37 +0200 Subject: [PATCH 36/76] Align URI escaping functions Attempt to use similar code in uri_escape() in userinput.c and url_encode() in http.c. --- src/userinput.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/userinput.c b/src/userinput.c index d928a4d1..ed0c5aaa 100644 --- a/src/userinput.c +++ b/src/userinput.c @@ -37,9 +37,8 @@ static char *uri_escape(const char *string) char *escaped = NULL; int allocated_len = 0; int real_len = 0; - int i; - for (i = 0; string[i]; i++) { + while (*string != '\0') { if (allocated_len + 4 >= real_len) { allocated_len += 16; char *tmp = realloc(escaped, allocated_len); @@ -52,12 +51,13 @@ static char *uri_escape(const char *string) } escaped = tmp; } - if (isalnum(string[i]) || string[i] == '-' || string[i] == '_' || - string[i] == '.' || string[i] == '~') - escaped[real_len++] = string[i]; + if (isalnum(*string) || *string == '-' || *string == '_' || + *string == '.' || *string == '~') + escaped[real_len++] = *string; else real_len += sprintf(&escaped[real_len], "%%%02X", - (unsigned char)string[i]); + (unsigned char)*string); + string++; } if (escaped) escaped[real_len] = '\0'; From d5ac4d6b385b94626325f69f52819dd24c1b2ed4 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 26 Apr 2021 20:46:39 +0200 Subject: [PATCH 37/76] Fix buffer overflow in URI escaping function --- src/userinput.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/userinput.c b/src/userinput.c index ed0c5aaa..8b5bff42 100644 --- a/src/userinput.c +++ b/src/userinput.c @@ -39,7 +39,7 @@ static char *uri_escape(const char *string) int real_len = 0; while (*string != '\0') { - if (allocated_len + 4 >= real_len) { + if (allocated_len < real_len + 4) { allocated_len += 16; char *tmp = realloc(escaped, allocated_len); From de0b9c9ee226f62df2bd1c40a6fb0618b058bfc1 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Tue, 27 Apr 2021 08:16:47 +0200 Subject: [PATCH 38/76] Revert 89615fc While it makes sense to use CFLAGS consistently, OPENSSL_CFLAGS and LIBSYSTEMD_CFLAGS define header inclusion paths, therefore they should be appended to CPPFLAGS. --- Makefile.am | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 810d007e..41fc6e6d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -12,8 +12,9 @@ openfortivpn_CPPFLAGS = -DSYSCONFDIR=\"$(sysconfdir)\" \ -DPPP_PATH=\"@PPP_PATH@\" \ -DNETSTAT_PATH=\"@NETSTAT_PATH@\" \ -DRESOLVCONF_PATH=\"@RESOLVCONF_PATH@\" \ - -DREVISION=\"@REVISION@\" -openfortivpn_CFLAGS = -Wall -pedantic $(OPENSSL_CFLAGS) $(LIBSYSTEMD_CFLAGS) + -DREVISION=\"@REVISION@\" \ + $(OPENSSL_CFLAGS) $(LIBSYSTEMD_CFLAGS) +openfortivpn_CFLAGS = -Wall -pedantic openfortivpn_LDADD = $(OPENSSL_LIBS) $(LIBSYSTEMD_LIBS) PATHFILES = From cbd9a00ff23c702f74949148c8a194e46a040b17 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Tue, 27 Apr 2021 08:25:37 +0200 Subject: [PATCH 39/76] Update checkpatch.pl from Linux kernel --- src/io.c | 2 +- src/log.c | 4 +- tests/ci/checkpatch/checkpatch.pl | 297 ++++++++++++++++++++++-------- tests/ci/checkpatch/spelling.txt | 30 +++ 4 files changed, 254 insertions(+), 79 deletions(-) diff --git a/src/io.c b/src/io.c index 577623b4..124c7ccc 100644 --- a/src/io.c +++ b/src/io.c @@ -116,7 +116,7 @@ static void destroy_ssl_locks(void) #endif // global variable to pass signal out of its handler -volatile sig_atomic_t sig_received = 0; +volatile sig_atomic_t sig_received; //static variables are initialized to zero in C99 int get_sig_received(void) { diff --git a/src/log.c b/src/log.c index e6c234e0..0d22c954 100644 --- a/src/log.c +++ b/src/log.c @@ -28,11 +28,11 @@ #include static pthread_mutex_t mutex; -static int do_syslog; //static variables of arithmetic type are initialized to zero in C99 +static int do_syslog; //static variables are initialized to zero in C99 enum log_verbosity loglevel; -static int is_a_tty; // static variables of arithmetic type are initialized to zero in C99 +static int is_a_tty; // static variables are initialized to zero in C99 struct log_param_s { const char *prefix; diff --git a/tests/ci/checkpatch/checkpatch.pl b/tests/ci/checkpatch/checkpatch.pl index 1afe3af1..f42e5ba1 100755 --- a/tests/ci/checkpatch/checkpatch.pl +++ b/tests/ci/checkpatch/checkpatch.pl @@ -23,6 +23,9 @@ use Getopt::Long qw(:config no_auto_abbrev); my $quiet = 0; +my $verbose = 0; +my %verbose_messages = (); +my %verbose_emitted = (); my $tree = 1; my $chk_signoff = 1; my $chk_patch = 1; @@ -61,6 +64,7 @@ my $codespell = 0; my $codespellfile = "/usr/share/codespell/dictionary.txt"; my $conststructsfile = "$D/const_structs.checkpatch"; +my $docsfile = "$D/../Documentation/dev-tools/checkpatch.rst"; my $typedefsfile; my $color = "auto"; my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE @@ -78,6 +82,7 @@ sub help { Options: -q, --quiet quiet + -v, --verbose verbose mode --no-tree run without a kernel tree --no-signoff do not check for 'Signed-off-by' line --patch treat FILE as patchfile (default) @@ -158,15 +163,51 @@ sub list_types { my $text = <$script>; close($script); - my @types = (); + my %types = (); # Also catch when type or level is passed through a variable - for ($text =~ /(?:(?:\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) { - push (@types, $_); + while ($text =~ /(?:(\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) { + if (defined($1)) { + if (exists($types{$2})) { + $types{$2} .= ",$1" if ($types{$2} ne $1); + } else { + $types{$2} = $1; + } + } else { + $types{$2} = "UNDETERMINED"; + } } - @types = sort(uniq(@types)); + print("#\tMessage type\n\n"); - foreach my $type (@types) { + if ($color) { + print(" ( Color coding: "); + print(RED . "ERROR" . RESET); + print(" | "); + print(YELLOW . "WARNING" . RESET); + print(" | "); + print(GREEN . "CHECK" . RESET); + print(" | "); + print("Multiple levels / Undetermined"); + print(" )\n\n"); + } + + foreach my $type (sort keys %types) { + my $orig_type = $type; + if ($color) { + my $level = $types{$type}; + if ($level eq "ERROR") { + $type = RED . $type . RESET; + } elsif ($level eq "WARN") { + $type = YELLOW . $type . RESET; + } elsif ($level eq "CHK") { + $type = GREEN . $type . RESET; + } + } print(++$count . "\t" . $type . "\n"); + if ($verbose && exists($verbose_messages{$orig_type})) { + my $message = $verbose_messages{$orig_type}; + $message =~ s/\n/\n\t/g; + print("\t" . $message . "\n\n"); + } } exit($exitcode); @@ -198,6 +239,46 @@ sub list_types { unshift(@ARGV, @conf_args) if @conf_args; } +sub load_docs { + open(my $docs, '<', "$docsfile") + or warn "$P: Can't read the documentation file $docsfile $!\n"; + + my $type = ''; + my $desc = ''; + my $in_desc = 0; + + while (<$docs>) { + chomp; + my $line = $_; + $line =~ s/\s+$//; + + if ($line =~ /^\s*\*\*(.+)\*\*$/) { + if ($desc ne '') { + $verbose_messages{$type} = trim($desc); + } + $type = $1; + $desc = ''; + $in_desc = 1; + } elsif ($in_desc) { + if ($line =~ /^(?:\s{4,}|$)/) { + $line =~ s/^\s{4}//; + $desc .= $line; + $desc .= "\n"; + } else { + $verbose_messages{$type} = trim($desc); + $type = ''; + $desc = ''; + $in_desc = 0; + } + } + } + + if ($desc ne '') { + $verbose_messages{$type} = trim($desc); + } + close($docs); +} + # Perl's Getopt::Long allows options to take optional arguments after a space. # Prevent --color by itself from consuming other arguments foreach (@ARGV) { @@ -208,6 +289,7 @@ sub list_types { GetOptions( 'q|quiet+' => \$quiet, + 'v|verbose!' => \$verbose, 'tree!' => \$tree, 'signoff!' => \$chk_signoff, 'patch!' => \$chk_patch, @@ -247,13 +329,27 @@ sub list_types { help(0) if ($help); +die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix)); +die "$P: --verbose cannot be used with --terse\n" if ($verbose && $terse); + +if ($color =~ /^[01]$/) { + $color = !$color; +} elsif ($color =~ /^always$/i) { + $color = 1; +} elsif ($color =~ /^never$/i) { + $color = 0; +} elsif ($color =~ /^auto$/i) { + $color = (-t STDOUT); +} else { + die "$P: Invalid color mode: $color\n"; +} + +load_docs() if ($verbose); list_types(0) if ($list_types); $fix = 1 if ($fix_inplace); $check_orig = $check; -die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix)); - my $exit = 0; my $perl_version_ok = 1; @@ -268,18 +364,6 @@ sub list_types { push(@ARGV, '-'); } -if ($color =~ /^[01]$/) { - $color = !$color; -} elsif ($color =~ /^always$/i) { - $color = 1; -} elsif ($color =~ /^never$/i) { - $color = 0; -} elsif ($color =~ /^auto$/i) { - $color = (-t STDOUT); -} else { - die "$P: Invalid color mode: $color\n"; -} - # skip TAB size 1 to avoid additional checks on $tabsize - 1 die "$P: Invalid TAB size: $tabsize\n" if ($tabsize < 2); @@ -382,6 +466,7 @@ sub hash_show_words { # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ const| + volatile| __percpu| __nocast| __safe| @@ -486,7 +571,7 @@ sub hash_show_words { our $allocFunctions = qr{(?x: (?:(?:devm_)? - (?:kv|k|v)[czm]alloc(?:_node|_array)? | + (?:kv|k|v)[czm]alloc(?:_array)?(?:_node)? | kstrdup(?:_const)? | kmemdup(?:_nul)?) | (?:\w+)?alloc_skb(?:_ip_align)? | @@ -506,6 +591,30 @@ sub hash_show_words { Cc: )}; +our $tracing_logging_tags = qr{(?xi: + [=-]*> | + <[=-]* | + \[ | + \] | + start | + called | + entered | + entry | + enter | + in | + inside | + here | + begin | + exit | + end | + done | + leave | + completed | + out | + return | + [\.\!:\s]* +)}; + sub edit_distance_min { my (@arr) = @_; my $len = scalar @arr; @@ -2184,7 +2293,16 @@ sub report { splice(@lines, 1, 1); $output = join("\n", @lines); } - $output = (split('\n', $output))[0] . "\n" if ($terse); + + if ($terse) { + $output = (split('\n', $output))[0] . "\n"; + } + + if ($verbose && exists($verbose_messages{$type}) && + !exists($verbose_emitted{$type})) { + $output .= $verbose_messages{$type} . "\n\n"; + $verbose_emitted{$type} = 1; + } push(our @report, $output); @@ -2428,6 +2546,15 @@ sub get_raw_comment { return $comment; } +sub exclude_global_initialisers { + my ($realfile) = @_; + + # Do not check for BPF programs (tools/testing/selftests/bpf/progs/*.c, samples/bpf/*_kern.c, *.bpf.c). + return $realfile =~ m@^tools/testing/selftests/bpf/progs/.*\.c$@ || + $realfile =~ m@^samples/bpf/.*_kern\.c$@ || + $realfile =~ m@/bpf/.*\.bpf\.c$@; +} + sub process { my $filename = shift; @@ -2973,7 +3100,7 @@ sub process { } if (!defined $lines[$linenr]) { WARN("BAD_SIGN_OFF", - "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline); + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline); } elsif ($rawlines[$linenr] !~ /^\s*signed-off-by:\s*(.*)/i) { WARN("BAD_SIGN_OFF", "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); @@ -2996,8 +3123,8 @@ sub process { if (ERROR("GERRIT_CHANGE_ID", "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr) && $fix) { - fix_delete_line($fixlinenr, $rawline); - } + fix_delete_line($fixlinenr, $rawline); + } } # Check if the commit log is in a possible stack dump @@ -3239,10 +3366,10 @@ sub process { next if ($start_char =~ /^\S$/); next if (index(" \t.,;?!", $end_char) == -1); - # avoid repeating hex occurrences like 'ff ff fe 09 ...' - if ($first =~ /\b[0-9a-f]{2,}\b/i) { - next if (!exists($allow_repeated_words{lc($first)})); - } + # avoid repeating hex occurrences like 'ff ff fe 09 ...' + if ($first =~ /\b[0-9a-f]{2,}\b/i) { + next if (!exists($allow_repeated_words{lc($first)})); + } if (WARN("REPEATED_WORD", "Possible repeated word: '$first'\n" . $herecurr) && @@ -3574,6 +3701,13 @@ sub process { } } +# check for .L prefix local symbols in .S files + if ($realfile =~ /\.S$/ && + $line =~ /^\+\s*(?:[A-Z]+_)?SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) { + WARN("AVOID_L_PREFIX", + "Avoid using '.L' prefixed local symbol names for denoting a range of code via 'SYM_*_START/END' annotations; see Documentation/asm-annotations.rst\n" . $herecurr); + } + # check we are in a valid source file C or perl if not then ignore this hunk next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/); @@ -3776,43 +3910,48 @@ sub process { } # check for missing blank lines after declarations - if ($sline =~ /^\+\s+\S/ && #Not at char 1 - # actual declarations - ($prevline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || +# (declarations must have the same indentation and not be at the start of line) + if (($prevline =~ /\+(\s+)\S/) && $sline =~ /^\+$1\S/) { + # use temporaries + my $sl = $sline; + my $pl = $prevline; + # remove $Attribute/$Sparse uses to simplify comparisons + $sl =~ s/\b(?:$Attribute|$Sparse)\b//g; + $pl =~ s/\b(?:$Attribute|$Sparse)\b//g; + if (($pl =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || # function pointer declarations - $prevline =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || + $pl =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || # foo bar; where foo is some local typedef or #define - $prevline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || + $pl =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || # known declaration macros - $prevline =~ /^\+\s+$declaration_macros/) && + $pl =~ /^\+\s+$declaration_macros/) && # for "else if" which can look like "$Ident $Ident" - !($prevline =~ /^\+\s+$c90_Keywords\b/ || + !($pl =~ /^\+\s+$c90_Keywords\b/ || # other possible extensions of declaration lines - $prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || + $pl =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || # not starting a section or a macro "\" extended line - $prevline =~ /(?:\{\s*|\\)$/) && + $pl =~ /(?:\{\s*|\\)$/) && # looks like a declaration - !($sline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || + !($sl =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || # function pointer declarations - $sline =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || + $sl =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || # foo bar; where foo is some local typedef or #define - $sline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || + $sl =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || # known declaration macros - $sline =~ /^\+\s+$declaration_macros/ || + $sl =~ /^\+\s+$declaration_macros/ || # start of struct or union or enum - $sline =~ /^\+\s+(?:static\s+)?(?:const\s+)?(?:union|struct|enum|typedef)\b/ || + $sl =~ /^\+\s+(?:static\s+)?(?:const\s+)?(?:union|struct|enum|typedef)\b/ || # start or end of block or continuation of declaration - $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ || + $sl =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ || # bitfield continuation - $sline =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ || + $sl =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ || # other possible extensions of declaration lines - $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/) && - # indentation of previous and current line are the same - (($prevline =~ /\+(\s+)\S/) && $sline =~ /^\+$1\S/)) { - if (WARN("LINE_SPACING", - "Missing a blank line after declarations\n" . $hereprev) && - $fix) { - fix_insert_line($fixlinenr, "\+"); + $sl =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/)) { + if (WARN("LINE_SPACING", + "Missing a blank line after declarations\n" . $hereprev) && + $fix) { + fix_insert_line($fixlinenr, "\+"); + } } } @@ -4283,8 +4422,7 @@ sub process { if (defined $realline_next && exists $lines[$realline_next - 1] && !defined $suppress_export{$realline_next} && - ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/ || - $lines[$realline_next - 1] =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { + ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/)) { # Handle definitions which produce identifiers with # a prefix: # XXX(foo); @@ -4311,8 +4449,7 @@ sub process { } if (!defined $suppress_export{$linenr} && $prevline =~ /^.\s*$/ && - ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ || - $line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { + ($line =~ /EXPORT_SYMBOL.*\((.*)\)/)) { #print "FOO B <$lines[$linenr - 1]>\n"; $suppress_export{$linenr} = 2; } @@ -4323,7 +4460,8 @@ sub process { } # check for global initialisers. - if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/) { + if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/ && + !exclude_global_initialisers($realfile)) { if (ERROR("GLOBAL_INITIALISERS", "do not initialise globals to $1\n" . $herecurr) && $fix) { @@ -4419,7 +4557,7 @@ sub process { WARN("STATIC_CONST_CHAR_ARRAY", "char * array declaration might be better as static const\n" . $herecurr); - } + } # check for sizeof(foo)/sizeof(foo[0]) that could be ARRAY_SIZE(foo) if ($line =~ m@\bsizeof\s*\(\s*($Lval)\s*\)@) { @@ -5009,7 +5147,7 @@ sub process { # A colon needs no spaces before when it is # terminating a case value or a label. } elsif ($opv eq ':C' || $opv eq ':L') { - if ($ctx =~ /Wx./) { + if ($ctx =~ /Wx./ and $realfile !~ m@.*\.lds\.h$@) { if (ERROR("SPACING", "space prohibited before that '$op' $at\n" . $hereptr)) { $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]); @@ -5272,7 +5410,7 @@ sub process { $lines[$linenr - 3] !~ /^[ +]\s*$Ident\s*:/) { WARN("RETURN_VOID", "void function return statements are not generally useful\n" . $hereprev); - } + } # if statements using unnecessary parentheses - ie: if ((foo == bar)) if ($perl_version_ok && @@ -5968,6 +6106,17 @@ sub process { "Prefer using '\"%s...\", __func__' to using '$context_function', this function's name, in a string\n" . $herecurr); } +# check for unnecessary function tracing like uses +# This does not use $logFunctions because there are many instances like +# 'dprintk(FOO, "%s()\n", __func__);' which do not match $logFunctions + if ($rawline =~ /^\+.*\([^"]*"$tracing_logging_tags{0,3}%s(?:\s*\(\s*\)\s*)?$tracing_logging_tags{0,3}(?:\\n)?"\s*,\s*__func__\s*\)\s*;/) { + if (WARN("TRACING_LOGGING", + "Unnecessary ftrace-like logging - prefer using ftrace\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } + } + # check for spaces before a quoted newline if ($rawline =~ /^.*\".*\s\\n/) { if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE", @@ -6479,18 +6628,18 @@ sub process { if ($line =~ /(\(\s*$C90_int_types\s*\)\s*)($Constant)\b/) { my $cast = $1; my $const = $2; + my $suffix = ""; + my $newconst = $const; + $newconst =~ s/${Int_type}$//; + $suffix .= 'U' if ($cast =~ /\bunsigned\b/); + if ($cast =~ /\blong\s+long\b/) { + $suffix .= 'LL'; + } elsif ($cast =~ /\blong\b/) { + $suffix .= 'L'; + } if (WARN("TYPECAST_INT_CONSTANT", - "Unnecessary typecast of c90 int constant\n" . $herecurr) && + "Unnecessary typecast of c90 int constant - '$cast$const' could be '$const$suffix'\n" . $herecurr) && $fix) { - my $suffix = ""; - my $newconst = $const; - $newconst =~ s/${Int_type}$//; - $suffix .= 'U' if ($cast =~ /\bunsigned\b/); - if ($cast =~ /\blong\s+long\b/) { - $suffix .= 'LL'; - } elsif ($cast =~ /\blong\b/) { - $suffix .= 'L'; - } $fixed[$fixlinenr] =~ s/\Q$cast\E$const\b/$newconst$suffix/; } } @@ -7021,12 +7170,14 @@ sub process { # use of NR_CPUS is usually wrong # ignore definitions of NR_CPUS and usage to define arrays as likely right +# ignore designated initializers using NR_CPUS if ($line =~ /\bNR_CPUS\b/ && $line !~ /^.\s*\s*#\s*if\b.*\bNR_CPUS\b/ && $line !~ /^.\s*\s*#\s*define\b.*\bNR_CPUS\b/ && $line !~ /^.\s*$Declare\s.*\[[^\]]*NR_CPUS[^\]]*\]/ && $line !~ /\[[^\]]*\.\.\.[^\]]*NR_CPUS[^\]]*\]/ && - $line !~ /\[[^\]]*NR_CPUS[^\]]*\.\.\.[^\]]*\]/) + $line !~ /\[[^\]]*NR_CPUS[^\]]*\.\.\.[^\]]*\]/ && + $line !~ /^.\s*\.\w+\s*=\s*.*\bNR_CPUS\b/) { WARN("NR_CPUS", "usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr); @@ -7062,12 +7213,6 @@ sub process { } } -# check for mutex_trylock_recursive usage - if ($line =~ /mutex_trylock_recursive/) { - ERROR("LOCKING", - "recursive locking is bad, do not use this ever.\n" . $herecurr); - } - # check for lockdep_set_novalidate_class if ($line =~ /^.\s*lockdep_set_novalidate_class\s*\(/ || $line =~ /__lockdep_no_validate__\s*\)/ ) { diff --git a/tests/ci/checkpatch/spelling.txt b/tests/ci/checkpatch/spelling.txt index 953f4a2d..2e3ba91a 100644 --- a/tests/ci/checkpatch/spelling.txt +++ b/tests/ci/checkpatch/spelling.txt @@ -103,6 +103,7 @@ alloated||allocated allocatote||allocate allocatrd||allocated allocte||allocate +allocted||allocated allpication||application alocate||allocate alogirhtms||algorithms @@ -339,6 +340,7 @@ comppatible||compatible compres||compress compresion||compression comression||compression +comunicate||communicate comunication||communication conbination||combination conditionaly||conditionally @@ -466,6 +468,7 @@ developpment||development deveolpment||development devided||divided deviece||device +devision||division diable||disable dicline||decline dictionnary||dictionary @@ -479,6 +482,7 @@ difinition||definition digial||digital dimention||dimension dimesions||dimensions +diconnected||disconnected disgest||digest dispalying||displaying diplay||display @@ -518,6 +522,7 @@ downlads||downloads droped||dropped droput||dropout druing||during +dyanmic||dynamic dynmaic||dynamic eanable||enable eanble||enable @@ -542,6 +547,7 @@ encrupted||encrypted encrypiton||encryption encryptio||encryption endianess||endianness +enpoint||endpoint enhaced||enhanced enlightnment||enlightenment enqueing||enqueuing @@ -566,6 +572,7 @@ estbalishment||establishment etsablishment||establishment etsbalishment||establishment evalution||evaluation +exeeds||exceeds excecutable||executable exceded||exceeded exceds||exceeds @@ -574,6 +581,7 @@ excellant||excellent execeeded||exceeded execeeds||exceeds exeed||exceed +exeeds||exceeds exeuction||execution existance||existence existant||existent @@ -641,6 +649,7 @@ forwardig||forwarding frambuffer||framebuffer framming||framing framwork||framework +frequence||frequency frequncy||frequency frequancy||frequency frome||from @@ -683,10 +692,12 @@ handfull||handful hanlde||handle hanled||handled happend||happened +hardare||hardware harware||hardware havind||having heirarchically||hierarchically helpfull||helpful +heterogenous||heterogeneous hexdecimal||hexadecimal hybernate||hibernate hierachy||hierarchy @@ -731,6 +742,7 @@ inconsistant||inconsistent increas||increase incremeted||incremented incrment||increment +incuding||including inculde||include indendation||indentation indended||intended @@ -741,6 +753,7 @@ indiate||indicate indicat||indicate inexpect||inexpected inferface||interface +infinit||infinite infomation||information informatiom||information informations||information @@ -771,6 +784,7 @@ instace||instance instal||install instanciate||instantiate instanciated||instantiated +instuments||instruments insufficent||insufficient inteface||interface integreated||integrated @@ -869,12 +883,14 @@ mailformed||malformed malplaced||misplaced malplace||misplace managable||manageable +managament||management managment||management mangement||management manger||manager manoeuvering||maneuvering manufaucturing||manufacturing mappping||mapping +maping||mapping matchs||matches mathimatical||mathematical mathimatic||mathematic @@ -886,6 +902,7 @@ meetign||meeting memeory||memory memmber||member memoery||memory +memroy||memory ment||meant mergable||mergeable mesage||message @@ -999,6 +1016,7 @@ overlaping||overlapping overide||override overrided||overridden overriden||overridden +overrrun||overrun overun||overrun overwritting||overwriting overwriten||overwritten @@ -1035,6 +1053,7 @@ peforming||performing peice||piece pendantic||pedantic peprocessor||preprocessor +perfomance||performance perfoming||performing perfomring||performing periperal||peripheral @@ -1100,6 +1119,7 @@ prodecure||procedure progamming||programming progams||programs progess||progress +programable||programmable programers||programmers programm||program programms||programs @@ -1144,6 +1164,7 @@ recieved||received recieve||receive reciever||receiver recieves||receives +recieving||receiving recogniced||recognised recognizeable||recognizable recommanded||recommended @@ -1247,6 +1268,7 @@ searchs||searches secquence||sequence secund||second segement||segment +seleted||selected semaphone||semaphore senario||scenario senarios||scenarios @@ -1263,6 +1285,7 @@ seqeunce||sequence seqeuncer||sequencer seqeuencer||sequencer sequece||sequence +sequemce||sequence sequencial||sequential serivce||service serveral||several @@ -1333,6 +1356,7 @@ suble||subtle substract||subtract submited||submitted submition||submission +succeded||succeeded suceed||succeed succesfully||successfully succesful||successful @@ -1353,6 +1377,7 @@ supportin||supporting suppoted||supported suppported||supported suppport||support +supprot||support supress||suppress surpressed||suppressed surpresses||suppresses @@ -1401,6 +1426,7 @@ thresold||threshold throught||through trackling||tracking troughput||throughput +trys||tries thses||these tiggers||triggers tiggered||triggered @@ -1414,7 +1440,9 @@ traking||tracking tramsmitted||transmitted tramsmit||transmit tranasction||transaction +tranceiver||transceiver tranfer||transfer +tranmission||transmission transcevier||transceiver transciever||transceiver transferd||transferred @@ -1468,6 +1496,7 @@ unnecesary||unnecessary unneedingly||unnecessarily unnsupported||unsupported unmached||unmatched +unprecise||imprecise unregester||unregister unresgister||unregister unrgesiter||unregister @@ -1503,6 +1532,7 @@ varient||variant vaule||value verbse||verbose veify||verify +veriosn||version verisons||versions verison||version verson||version From b4e7e5155a5df02a40c5de133f95f85be3958449 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 26 Apr 2021 22:43:36 +0200 Subject: [PATCH 40/76] Different hint for each different connection --- src/http.c | 15 ++++++++++++--- src/main.c | 8 ++++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/http.c b/src/http.c index f9f2fc3b..855ee97d 100644 --- a/src/http.c +++ b/src/http.c @@ -583,7 +583,12 @@ static int try_otp_auth(struct tunnel *tunnel, const char *buffer, v = NULL; if (cfg->otp[0] == '\0') { - read_password(cfg->pinentry, "otp", + // Interactively ask user for OTP + char hint[USERNAME_SIZE + 1 + REALM_SIZE + 1 + GATEWAY_HOST_SIZE + 5]; + + sprintf(hint, "%s_%s_%s_otp", + cfg->username, cfg->realm, cfg->gateway_host); + read_password(cfg->pinentry, hint, p, cfg->otp, OTP_SIZE); if (cfg->otp[0] == '\0') { log_error("No OTP specified\n"); @@ -727,8 +732,12 @@ int auth_log_in(struct tunnel *tunnel) snprintf(tokenparams, sizeof(tokenparams), "ftmpush=1"); } else { if (cfg->otp[0] == '\0') { - // Prompt for 2FA token - read_password(cfg->pinentry, "2fa", + // Interactively ask user for 2FA token + char hint[USERNAME_SIZE + 1 + REALM_SIZE + 1 + GATEWAY_HOST_SIZE + 5]; + + sprintf(hint, "%s_%s_%s_2fa", + cfg->username, cfg->realm, cfg->gateway_host); + read_password(cfg->pinentry, hint, "Two-factor authentication token: ", cfg->otp, OTP_SIZE); diff --git a/src/main.c b/src/main.c index 9adfa018..bd261c47 100644 --- a/src/main.c +++ b/src/main.c @@ -613,8 +613,12 @@ int main(int argc, char **argv) } // If username but no password given, interactively ask user if (!cfg.password_set && cfg.username[0] != '\0') { - read_password(cfg.pinentry, - "password", "VPN account password: ", + char hint[USERNAME_SIZE + 1 + REALM_SIZE + 1 + GATEWAY_HOST_SIZE + 10]; + + sprintf(hint, "%s_%s_%s_password", + cfg.username, cfg.realm, cfg.gateway_host); + read_password(cfg.pinentry, hint, + "VPN account password: ", cfg.password, PASSWORD_SIZE); } log_debug("Configuration host = \"%s\"\n", cfg.gateway_host); From e2f704fea576001ae0a4d02982eaf6f51b08fe35 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Tue, 18 May 2021 10:44:21 +0200 Subject: [PATCH 41/76] Memory management issues Fixes #896 and a couple minor memory leaks at exit. --- src/config.c | 3 +++ src/main.c | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/config.c b/src/config.c index 098ea933..825febbf 100644 --- a/src/config.c +++ b/src/config.c @@ -493,6 +493,9 @@ void destroy_vpn_config(struct vpn_config *cfg) free(cfg->user_cert); free(cfg->user_key); free(cfg->cipher_list); + free(cfg->user_agent); + free(cfg->hostcheck); + free(cfg->check_virtual_desktop); while (cfg->cert_whitelist != NULL) { struct x509_digest *tmp = cfg->cert_whitelist->next; diff --git a/src/main.c b/src/main.c index bd261c47..4d3c8d13 100644 --- a/src/main.c +++ b/src/main.c @@ -237,7 +237,7 @@ int main(int argc, char **argv) .cipher_list = NULL, .cert_whitelist = NULL, .use_engine = 0, - .user_agent = "Mozilla/5.0 SV1", + .user_agent = NULL, }; struct vpn_config cli_cfg = invalid_cfg; @@ -582,6 +582,10 @@ int main(int argc, char **argv) merge_config(&cfg, &cli_cfg); set_syslog(cfg.use_syslog); + // Set default UA + if (cfg.user_agent == NULL) + cfg.user_agent = strdup("Mozilla/5.0 SV1"); + // Read host and port from the command line if (optind == argc - 1) { host = argv[optind++]; From 4b94dc8e49ca0ad62c0f47d7a69a8c6fe3c0720e Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 19 May 2021 08:09:39 +0200 Subject: [PATCH 42/76] Do we really require GNU C library compatibility? MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I don't think we need AC_FUNC_MALLOC and AC_FUNC_REALLOC. From the documentation: If the malloc function is compatible with the GNU C library malloc (i.e., ‘malloc (0)’ returns a valid pointer), define HAVE_MALLOC to 1. Otherwise define HAVE_MALLOC to 0, ask for an AC_LIBOBJ replacement for ‘malloc’, and define malloc to rpl_malloc so that the native malloc is not used in the main project. We do not rely on this specificity as far as I can see. --- configure.ac | 2 -- 1 file changed, 2 deletions(-) diff --git a/configure.ac b/configure.ac index fb232c7c..c3b06877 100644 --- a/configure.ac +++ b/configure.ac @@ -103,8 +103,6 @@ AC_TYPE_UINT8_T AC_CHECK_TYPES([struct termios], [], [], [#include ]) # Checks for library functions. -AC_FUNC_MALLOC -AC_FUNC_REALLOC AC_CHECK_FUNCS([ \ access \ close \ From d4d0548907e7a6147f0064ad5627078826841422 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 19 May 2021 09:08:37 +0200 Subject: [PATCH 43/76] Update checkpatch.pl from Linux kernel --- tests/ci/checkpatch/checkpatch.pl | 23 ++++++++++++++++++----- tests/ci/checkpatch/spelling.txt | 29 ++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/tests/ci/checkpatch/checkpatch.pl b/tests/ci/checkpatch/checkpatch.pl index f42e5ba1..23697a6b 100755 --- a/tests/ci/checkpatch/checkpatch.pl +++ b/tests/ci/checkpatch/checkpatch.pl @@ -3245,7 +3245,7 @@ sub process { ($line =~ /^new file mode\s*\d+\s*$/) && ($realfile =~ m@^Documentation/devicetree/bindings/.*\.txt$@)) { WARN("DT_SCHEMA_BINDING_PATCH", - "DT bindings should be in DT schema format. See: Documentation/devicetree/writing-schema.rst\n"); + "DT bindings should be in DT schema format. See: Documentation/devicetree/bindings/writing-schema.rst\n"); } # Check for wrappage within a valid hunk of the file @@ -5829,7 +5829,7 @@ sub process { next if ($arg =~ /\.\.\./); next if ($arg =~ /^type$/i); my $tmp_stmt = $define_stmt; - $tmp_stmt =~ s/\b(sizeof|typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; + $tmp_stmt =~ s/\b(__must_be_array|offsetof|sizeof|sizeof_field|__stringify|typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; $tmp_stmt =~ s/\#+\s*$arg\b//g; $tmp_stmt =~ s/\b$arg\s*\#\#//g; my $use_cnt = () = $tmp_stmt =~ /\b$arg\b/g; @@ -6699,9 +6699,11 @@ sub process { $specifier = $1; $extension = $2; $qualifier = $3; - if ($extension !~ /[SsBKRraEehMmIiUDdgVCbGNOxtf]/ || + if ($extension !~ /[4SsBKRraEehMmIiUDdgVCbGNOxtf]/ || ($extension eq "f" && - defined $qualifier && $qualifier !~ /^w/)) { + defined $qualifier && $qualifier !~ /^w/) || + ($extension eq "4" && + defined $qualifier && $qualifier !~ /^cc/)) { $bad_specifier = $specifier; last; } @@ -7004,7 +7006,7 @@ sub process { } # check for alloc argument mismatch - if ($line =~ /\b(kcalloc|kmalloc_array)\s*\(\s*sizeof\b/) { + if ($line =~ /\b((?:devm_)?(?:kcalloc|kmalloc_array))\s*\(\s*sizeof\b/) { WARN("ALLOC_ARRAY_ARGS", "$1 uses number as first arg, sizeof is generally wrong\n" . $herecurr); } @@ -7196,6 +7198,17 @@ sub process { "Using $1 should generally have parentheses around the comparison\n" . $herecurr); } +# return sysfs_emit(foo, fmt, ...) fmt without newline + if ($line =~ /\breturn\s+sysfs_emit\s*\(\s*$FuncArg\s*,\s*($String)/ && + substr($rawline, $-[6], $+[6] - $-[6]) !~ /\\n"$/) { + my $offset = $+[6] - 1; + if (WARN("SYSFS_EMIT", + "return sysfs_emit(...) formats should include a terminating newline\n" . $herecurr) && + $fix) { + substr($fixed[$fixlinenr], $offset, 0) = '\\n'; + } + } + # nested likely/unlikely calls if ($line =~ /\b(?:(?:un)?likely)\s*\(\s*!?\s*(IS_ERR(?:_OR_NULL|_VALUE)?|WARN)/) { WARN("LIKELY_MISUSE", diff --git a/tests/ci/checkpatch/spelling.txt b/tests/ci/checkpatch/spelling.txt index 2e3ba91a..7b6a0129 100644 --- a/tests/ci/checkpatch/spelling.txt +++ b/tests/ci/checkpatch/spelling.txt @@ -84,6 +84,7 @@ againt||against agaist||against aggreataon||aggregation aggreation||aggregation +ajust||adjust albumns||albums alegorical||allegorical algined||aligned @@ -161,10 +162,13 @@ asign||assign asser||assert assertation||assertion assertting||asserting +assgined||assigned assiged||assigned assigment||assignment assigments||assignments assistent||assistant +assocaited||associated +assocating||associating assocation||association associcated||associated assotiated||associated @@ -177,9 +181,11 @@ asynchnous||asynchronous asynchromous||asynchronous asymetric||asymmetric asymmeric||asymmetric +atleast||at least atomatically||automatically atomicly||atomically atempt||attempt +atrributes||attributes attachement||attachment attatch||attach attched||attached @@ -315,6 +321,7 @@ comminucation||communication commited||committed commiting||committing committ||commit +commnunication||communication commoditiy||commodity comsume||consume comsumer||consumer @@ -349,6 +356,7 @@ condtion||condition conected||connected conector||connector configration||configuration +configred||configured configuartion||configuration configuation||configuration configued||configured @@ -402,6 +410,7 @@ cunter||counter curently||currently cylic||cyclic dafault||default +deactive||deactivate deafult||default deamon||daemon debouce||debounce @@ -417,6 +426,7 @@ deffered||deferred defferred||deferred definate||definite definately||definitely +definiation||definition defintion||definition defintions||definitions defualt||default @@ -470,6 +480,7 @@ devided||divided deviece||device devision||division diable||disable +diabled||disabled dicline||decline dictionnary||dictionary didnt||didn't @@ -571,8 +582,9 @@ errror||error estbalishment||establishment etsablishment||establishment etsbalishment||establishment +evalute||evaluate +evalutes||evaluates evalution||evaluation -exeeds||exceeds excecutable||executable exceded||exceeded exceds||exceeds @@ -696,6 +708,7 @@ hardare||hardware harware||hardware havind||having heirarchically||hierarchically +heirarchy||hierarchy helpfull||helpful heterogenous||heterogeneous hexdecimal||hexadecimal @@ -796,6 +809,7 @@ interanl||internal interchangable||interchangeable interferring||interfering interger||integer +intergrated||integrated intermittant||intermittent internel||internal interoprability||interoperability @@ -808,6 +822,7 @@ interrup||interrupt interrups||interrupts interruptted||interrupted interupted||interrupted +intiailized||initialized intial||initial intialisation||initialisation intialised||initialised @@ -1013,6 +1028,8 @@ oustanding||outstanding overaall||overall overhread||overhead overlaping||overlapping +overflw||overflow +overlfow||overflow overide||override overrided||overridden overriden||overridden @@ -1091,11 +1108,14 @@ preemptable||preemptible prefered||preferred prefferably||preferably prefitler||prefilter +preform||perform premption||preemption prepaired||prepared preperation||preparation preprare||prepare pressre||pressure +presuambly||presumably +previosuly||previously primative||primitive princliple||principle priorty||priority @@ -1265,6 +1285,7 @@ scarch||search schdule||schedule seach||search searchs||searches +secion||section secquence||sequence secund||second segement||segment @@ -1312,6 +1333,8 @@ singed||signed sleeped||slept sliped||slipped softwares||software +soley||solely +souce||source speach||speech specfic||specific specfield||specified @@ -1320,7 +1343,9 @@ specifc||specific specifed||specified specificatin||specification specificaton||specification +specificed||specified specifing||specifying +specifiy||specify specifiying||specifying speficied||specified speicify||specify @@ -1436,6 +1461,7 @@ timout||timeout tmis||this toogle||toggle torerable||tolerable +traget||target traking||tracking tramsmitted||transmitted tramsmit||transmit @@ -1558,6 +1584,7 @@ wiil||will wirte||write withing||within wnat||want +wont||won't workarould||workaround writeing||writing writting||writing From fad31a9093f28310fe007bdd0181e45d1a83c983 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 3 Jun 2021 20:53:17 +0200 Subject: [PATCH 44/76] Disable support for FortiOS 4 by default Do not read the HTML-formatted configuration from FortiOS 4. Just read the XML-formatted configuration. --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index c3b06877..bdede0e7 100644 --- a/configure.ac +++ b/configure.ac @@ -439,8 +439,8 @@ int main(int argc, char **argv){ # prepare to get rid of obsolete code (FortiOS 4) AC_ARG_ENABLE([obsolete], - [AS_HELP_STRING([--disable-obsolete], [disable support for FortiOS 4])],, - [enable_obsolete=yes]) + [AS_HELP_STRING([--enable-obsolete], [enable support for FortiOS 4])],, + [enable_obsolete=no]) AS_CASE(["$enable_obsolete"], [yes], [], [no], [], From 663f6147570548f6732ed47356628b3e8c626454 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 4 Jun 2021 13:17:04 +0200 Subject: [PATCH 45/76] Very minor typo :-) --- src/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.c b/src/main.c index 4d3c8d13..a3a1271b 100644 --- a/src/main.c +++ b/src/main.c @@ -160,7 +160,7 @@ PPPD_USAGE \ " (compiled in) list of ciphers. This lowers limits on\n" \ " dh key." help_seclevel_1 "\n" \ " --persistent= Run the vpn persistently in a loop and try to re-\n" \ -" connect every seconds when dropping out\n" \ +" connect every seconds when dropping out.\n" \ " -v Increase verbosity. Can be used multiple times\n" \ " to be even more verbose.\n" \ " -q Decrease verbosity. Can be used multiple times\n" \ From abb1e29b86e678964e62dcd73efae1220288f998 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 3 Jun 2021 13:16:58 +0200 Subject: [PATCH 46/76] Attempt to release allocated resources Properly reset variables after releasing resources. The following resources are allocated in this order in ssl_connect(): ssl_socket ssl_context ssl_handle Release them in the reverse order in ssl_disconnect() and in case of error in ssl_connect(): ssl_handle ssl_context ssl_socket I have not fixed cases where tunnel->ssl_socket is released, but tunnel->ssl_context and tunnel->ssl_handle are left behind. This seems very wrong, but fixing it breaks --persistent. --- src/tunnel.c | 64 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/src/tunnel.c b/src/tunnel.c index 9c061097..ec378f5c 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -327,6 +327,7 @@ static int pppd_run(struct tunnel *tunnel) if (close(tunnel->ssl_socket)) log_warn("Could not close ssl socket (%s).\n", strerror(errno)); + tunnel->ssl_socket = -1; execv(pppd_args.data[0], (char *const *)pppd_args.data); free(pppd_args.data); @@ -959,12 +960,14 @@ static void ssl_disconnect(struct tunnel *tunnel) SSL_shutdown(tunnel->ssl_handle); SSL_free(tunnel->ssl_handle); + tunnel->ssl_handle = NULL; + SSL_CTX_free(tunnel->ssl_context); + tunnel->ssl_context = NULL; + if (close(tunnel->ssl_socket)) log_warn("Could not close ssl socket (%s).\n", strerror(errno)); - - tunnel->ssl_handle = NULL; - tunnel->ssl_context = NULL; + tunnel->ssl_socket = -1; } /* @@ -1011,7 +1014,7 @@ int ssl_connect(struct tunnel *tunnel) tunnel->ssl_socket = tcp_connect(tunnel); if (tunnel->ssl_socket == -1) - return 1; + goto err_tcp_connect; // https://wiki.openssl.org/index.php/Library_Initialization #if OPENSSL_VERSION_NUMBER < 0x10100000L @@ -1027,7 +1030,7 @@ int ssl_connect(struct tunnel *tunnel) if (tunnel->ssl_context == NULL) { log_error("SSL_CTX_new: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_socket; } /* Load the OS default CA files */ @@ -1039,7 +1042,7 @@ int ssl_connect(struct tunnel *tunnel) tunnel->config->ca_file, NULL)) { log_error("SSL_CTX_load_verify_locations: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } } @@ -1070,7 +1073,7 @@ int ssl_connect(struct tunnel *tunnel) tunnel->config->cipher_list)) { log_error("SSL_CTX_set_cipher_list failed: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } } @@ -1082,7 +1085,7 @@ int ssl_connect(struct tunnel *tunnel) tunnel->config->min_tls)) { log_error("Cannot set minimum protocol version (%s).\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } } #else @@ -1108,7 +1111,7 @@ int ssl_connect(struct tunnel *tunnel) if ((checkopt & sslctxopt) != sslctxopt) { log_error("SSL_CTX_set_options didn't set opt: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } } #endif @@ -1123,13 +1126,13 @@ int ssl_connect(struct tunnel *tunnel) if (!e) { log_error("Could not load pkcs11 Engine: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } if (!ENGINE_init(e)) { log_error("Could not init pkcs11 Engine: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); ENGINE_free(e); - return 1; + goto err_ssl_context; } if (!ENGINE_set_default_RSA(e)) abort(); @@ -1145,13 +1148,13 @@ int ssl_connect(struct tunnel *tunnel) if (!ENGINE_ctrl_cmd(e, "LOAD_CERT_CTRL", 0, &parms, NULL, 1)) { log_error("PKCS11 ENGINE_ctrl_cmd: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } if (!SSL_CTX_use_certificate(tunnel->ssl_context, parms.cert)) { log_error("PKCS11 SSL_CTX_use_certificate: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } EVP_PKEY * privkey = ENGINE_load_private_key( @@ -1159,19 +1162,19 @@ int ssl_connect(struct tunnel *tunnel) if (!privkey) { log_error("PKCS11 ENGINE_load_private_key: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } if (!SSL_CTX_use_PrivateKey(tunnel->ssl_context, privkey)) { log_error("PKCS11 SSL_CTX_use_PrivateKey_file: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } if (!SSL_CTX_check_private_key(tunnel->ssl_context)) { log_error("PKCS11 SSL_CTX_check_private_key: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } } else { /* end PKCS11-engine */ @@ -1182,7 +1185,7 @@ int ssl_connect(struct tunnel *tunnel) tunnel->ssl_context, tunnel->config->user_cert)) { log_error("SSL_CTX_use_certificate_chain_file: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } } @@ -1192,7 +1195,7 @@ int ssl_connect(struct tunnel *tunnel) SSL_FILETYPE_PEM)) { log_error("SSL_CTX_use_PrivateKey_file: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } } @@ -1200,7 +1203,7 @@ int ssl_connect(struct tunnel *tunnel) if (!SSL_CTX_check_private_key(tunnel->ssl_context)) { log_error("SSL_CTX_check_private_key: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } } #ifdef OPENSSL_ENGINE @@ -1211,13 +1214,13 @@ int ssl_connect(struct tunnel *tunnel) if (tunnel->ssl_handle == NULL) { log_error("SSL_new: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_context; } if (!SSL_set_fd(tunnel->ssl_handle, tunnel->ssl_socket)) { log_error("SSL_set_fd: %s\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_handle; } SSL_set_mode(tunnel->ssl_handle, SSL_MODE_AUTO_RETRY); @@ -1226,18 +1229,32 @@ int ssl_connect(struct tunnel *tunnel) log_error("SSL_connect: %s\n" "You might want to try --insecure-ssl or specify a different --cipher-list\n", ERR_error_string(ERR_peek_last_error(), NULL)); - return 1; + goto err_ssl_handle; } SSL_set_mode(tunnel->ssl_handle, SSL_MODE_AUTO_RETRY); if (ssl_verify_cert(tunnel)) - return 1; + goto err_ssl_handle; // Disable SIGPIPE (occurs when trying to write to an already-closed // socket). signal(SIGPIPE, SIG_IGN); return 0; + +err_ssl_handle: + SSL_shutdown(tunnel->ssl_handle); + SSL_free(tunnel->ssl_handle); + tunnel->ssl_handle = NULL; +err_ssl_context: + SSL_CTX_free(tunnel->ssl_context); + tunnel->ssl_context = NULL; +err_ssl_socket: + if (close(tunnel->ssl_socket)) + log_warn("Could not close ssl socket (%s).\n", strerror(errno)); + tunnel->ssl_socket = -1; +err_tcp_connect: + return 1; } int run_tunnel(struct vpn_config *config) @@ -1246,6 +1263,7 @@ int run_tunnel(struct vpn_config *config) struct tunnel tunnel = { .config = config, .state = STATE_DOWN, + .ssl_socket = -1, .ssl_context = NULL, .ssl_handle = NULL, .ipv4.ns1_addr.s_addr = 0, From fb89f2a64b63d1734d4d71d97f3cc939fffe9214 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 14 Jul 2021 23:12:55 +0200 Subject: [PATCH 47/76] Update checkpatch.pl from Linux kernel --- tests/ci/checkpatch/checkpatch.pl | 16 ++++++++++------ tests/ci/checkpatch/spelling.txt | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/ci/checkpatch/checkpatch.pl b/tests/ci/checkpatch/checkpatch.pl index 23697a6b..461d4221 100755 --- a/tests/ci/checkpatch/checkpatch.pl +++ b/tests/ci/checkpatch/checkpatch.pl @@ -1084,10 +1084,10 @@ sub is_maintained_obsolete { sub is_SPDX_License_valid { my ($license) = @_; - return 1 if (!$tree || which("python") eq "" || !(-e "$root/scripts/spdxcheck.py") || !(-e "$gitroot")); + return 1 if (!$tree || which("python3") eq "" || !(-x "$root/scripts/spdxcheck.py") || !(-e "$gitroot")); my $root_path = abs_path($root); - my $status = `cd "$root_path"; echo "$license" | python scripts/spdxcheck.py -`; + my $status = `cd "$root_path"; echo "$license" | scripts/spdxcheck.py -`; return 0 if ($status ne ""); return 1; } @@ -5361,9 +5361,13 @@ sub process { } } -#goto labels aren't indented, allow a single space however - if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and - !($line=~/^. [A-Za-z\d_]+:/) and !($line=~/^.\s+default:/)) { +# check that goto labels aren't indented (allow a single space indentation) +# and ignore bitfield definitions like foo:1 +# Strictly, labels can have whitespace after the identifier and before the : +# but this is not allowed here as many ?: uses would appear to be labels + if ($sline =~ /^.\s+[A-Za-z_][A-Za-z\d_]*:(?!\s*\d+)/ && + $sline !~ /^. [A-Za-z\d_][A-Za-z\d_]*:/ && + $sline !~ /^.\s+default:/) { if (WARN("INDENTED_LABEL", "labels should not be indented\n" . $herecurr) && $fix) { @@ -5458,7 +5462,7 @@ sub process { # Return of what appears to be an errno should normally be negative if ($sline =~ /\breturn(?:\s*\(+\s*|\s+)(E[A-Z]+)(?:\s*\)+\s*|\s*)[;:,]/) { my $name = $1; - if ($name ne 'EOF' && $name ne 'ERROR') { + if ($name ne 'EOF' && $name ne 'ERROR' && $name !~ /^EPOLL/) { WARN("USE_NEGATIVE_ERRNO", "return of an errno should typically be negative (ie: return -$1)\n" . $herecurr); } diff --git a/tests/ci/checkpatch/spelling.txt b/tests/ci/checkpatch/spelling.txt index 7b6a0129..17fdc620 100644 --- a/tests/ci/checkpatch/spelling.txt +++ b/tests/ci/checkpatch/spelling.txt @@ -22,6 +22,7 @@ absolut||absolute absoulte||absolute acccess||access acceess||access +accelaration||acceleration acceleratoin||acceleration accelleration||acceleration accesing||accessing @@ -264,6 +265,7 @@ calucate||calculate calulate||calculate cancelation||cancellation cancle||cancel +canot||cannot capabilites||capabilities capabilties||capabilities capabilty||capability @@ -494,7 +496,10 @@ digial||digital dimention||dimension dimesions||dimensions diconnected||disconnected +disabed||disabled +disble||disable disgest||digest +disired||desired dispalying||displaying diplay||display directon||direction @@ -710,6 +715,7 @@ havind||having heirarchically||hierarchically heirarchy||hierarchy helpfull||helpful +hearbeat||heartbeat heterogenous||heterogeneous hexdecimal||hexadecimal hybernate||hibernate @@ -989,6 +995,7 @@ notications||notifications notifcations||notifications notifed||notified notity||notify +nubmer||number numebr||number numner||number obtaion||obtain @@ -1014,8 +1021,10 @@ ommiting||omitting ommitted||omitted onself||oneself ony||only +openning||opening operatione||operation opertaions||operations +opportunies||opportunities optionnal||optional optmizations||optimizations orientatied||orientated @@ -1111,6 +1120,7 @@ prefitler||prefilter preform||perform premption||preemption prepaired||prepared +prepate||prepare preperation||preparation preprare||prepare pressre||pressure @@ -1123,6 +1133,7 @@ privilaged||privileged privilage||privilege priviledge||privilege priviledges||privileges +privleges||privileges probaly||probably procceed||proceed proccesors||processors @@ -1167,6 +1178,7 @@ promixity||proximity psudo||pseudo psuedo||pseudo psychadelic||psychedelic +purgable||purgeable pwoer||power queing||queuing quering||querying @@ -1180,6 +1192,7 @@ receieve||receive recepient||recipient recevied||received receving||receiving +recievd||received recieved||received recieve||receive reciever||receiver @@ -1228,6 +1241,7 @@ reponse||response representaion||representation reqeust||request reqister||register +requed||requeued requestied||requested requiere||require requirment||requirement @@ -1332,6 +1346,7 @@ singal||signal singed||signed sleeped||slept sliped||slipped +softwade||software softwares||software soley||solely souce||source @@ -1510,6 +1525,7 @@ unintialized||uninitialized unitialized||uninitialized unkmown||unknown unknonw||unknown +unknouwn||unknown unknow||unknown unkown||unknown unamed||unnamed From b8de256c5a935957824cd17159985fddd29a1b76 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 15 Jul 2021 00:15:37 +0200 Subject: [PATCH 48/76] =?UTF-8?q?CONFIG=20FILES=20=E2=86=92=20CONFIGURATIO?= =?UTF-8?q?N?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 6342cdce..3f927165 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ Examples trusted-cert = e46d4aff08ba6914e64daa85bc6112a422fa7ce16631bff0b592a28556f993db ``` -* For the full list of config options, see the `CONFIG FILE` section of +* For the full list of config options, see the `CONFIGURATION` section of ``` man openfortivpn ``` From c7f9f2c2c7dfee4ce930a4f053eeb3cdd4ba2482 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 15 Jul 2021 00:06:01 +0200 Subject: [PATCH 49/76] openfortivpn version 1.17.0 --- CHANGELOG.md | 8 ++++++++ configure.ac | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a87dd14b..9b5d55bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,14 @@ Releases This high level changelog is usually updated when a release is tagged. On the master branch there may be changes that are not (yet) described here. +### 1.17.0 + +* [-] make OpenSSL engines optional +* [+] document and favor --pinentry over plain text password in configuration file +* [-] fix buffer overflow and other errors in URI espcaping for --pinentry +* [~] use different --pinentry hints for different hosts, usernames and realms +* [-] fix memory management errors related to --user-agent option + ### 1.16.0 * [+] support for user key pass phrase diff --git a/configure.ac b/configure.ac index bdede0e7..2818f9ef 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ([2.63]) -AC_INIT([openfortivpn], [1.16.0]) +AC_INIT([openfortivpn], [1.17.0]) AC_CONFIG_SRCDIR([src/main.c]) AM_INIT_AUTOMAKE([foreign subdir-objects]) From 7e17fce06af630892716ae354d6400aa650b7523 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sun, 1 Aug 2021 13:31:28 +0200 Subject: [PATCH 50/76] More readable --- src/tunnel.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/tunnel.c b/src/tunnel.c index ec378f5c..3eece438 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -511,17 +511,16 @@ int ppp_interface_is_up(struct tunnel *tunnel) } for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) { - if (( + if ( #if HAVE_USR_SBIN_PPPD - (tunnel->config->pppd_ifname - && strstr(ifa->ifa_name, tunnel->config->pppd_ifname) - != NULL) - || strstr(ifa->ifa_name, "ppp") != NULL + ((tunnel->config->pppd_ifname && + strstr(ifa->ifa_name, tunnel->config->pppd_ifname) != NULL) || + strstr(ifa->ifa_name, "ppp") != NULL) && #endif #if HAVE_USR_SBIN_PPP - strstr(ifa->ifa_name, "tun") != NULL + strstr(ifa->ifa_name, "tun") != NULL && #endif - ) && ifa->ifa_flags & IFF_UP) { + ifa->ifa_flags & IFF_UP) { if (&(ifa->ifa_addr->sa_family) != NULL && ifa->ifa_addr->sa_family == AF_INET) { struct in_addr if_ip_addr = From 680a17adef1853b1a05cc625e0c54bbf106d3f88 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sat, 7 Aug 2021 14:50:58 +0200 Subject: [PATCH 51/76] Typos caught by codespell --- CHANGELOG.md | 8 ++++---- configure.ac | 2 +- src/ipv4.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b5d55bd..ba1d8445 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,7 +51,7 @@ On the master branch there may be changes that are not (yet) described here. * [+] add git commit id in debug output * [-] do not use interface ip for routing on linux * [-] avoid extra hop on interface for default route -* [+] clean up, updates and improvments in the build system +* [+] clean up, updates and improvements in the build system * [+] increase the inbound HTTP buffer capacity when needed * [+] print domain search list to output * [+] add systemd service file @@ -191,7 +191,7 @@ On the master branch there may be changes that are not (yet) described here. * [+] Print clear text error messages of pppd upon failure * [~] Existing configuration file is not overwritten anymore at installation time * [~] Increase the accepted cookie size and align the error behavior according to RFCs -* [-] More gracefully handle unexcpected content of resolv.conf +* [-] More gracefully handle unexpected content of resolv.conf * [~] Dynamically allocate memory for split routes and thus support larger numbers of routes ### 1.5.0 @@ -286,8 +286,8 @@ On the master branch there may be changes that are not (yet) described here. * [+] Add support for client keys and certificates * [~] Extend the split VPN support with older FortiOS servers * [+] Add a config parser to handle received non-xml content -* [~] Allow ommitting the gateway for split routes -* [~] Allow ommitting DNS servers +* [~] Allow omitting the gateway for split routes +* [~] Allow omitting DNS servers * [-] Fix a memory leak in auth_get_config * [+] Support split routes * [+] Export the configuration of routes and gateway to environment diff --git a/configure.ac b/configure.ac index 2818f9ef..2a7aa0b4 100644 --- a/configure.ac +++ b/configure.ac @@ -372,7 +372,7 @@ AC_ARG_ENABLE([resolvconf], AS_HELP_STRING([--enable-resolvconf], [Enable usage of resolvconf at runtime by default. \ Use --disable-resolvconf for the opposite, note that \ - resolvconf support will still be compliled in, but \ + resolvconf support will still be compilled in, but \ disabled if not explicitly enabled at runtime.])) # Determine how resolvconf works at build-time if it is installed: diff --git a/src/ipv4.c b/src/ipv4.c index b8cb2ce6..836221aa 100644 --- a/src/ipv4.c +++ b/src/ipv4.c @@ -143,7 +143,7 @@ static int ipv4_get_route(struct rtentry *route) * - the routing table is to some extent trusted input, * - it's not that large, * - and the loop in strtok_r increments the pointer in each - * interation until it reaches the area where we have ensured + * iteration until it reaches the area where we have ensured * that there is a delimiting '\0' character by proper * initialization. We ensure this also when growing the buffer. */ From 6a2873bd8c1ef02c1673fc341df5537c32682cf5 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sat, 7 Aug 2021 21:07:20 +0200 Subject: [PATCH 52/76] LGTM alert: Comparison is always false because size <= 65529. * size is an uint16_t so it is 65535 == 0xffff at most * size + 6 <= 0xffff, therefore size <= 65529 --- src/io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/io.c b/src/io.c index 124c7ccc..9acf520e 100644 --- a/src/io.c +++ b/src/io.c @@ -465,7 +465,7 @@ static void *ssl_read(void *arg) total = (header[0] << 8) | header[1]; magic = (header[2] << 8) | header[3]; size = (header[4] << 8) | header[5]; - if (magic != 0x5050 || total != 6 + size || size == 0 || size >= 0xffff) { + if (magic != 0x5050 || total != 6 + size || size == 0) { log_error("Received bad header from gateway:\n"); debug_bad_packet(tunnel, header); break; From c9f2aa183fc982ad060b9241da52bb10783ed59f Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sat, 7 Aug 2021 21:31:50 +0200 Subject: [PATCH 53/76] LGTM alert: Comparison is always false because exit_status >= 0. That's not very clearly documented but can be somehow inferred from wait(2): WEXITSTATUS(wstatus) returns the exit status of the child. This consists of the least significant 8 bits of the status argument that the child specified in a call to exit(3) or _exit(2) or as the argument for a return statement in main(). and exit(3): The exit() function causes normal process termination and the value of status & 0377 is returned to the parent (see wait(2)). See also the implementation of the WEXITSTATUS() macro: #define __WEXITSTATUS(status) (((status) & 0xff00) >> 8) --- src/tunnel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tunnel.c b/src/tunnel.c index 3eece438..a9276239 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -447,7 +447,7 @@ static int pppd_terminate(struct tunnel *tunnel) */ log_debug("waitpid: %s exit status code %d\n", PPP_DAEMON, exit_status); - if (exit_status >= ARRAY_SIZE(ppp_message) || exit_status < 0) { + if (exit_status >= ARRAY_SIZE(ppp_message)) { log_error("%s: Returned an unknown exit status code: %d\n", PPP_DAEMON, exit_status); } else { From bf8dbb2e29038c40d2bfc59c517a8fab4632540c Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sat, 7 Aug 2021 21:47:52 +0200 Subject: [PATCH 54/76] LGTM alert: This will only be NULL if ifa == -24 See rule "Dubious NULL check" in LGTM help: The expression &foo->bar gets the address of foo's member bar, which is the address of foo plus the offset of the bar member. If said offset is non-zero, then the expression &foo->bar only equals NULL when the address of foo is negative. While this is not impossible, it can only happen if foo is a negative integer explicitly cast to a pointer, or if foo is a pointer into kernel-mode address space. As neither of these cases are particularly likely, the NULL-check is dubious. --- src/tunnel.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tunnel.c b/src/tunnel.c index a9276239..f10332bd 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -521,8 +521,7 @@ int ppp_interface_is_up(struct tunnel *tunnel) strstr(ifa->ifa_name, "tun") != NULL && #endif ifa->ifa_flags & IFF_UP) { - if (&(ifa->ifa_addr->sa_family) != NULL - && ifa->ifa_addr->sa_family == AF_INET) { + if (ifa->ifa_addr->sa_family == AF_INET) { struct in_addr if_ip_addr = cast_addr(ifa->ifa_addr)->sin_addr; From 13219fd4075301fcff4b15b89a346b1297ed0582 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Mon, 9 Aug 2021 23:54:19 +0200 Subject: [PATCH 55/76] Rework 6a2873b MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The type of total is uint16_t, so it's 65535 at most. The type of size is uint16_t too, but in addition we have: total == 6 + size So size is 65529 at most and cannot be 0xffff. So far LGTM seems to be right when suggesting removing the comparison. So, why does Coverity Scan still complain? The following failure might be possible: • total < 6 * size > 65529 and operation "6 + size" overflows While comparing size with 0xffff does seem useless, we do add a check to make sure that total >= 6, which may happen if an attacker sends a crafted packet. --- src/io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/io.c b/src/io.c index 9acf520e..a1546bfb 100644 --- a/src/io.c +++ b/src/io.c @@ -465,7 +465,7 @@ static void *ssl_read(void *arg) total = (header[0] << 8) | header[1]; magic = (header[2] << 8) | header[3]; size = (header[4] << 8) | header[5]; - if (magic != 0x5050 || total != 6 + size || size == 0) { + if (magic != 0x5050 || total < 6 || total != 6 + size || size == 0) { log_error("Received bad header from gateway:\n"); debug_bad_packet(tunnel, header); break; From 0deb63b42f351202dfe880d93b5aca9ed2cf4725 Mon Sep 17 00:00:00 2001 From: Arthur Lutz Date: Wed, 18 Aug 2021 16:37:38 +0200 Subject: [PATCH 56/76] [README] debian : available un buster and later --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 3f927165..de89444d 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,7 @@ Some Linux distributions provide `openfortivpn` packages: * [Gentoo](https://packages.gentoo.org/packages/net-vpn/openfortivpn) * [NixOS](https://github.com/NixOS/nixpkgs/tree/master/pkgs/tools/networking/openfortivpn) * [Arch Linux](https://www.archlinux.org/packages/community/x86_64/openfortivpn) -* [Debian (testing)](https://packages.debian.org/buster/openfortivpn) +* [Debian (buster and later)](https://packages.debian.org/stable/openfortivpn) * [Ubuntu (bionic and later)](https://packages.ubuntu.com/search?keywords=openfortivpn) and [pre-bionic (ppa)](https://launchpad.net/~ar-lex/+archive/ubuntu/fortisslvpn) * [Solus](https://dev.getsol.us/source/openfortivpn/) From 12700442c657d1479b3e79d51fa467abc55ea133 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 18 Aug 2021 18:58:45 +0300 Subject: [PATCH 57/76] pre-bionic is old, so discard such details --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index de89444d..3ed1ee49 100644 --- a/README.md +++ b/README.md @@ -95,8 +95,8 @@ Some Linux distributions provide `openfortivpn` packages: * [Gentoo](https://packages.gentoo.org/packages/net-vpn/openfortivpn) * [NixOS](https://github.com/NixOS/nixpkgs/tree/master/pkgs/tools/networking/openfortivpn) * [Arch Linux](https://www.archlinux.org/packages/community/x86_64/openfortivpn) -* [Debian (buster and later)](https://packages.debian.org/stable/openfortivpn) -* [Ubuntu (bionic and later)](https://packages.ubuntu.com/search?keywords=openfortivpn) and [pre-bionic (ppa)](https://launchpad.net/~ar-lex/+archive/ubuntu/fortisslvpn) +* [Debian](https://packages.debian.org/stable/openfortivpn) +* [Ubuntu](https://packages.ubuntu.com/search?keywords=openfortivpn) * [Solus](https://dev.getsol.us/source/openfortivpn/) On macOS both [Homebrew](https://formulae.brew.sh/formula/openfortivpn) and From b7608bf354c8ae078ce09ae451562cdc909bd00e Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 18 Aug 2021 12:45:52 +0300 Subject: [PATCH 58/76] Add codespell GitHub action --- .github/workflows/codespell.yml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .github/workflows/codespell.yml diff --git a/.github/workflows/codespell.yml b/.github/workflows/codespell.yml new file mode 100644 index 00000000..6b25a96a --- /dev/null +++ b/.github/workflows/codespell.yml @@ -0,0 +1,21 @@ +--- +name: Codespell + +on: + push: + + pull_request: + branches: + - master + +jobs: + codespell: + name: Check for spelling errors + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + - uses: codespell-project/actions-codespell@master + with: + skip: checkpatch.pl,spelling.txt,LICENSE.OpenSSL + ignore_words_list: synopsys,parms From 6feac1bd1361bbd9e1fc13d5ad44094bf3f095be Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 25 Aug 2021 19:23:21 +0300 Subject: [PATCH 59/76] Revert bf8dbb2 Need to look the LGTM.com alert that resulted in the above commit again. --- src/tunnel.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tunnel.c b/src/tunnel.c index f10332bd..a9276239 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -521,7 +521,8 @@ int ppp_interface_is_up(struct tunnel *tunnel) strstr(ifa->ifa_name, "tun") != NULL && #endif ifa->ifa_flags & IFF_UP) { - if (ifa->ifa_addr->sa_family == AF_INET) { + if (&(ifa->ifa_addr->sa_family) != NULL + && ifa->ifa_addr->sa_family == AF_INET) { struct in_addr if_ip_addr = cast_addr(ifa->ifa_addr)->sin_addr; From ffab7ec25ab7f45025f48a406f9fd6d81257a4ec Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 26 Aug 2021 16:20:18 +0300 Subject: [PATCH 60/76] check the address of ifa_addr, not of an ifa_addr member "ifa_addr" points to a "struct sockaddr": struct ifaddrs { [...] struct sockaddr *ifa_addr; /* Address of interface */ [...] }; Then in turn "struct sockaddr" looks like this: struct sockaddr { unsigned short sa_family; char sa_data[14]; }; Clearly, ifa_addr and &(ifa->sa_family) are the same. Use the simplest and most straightforward form. --- src/tunnel.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tunnel.c b/src/tunnel.c index a9276239..f1db6144 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -521,8 +521,7 @@ int ppp_interface_is_up(struct tunnel *tunnel) strstr(ifa->ifa_name, "tun") != NULL && #endif ifa->ifa_flags & IFF_UP) { - if (&(ifa->ifa_addr->sa_family) != NULL - && ifa->ifa_addr->sa_family == AF_INET) { + if (ifa->ifa_addr && ifa->ifa_addr->sa_family == AF_INET) { struct in_addr if_ip_addr = cast_addr(ifa->ifa_addr)->sin_addr; From eb66a95e95be5ced54e68cb5ecc2820765bb517e Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 26 Aug 2021 16:43:38 +0300 Subject: [PATCH 61/76] size == 0 is equivalent to total == 6 --- src/io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/io.c b/src/io.c index a1546bfb..39037651 100644 --- a/src/io.c +++ b/src/io.c @@ -465,7 +465,7 @@ static void *ssl_read(void *arg) total = (header[0] << 8) | header[1]; magic = (header[2] << 8) | header[3]; size = (header[4] << 8) | header[5]; - if (magic != 0x5050 || total < 6 || total != 6 + size || size == 0) { + if (magic != 0x5050 || total < 7 || total - 6 != size) { log_error("Received bad header from gateway:\n"); debug_bad_packet(tunnel, header); break; From b263afd3c8bf04d8f9762dd60aa173ce8be1aa16 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 26 Aug 2021 17:44:24 +0300 Subject: [PATCH 62/76] Attempt to work around Coverity Scan false positive CID 349819: Untrusted loop bound (TAINTED_SCALAR) tainted_data: Passing tainted expression size to safe_ssl_read_all, which uses it as a loop boundary. --- src/io.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/io.c b/src/io.c index 39037651..47d9267b 100644 --- a/src/io.c +++ b/src/io.c @@ -462,9 +462,9 @@ static void *ssl_read(void *arg) break; } - total = (header[0] << 8) | header[1]; - magic = (header[2] << 8) | header[3]; - size = (header[4] << 8) | header[5]; + total = (uint16_t)(header[0]) << 8 | header[1]; + magic = (uint16_t)(header[2]) << 8 | header[3]; + size = (uint16_t)(header[4]) << 8 | header[5]; if (magic != 0x5050 || total < 7 || total - 6 != size) { log_error("Received bad header from gateway:\n"); debug_bad_packet(tunnel, header); From 9803e861db02e8a58e11b15756cd08928b7e812d Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 8 Sep 2021 06:06:15 +0300 Subject: [PATCH 63/76] Revert #846 / e9dfa8e Actually, I have not entirely reverted this previous patch. I have just modified the default setting: OpenSSL engines are now enabled by default, but are disabled when OPENSSL_NO_ENGINE is set. Indeed, OPENSSL_NO_ENGINE is never defined, while OPENSSL_NO_ENGINE may be defined by OpenSSL in . --- src/tunnel.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tunnel.c b/src/tunnel.c index f1db6144..7aede6f2 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -35,7 +35,7 @@ #endif #include -#ifdef OPENSSL_ENGINE +#ifndef OPENSSL_NO_ENGINE #include #endif #include @@ -1115,7 +1115,7 @@ int ssl_connect(struct tunnel *tunnel) #endif /* Use engine for PIV if user-cert config starts with pkcs11 URI: */ -#ifdef OPENSSL_ENGINE +#ifndef OPENSSL_NO_ENGINE if (tunnel->config->use_engine > 0) { ENGINE *e; @@ -1204,9 +1204,9 @@ int ssl_connect(struct tunnel *tunnel) goto err_ssl_context; } } -#ifdef OPENSSL_ENGINE +#ifndef OPENSSL_NO_ENGINE } -#endif /* PKCS11-engine */ +#endif tunnel->ssl_handle = SSL_new(tunnel->ssl_context); if (tunnel->ssl_handle == NULL) { From ec8bee242e2a4fb86a35b8a7a5909debea22d361 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 8 Sep 2021 05:50:59 +0200 Subject: [PATCH 64/76] openfortivpn version 1.17.1 --- CHANGELOG.md | 6 ++++++ configure.ac | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba1d8445..ace0a0ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,12 @@ Releases This high level changelog is usually updated when a release is tagged. On the master branch there may be changes that are not (yet) described here. +### 1.17.1 + +* [-] fix regression: enable OpenSSL engines by default +* [-] fix typos found by codespell +* [-] fix LGTM alerts + ### 1.17.0 * [-] make OpenSSL engines optional diff --git a/configure.ac b/configure.ac index 2a7aa0b4..5a7740f3 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ([2.63]) -AC_INIT([openfortivpn], [1.17.0]) +AC_INIT([openfortivpn], [1.17.1]) AC_CONFIG_SRCDIR([src/main.c]) AM_INIT_AUTOMAKE([foreign subdir-objects]) From ef8ee6e421b1d6c2b0a1f6ae00a7e45074371bfc Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 8 Oct 2021 08:41:51 +0200 Subject: [PATCH 65/76] Print HTTP requests Use log_info() for consistency with openconnect. --- src/http.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/http.c b/src/http.c index 855ee97d..8bade67b 100644 --- a/src/http.c +++ b/src/http.c @@ -285,6 +285,7 @@ static int do_http_request(struct tunnel *tunnel, "Content-Length: %d\r\n" "\r\n%s"); + log_info("%s %s\n", method, uri); ret = http_send(tunnel, template, method, uri, tunnel->config->gateway_host, tunnel->config->gateway_port, tunnel->config->user_agent, tunnel->cookie, From 1f692aa4fe1eec5853f154c8f85dcef2a8b1d5a6 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Fri, 8 Oct 2021 09:03:54 +0200 Subject: [PATCH 66/76] Add pkg-build as a build dependency --- .github/workflows/openfortivpn.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/openfortivpn.yml b/.github/workflows/openfortivpn.yml index de4afb16..4a36254b 100644 --- a/.github/workflows/openfortivpn.yml +++ b/.github/workflows/openfortivpn.yml @@ -41,7 +41,9 @@ jobs: uses: actions/checkout@v2 - name: Install Dependencies - run: ./tests/ci/install_openssl.sh $HOME/.openfortivpn-deps + run: | + sudo apt-get install -y pkg-config + ./tests/ci/install_openssl.sh $HOME/.openfortivpn-deps - name: Build run: | From d317ad048765f5d35baf4079477ee159f638d914 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 14 Oct 2021 09:14:06 +0200 Subject: [PATCH 67/76] Add an example for user cert authentication --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 3ed1ee49..fd2cd256 100644 --- a/README.md +++ b/README.md @@ -32,10 +32,16 @@ Examples openfortivpn vpn-gateway:8443 --username=foo --pinentry=pinentry-mac ``` +* Connect with a user certificate and no password: + ``` + openfortivpn vpn-gateway:8443 --username= --password= --user-cert=cert.pem --user-key=key.pem + ``` + * Don't set IP routes and don't add VPN nameservers to `/etc/resolv.conf`: ``` openfortivpn vpn-gateway:8443 -u foo --no-routes --no-dns --pppd-no-peerdns ``` + * Using a configuration file: ``` openfortivpn -c /etc/openfortivpn/my-config From 1968e119aeb7acbb26a6e7913a9e26ff28e12eb9 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 20 Oct 2021 07:39:28 +0200 Subject: [PATCH 68/76] Change type of systemd.service Change to notify: Behavior of notify is similar to exec; however, it is expected that the service sends a notification message via sd_notify(3) or an equivalent call when it has finished starting up. systemd will proceed with starting follow-up units after this notification message has been sent. If notify doesn't work, we could fall back to exec: The exec type is similar to simple, but the service manager will consider the unit started immediately after the main service binary has been executed. --- lib/systemd/system/openfortivpn@.service.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/systemd/system/openfortivpn@.service.in b/lib/systemd/system/openfortivpn@.service.in index 6c302304..154bf605 100644 --- a/lib/systemd/system/openfortivpn@.service.in +++ b/lib/systemd/system/openfortivpn@.service.in @@ -4,7 +4,7 @@ After=network-online.target Documentation=man:openfortivpn(1) [Service] -Type=simple +Type=notify PrivateTmp=true ExecStart=@BINDIR@/openfortivpn -c @SYSCONFDIR@/openfortivpn/%I.conf OOMScoreAdjust=-100 From f79258c97c9534a41590a94c63f1fccef9e68dc4 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 18 Aug 2021 20:01:47 +0300 Subject: [PATCH 69/76] Have CodeQL run LGTM.com tests --- .github/workflows/codeql-analysis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index f4e0b48f..65a02995 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -41,6 +41,7 @@ jobs: - name: Initialize CodeQL uses: github/codeql-action/init@v1 with: + queries: +security-extended languages: ${{ matrix.language }} # If you wish to specify custom queries, you can do so here or in a config file. # By default, queries listed here will override any specified in a config file. From a781dd037cf23db8e79a453ca996edbaa31c87ee Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 19 Aug 2021 22:29:34 +0300 Subject: [PATCH 70/76] Fix CodeQL false positive The length of char env_var[24] is enough to hold: "VPN_ROUTE_GATEWAY_" + "65535" + '\0' The key is that the value of tunnel->ipv4.split_routes is between 0 and MAX_SPLIT_ROUTES, which is 65535 and hence cannot be longer than 5 characters. CodeQL doesn't see that and believes that tunnel->ipv4.split_routes may range from -2147483646 to 2147483646, hence print as 11 characters. That's why CodeQL suggests to increase the size of env_var from 24 to 30. But that a false positive of course. Attempt to work around it by adding an assert(). --- src/ipv4.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ipv4.c b/src/ipv4.c index 836221aa..743219f0 100644 --- a/src/ipv4.c +++ b/src/ipv4.c @@ -33,6 +33,7 @@ #include #include #include +#include #define IPV4_GET_ROUTE_BUFFER_CHUNK_SIZE 65536 #define SHOW_ROUTE_BUFFER_SIZE 128 @@ -835,7 +836,7 @@ int ipv4_add_split_vpn_route(struct tunnel *tunnel, char *dest, char *mask, char *gateway) { struct rtentry *route; - char env_var[24]; + char env_var[24]; // strlen("VPN_ROUTE_GATEWAY_") + strlen("65535") + 1 #if HAVE_USR_SBIN_PPPD add_text_route(tunnel, dest, mask, gateway); @@ -853,13 +854,14 @@ int ipv4_add_split_vpn_route(struct tunnel *tunnel, char *dest, char *mask, tunnel->ipv4.split_rt = new_ptr; } + assert(tunnel->ipv4.split_routes >= 0 && + tunnel->ipv4.split_routes < MAX_SPLIT_ROUTES); sprintf(env_var, "VPN_ROUTE_DEST_%d", tunnel->ipv4.split_routes); setenv(env_var, dest, 0); sprintf(env_var, "VPN_ROUTE_MASK_%d", tunnel->ipv4.split_routes); setenv(env_var, mask, 0); if (gateway != NULL) { - sprintf(env_var, "VPN_ROUTE_GATEWAY_%d", - tunnel->ipv4.split_routes); + sprintf(env_var, "VPN_ROUTE_GATEWAY_%d", tunnel->ipv4.split_routes); setenv(env_var, gateway, 0); } From 73bff80595e1960538022d2dbf59bc6725a4ac13 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Sun, 22 Aug 2021 11:45:55 +0300 Subject: [PATCH 71/76] Fix CodeQL false positive Note sure why CodeQL believes request[128] cannot hold this format: "CONNECT %s:%u HTTP/1.1\r\nHost: %s:%u\r\n\r\n" At most, the sprintf() output happily fits in less than 100 chars: "CONNECT: 123.456.789.012:65535 HTTP/1.1\r\nHost: 123.456.789.012:65535\r\n\r\n" Not sure we can work around this false positive, but at least streamline gateway_port initialisation to make sure it is between 1 and 65535 and we catch all user errors. --- src/config.c | 8 ++++---- src/main.c | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/config.c b/src/config.c index 825febbf..23bc86b1 100644 --- a/src/config.c +++ b/src/config.c @@ -254,14 +254,14 @@ int load_config(struct vpn_config *cfg, const char *filename) strncpy(cfg->gateway_host, val, GATEWAY_HOST_SIZE); cfg->gateway_host[GATEWAY_HOST_SIZE] = '\0'; } else if (strcmp(key, "port") == 0) { - unsigned long port = strtoul(val, NULL, 0); + long port = strtol(val, NULL, 0); - if (port == 0 || port > 65535) { - log_warn("Bad port in configuration file: \"%lu\".\n", + if (port <= 0 || port > 65535) { + log_warn("Bad port in configuration file: \"%l\".\n", port); continue; } - cfg->gateway_port = port; + cfg->gateway_port = (uint16_t)port; } else if (strcmp(key, "username") == 0) { strncpy(cfg->username, val, USERNAME_SIZE); cfg->username[USERNAME_SIZE] = '\0'; diff --git a/src/main.c b/src/main.c index a3a1271b..1a12e519 100644 --- a/src/main.c +++ b/src/main.c @@ -591,13 +591,16 @@ int main(int argc, char **argv) host = argv[optind++]; port_str = strchr(host, ':'); if (port_str != NULL) { + long port; + port_str[0] = '\0'; port_str++; - cfg.gateway_port = strtol(port_str, NULL, 0); - if (cfg.gateway_port == 0 || cfg.gateway_port > 65535) { + port = strtol(port_str, NULL, 0); + if (port <= 0 || port > 65535) { log_error("Specify a valid port.\n"); goto user_error; } + cfg.gateway_port = (uint16_t)port; } strncpy(cfg.gateway_host, host, GATEWAY_HOST_SIZE); cfg.gateway_host[GATEWAY_HOST_SIZE] = '\0'; From 7b3482a6f8e4fd4856f2671ce098e0cdaa4cefd7 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Wed, 20 Oct 2021 08:24:24 +0200 Subject: [PATCH 72/76] Fix Coverity defect introduced by working around CodeQL false positive CID 373615: Invalid printf format string (PRINTF_ARGS) format_error: Invalid conversion specifier in %l". --- src/config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.c b/src/config.c index 23bc86b1..6cbba619 100644 --- a/src/config.c +++ b/src/config.c @@ -257,7 +257,7 @@ int load_config(struct vpn_config *cfg, const char *filename) long port = strtol(val, NULL, 0); if (port <= 0 || port > 65535) { - log_warn("Bad port in configuration file: \"%l\".\n", + log_warn("Bad port in configuration file: \"%ld\".\n", port); continue; } From cd93bcba1f9156100c3d9167a66e32f2f193ad64 Mon Sep 17 00:00:00 2001 From: Simon Tegelid Date: Wed, 27 Oct 2021 11:13:00 +0200 Subject: [PATCH 73/76] Allow config from proc subst Use getline instead of strtok_r for reading config files, since getline reads the file incrementally and doesn't require the file to be read on beforehand. Files coming from process substitutions doesn't have a size since they are just stdout feeds from another process (or processes). Example use case: $ openfortivpn -c <(cat /path/to/a-config-file | grep -v remove_me) --- src/config.c | 39 ++++++++++++++------------------------- 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/src/config.c b/src/config.c index 6cbba619..de7ad82c 100644 --- a/src/config.c +++ b/src/config.c @@ -183,7 +183,9 @@ int load_config(struct vpn_config *cfg, const char *filename) int ret = ERR_CFG_UNKNOWN; FILE *file; struct stat stat; - char *buffer, *line, *saveptr = NULL; + char *line = NULL; + size_t len = 0; + ssize_t read; file = fopen(filename, "r"); if (file == NULL) @@ -193,31 +195,19 @@ int load_config(struct vpn_config *cfg, const char *filename) ret = ERR_CFG_SEE_ERRNO; goto err_close; } - if (stat.st_size == 0) { - ret = ERR_CFG_EMPTY_FILE; - goto err_close; - } - - buffer = malloc(stat.st_size + 1); - if (buffer == NULL) { - ret = ERR_CFG_NO_MEM; - goto err_close; - } - - // Copy all file contents at once - if (fread(buffer, stat.st_size, 1, file) != 1) { - ret = ERR_CFG_CANNOT_READ; - goto err_free; - } - - buffer[stat.st_size] = '\0'; // Read line by line - for (line = strtok_r(buffer, "\n", &saveptr); line != NULL; - line = strtok_r(NULL, "\n", &saveptr)) { + while ((read = getline(&line, &len, file)) != -1) { char *key, *equals, *val; int i; + // Ignore blank lines. We could argue that the string must be at least + // 3 chars to be valid, eg. 'x=\n' but let the rest of the function + // logic handle that. NOTE: getline includes the '\n' in the string, + // which is removed later on. + if (read < 2) + continue; + if (line[0] == '#') continue; @@ -460,18 +450,17 @@ int load_config(struct vpn_config *cfg, const char *filename) cfg->check_virtual_desktop = strdup(val); } else { log_warn("Bad key in configuration file: \"%s\".\n", key); - goto err_free; + goto err_close; } } ret = 0; -err_free: - free(buffer); err_close: if (fclose(file)) log_warn("Could not close %s (%s).\n", filename, strerror(errno)); - + if (line) + free(line); return ret; } From b070c0b7fcc3bc0e1f33988947637f7363e9f599 Mon Sep 17 00:00:00 2001 From: Simon Tegelid Date: Wed, 27 Oct 2021 19:32:25 +0200 Subject: [PATCH 74/76] Handle errno from getline --- src/config.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/config.c b/src/config.c index de7ad82c..2592a22e 100644 --- a/src/config.c +++ b/src/config.c @@ -454,13 +454,20 @@ int load_config(struct vpn_config *cfg, const char *filename) } } - ret = 0; + if (errno != 0) // From getline + ret = ERR_CFG_SEE_ERRNO; + else + ret = 0; err_close: - if (fclose(file)) + if (fclose(file)) { log_warn("Could not close %s (%s).\n", filename, strerror(errno)); - if (line) - free(line); + if (ret == ERR_CFG_SEE_ERRNO) { + // fclose just ruined the errno, so don't rely on it anymore. + ret = ERR_CFG_UNKNOWN; + } + } + free(line); return ret; } From 2fa5c584dda16931a7c15fa4b2429c261fb662ec Mon Sep 17 00:00:00 2001 From: Daniele Bortoluzzi <10025418+udanieli@users.noreply.github.com> Date: Fri, 29 Oct 2021 12:48:28 +0200 Subject: [PATCH 75/76] Getsockopt fixes (#953) * fix getsockopt() wrapper: need to specify the level, e.g. SO_KEEPALIVE is on SOL_SOCKET, while TCP_* options are on the IPPROTO_TCP level * include missing TCP header * better messages --- src/io.c | 2 +- src/tunnel.c | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/io.c b/src/io.c index 47d9267b..388a005e 100644 --- a/src/io.c +++ b/src/io.c @@ -647,7 +647,7 @@ int io_loop(struct tunnel *tunnel) */ if (setsockopt(tunnel->ssl_socket, IPPROTO_TCP, TCP_NODELAY, (const char *) &tcp_nodelay_flag, sizeof(int))) { - log_error("setsockopt: %s\n", strerror(errno)); + log_error("setsockopt TCP_NODELAY: %s\n", strerror(errno)); goto err_sockopt; } diff --git a/src/tunnel.c b/src/tunnel.c index 7aede6f2..a919858c 100644 --- a/src/tunnel.c +++ b/src/tunnel.c @@ -56,6 +56,7 @@ #endif #include #include +#include #include #include #include @@ -566,13 +567,12 @@ static int get_gateway_host_ip(struct tunnel *tunnel) return 0; } -static int tcp_getsockopt(int sockfd, int optname) +static int get_socket_option(int sockfd, int level, int optname) { int optval; socklen_t optlen = sizeof(optval); - if (getsockopt(sockfd, IPPROTO_TCP, optname, - (void *)&optval, &optlen)) + if (getsockopt(sockfd, level, optname, &optval, &optlen) < 0) return -1; assert(optlen == sizeof(optval)); return optval; @@ -599,42 +599,42 @@ static int tcp_connect(struct tunnel *tunnel) * Attempt to find default TCP socket options on different platforms. */ #ifdef SO_KEEPALIVE - ret = tcp_getsockopt(handle, SO_KEEPALIVE); + ret = get_socket_option(handle, SOL_SOCKET, SO_KEEPALIVE); if (ret < 0) log_warn("getsockopt: %s: %s\n", "SO_KEEPALIVE", strerror(errno)); else - log_debug("SO_KEEPALIVE: %d\n", ret); + log_debug("SO_KEEPALIVE: %s\n", (ret ? "ON" : "OFF")); #endif #ifdef TCP_KEEPIDLE - ret = tcp_getsockopt(handle, TCP_KEEPIDLE); + ret = get_socket_option(handle, IPPROTO_TCP, TCP_KEEPIDLE); if (ret < 0) log_warn("getsockopt: %s: %s\n", "TCP_KEEPIDLE", strerror(errno)); else log_debug("TCP_KEEPIDLE: %d\n", ret); #endif #ifdef TCP_KEEPALIVE - ret = tcp_getsockopt(handle, TCP_KEEPALIVE); + ret = get_socket_option(handle, IPPROTO_TCP, TCP_KEEPALIVE); if (ret < 0) log_warn("getsockopt: %s: %s\n", "TCP_KEEPALIVE", strerror(errno)); else log_debug("TCP_KEEPALIVE: %d\n", ret); #endif #ifdef TCP_KEEPINTVL - ret = tcp_getsockopt(handle, TCP_KEEPINTVL); + ret = get_socket_option(handle, IPPROTO_TCP, TCP_KEEPINTVL); if (ret < 0) log_warn("getsockopt: %s: %s\n", "TCP_KEEPINTVL", strerror(errno)); else log_debug("TCP_KEEPINTVL: %d\n", ret); #endif #ifdef TCP_KEEPCNT - ret = tcp_getsockopt(handle, TCP_KEEPCNT); + ret = get_socket_option(handle, IPPROTO_TCP, TCP_KEEPCNT); if (ret < 0) log_warn("getsockopt: %s: %s\n", "TCP_KEEPCNT", strerror(errno)); else log_debug("TCP_KEEPCNT: %d\n", ret); #endif #ifdef SO_SNDBUF - ret = tcp_getsockopt(handle, SO_SNDBUF); + ret = get_socket_option(handle, SOL_SOCKET, SO_SNDBUF); if (ret < 0) #ifndef __APPLE__ log_warn("getsockopt: %s: %s\n", "SO_SNDBUF", strerror(errno)); @@ -643,7 +643,7 @@ static int tcp_connect(struct tunnel *tunnel) log_debug("SO_SNDBUF: %d\n", ret); #endif #ifdef SO_RCVBUF - ret = tcp_getsockopt(handle, SO_RCVBUF); + ret = get_socket_option(handle, SOL_SOCKET, SO_RCVBUF); if (ret < 0) #ifndef __APPLE__ log_warn("getsockopt: %s: %s\n", "SO_RCVBUF", strerror(errno)); From d1b2801b6e866411398414be199c7648eb3e620b Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3234522+DimitriPapadopoulos@users.noreply.github.com> Date: Thu, 18 Nov 2021 07:08:43 +0100 Subject: [PATCH 76/76] Remove repeated word in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ace0a0ba..2c16198d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -188,7 +188,7 @@ On the master branch there may be changes that are not (yet) described here. ### 1.6.0 -* [-] Fix possible buffer overflow in in long requests +* [-] Fix possible buffer overflow in long requests * [~] Code improvements in terms of header inclusion and some other coverity warnings * [+] Add proxy support * [~] Use the compiled-in fixed full path to pppd