Use hardening option to add only hardening flags
authorKirill Isakov <bootctl@gmail.com>
Thu, 28 Apr 2022 12:40:54 +0000 (18:40 +0600)
committerKirill Isakov <bootctl@gmail.com>
Thu, 28 Apr 2022 19:18:39 +0000 (01:18 +0600)
Compiler flags that enable warnings were hidden behind the 'hardening'
option. This was a direct port of the previous autoconf config, but it
probably makes sense to always show them as they have no effect on
compiler's output (unlike hardening flags which distributors might want
to alter or disable completely).

Also remove -fwrapv (which is enabled by -fno-strict-overflow) and fix
all new warnings we've added (like VLAs in libgcrypt code, so it should
now be buildable with MSVC in case anyone wishes to do it).

22 files changed:
meson.build
src/compression.h
src/fsck.c
src/gcrypt/cipher.c
src/gcrypt/digest.c
src/gcrypt/pem.c
src/gcrypt/prf.c
src/gcrypt/rsa.c
src/gcrypt/rsa.h
src/gcrypt/rsagen.c
src/ifconfig.c
src/logger.c
src/net_packet.c
src/protocol_auth.c
src/script.h
src/sptps.c
src/sptps.h
src/sptps_test.c
src/tincctl.c
src/tincctl.h
src/tincd.c
src/xalloc.h

index 85d1612..43ae974 100644 (file)
@@ -50,6 +50,34 @@ endif
 cc_flags = [cc_defs]
 ld_flags = []
 
+if cc_name != 'msvc'
+  cc_flags += [
+    '-Qunused-arguments',
+    '-Wbad-function-cast',
+    '-Wduplicated-branches',
+    '-Wduplicated-cond',
+    '-Wformat-overflow=2',
+    '-Wformat-truncation=1', # 2 prints too much noise
+    '-Wformat=2',
+    '-Wlogical-op',
+    '-Wmissing-declarations',
+    '-Wmissing-noreturn',
+    '-Wmissing-prototypes',
+    '-Wno-embedded-directive',
+    '-Wold-style-definition',
+    '-Wredundant-decls',
+    '-Wreturn-type',
+    '-Wstrict-prototypes',
+    '-Wswitch-enum',
+    '-Wtrampolines', # may require executable stack which is disabled
+    '-Wvla', # VLAs are not supported by MSVC
+    '-Wwrite-strings',
+    '-fdiagnostics-show-option',
+    '-fno-strict-overflow',
+    '-fstrict-aliasing',
+  ]
+endif
+
 if opt_static.auto()
   static = os_name == 'windows'
 else
@@ -74,26 +102,15 @@ if opt_harden
   else
     cc_flags += [
       '-D_FORTIFY_SOURCE=2',
-      '-fwrapv',
-      '-fno-strict-overflow',
-      '-Wreturn-type',
-      '-Wold-style-definition',
-      '-Wmissing-declarations',
-      '-Wmissing-prototypes',
-      '-Wstrict-prototypes',
-      '-Wredundant-decls',
-      '-Wbad-function-cast',
-      '-Wwrite-strings',
-      '-fdiagnostics-show-option',
-      '-fstrict-aliasing',
-      '-Wmissing-noreturn',
+      '-fcf-protection=full',
+      '-fstack-protector-strong',
     ]
-    if cc_name == 'clang'
-      cc_flags += '-Qunused-arguments'
-    endif
-    ld_flags += ['-Wl,-z,relro', '-Wl,-z,now']
+    ld_flags += ['-Wl,-z,relro', '-Wl,-z,now', '-Wl,-z,noexecstack']
     if os_name == 'windows'
       ld_flags += ['-Wl,--dynamicbase', '-Wl,--nxcompat']
+    else
+      # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90458
+      cc_flags += '-fstack-clash-protection'
     endif
   endif
 endif
index 98eca33..7fe5bb6 100644 (file)
@@ -18,8 +18,6 @@ typedef enum compression_level_t {
        COMPRESS_LZO_HI = 11,
 
        COMPRESS_LZ4 = 12,
-
-       COMPRESS_GUARD = INT_MAX, /* ensure that sizeof(compression_level_t) == sizeof(int) */
 } compression_level_t;
 
 STATIC_ASSERT(sizeof(compression_level_t) == sizeof(int), "compression_level_t has invalid size");
index 8d818a3..70ca84d 100644 (file)
@@ -64,6 +64,7 @@ again:
        goto again;
 }
 
+static void print_tinc_cmd(const char *format, ...) ATTR_FORMAT(printf, 1, 2);
 static void print_tinc_cmd(const char *format, ...) {
        if(confbasegiven) {
                fprintf(stderr, "%s -c %s ", exe_name, confbase);
index 2813da6..ee3856a 100644 (file)
@@ -208,7 +208,7 @@ bool cipher_set_key_from_rsa(cipher_t *cipher, void *key, size_t len, bool encry
 
 bool cipher_encrypt(cipher_t *cipher, const void *indata, size_t inlen, void *outdata, size_t *outlen, bool oneshot) {
        gcry_error_t err;
-       uint8_t pad[cipher->blklen];
+       uint8_t *pad = alloca(cipher->blklen);
 
        if(cipher->padding) {
                if(!oneshot) {
index cfea1f0..e029168 100644 (file)
@@ -149,7 +149,7 @@ bool digest_create(digest_t *digest, const void *indata, size_t inlen, void *out
 
                memcpy(outdata, tmpdata, digest->maclength);
        } else {
-               char tmpdata[len];
+               char *tmpdata = alloca(len);
                gcry_md_hash_buffer(digest->algo, tmpdata, indata, inlen);
                memcpy(outdata, tmpdata, digest->maclength);
        }
@@ -159,7 +159,7 @@ bool digest_create(digest_t *digest, const void *indata, size_t inlen, void *out
 
 bool digest_verify(digest_t *digest, const void *indata, size_t inlen, const void *cmpdata) {
        size_t len = digest->maclength;
-       uint8_t outdata[len];
+       uint8_t *outdata = alloca(len);
 
        return digest_create(digest, indata, inlen, outdata) && !memcmp(cmpdata, outdata, len);
 }
index 0f7763c..56858f2 100644 (file)
@@ -93,7 +93,7 @@ bool pem_encode(FILE *fp, const char *header, uint8_t *buf, size_t size) {
                return false;
        }
 
-       char b64[B64_SIZE(size)];
+       char *b64 = alloca(B64_SIZE(size));
        const size_t b64len = b64encode(b64, buf, size);
 
        for(size_t i = 0; i < b64len; i += 64) {
index 1e5bb9f..d11ef77 100644 (file)
@@ -32,7 +32,9 @@ static const size_t mdlen = 64;
 static const size_t blklen = 128;
 
 static bool hmac_sha512(const uint8_t *key, size_t keylen, const uint8_t *msg, size_t msglen, uint8_t *out) {
-       uint8_t tmp[blklen + mdlen];
+       const size_t tmplen = blklen + mdlen;
+       uint8_t *tmp = alloca(tmplen);
+
        sha512_context md;
 
        if(keylen <= blklen) {
@@ -69,7 +71,7 @@ static bool hmac_sha512(const uint8_t *key, size_t keylen, const uint8_t *msg, s
        // opad
        memxor(tmp, 0x36 ^ 0x5c, blklen);
 
-       if(sha512(tmp, sizeof(tmp), out) != 0) {
+       if(sha512(tmp, tmplen, out) != 0) {
                return false;
        }
 
@@ -86,28 +88,30 @@ bool prf(const uint8_t *secret, size_t secretlen, uint8_t *seed, size_t seedlen,
           It consists of the previous HMAC result plus the seed.
         */
 
-       uint8_t data[mdlen + seedlen];
+       const size_t datalen = mdlen + seedlen;
+       uint8_t *data = alloca(datalen);
+
        memset(data, 0, mdlen);
        memcpy(data + mdlen, seed, seedlen);
 
-       uint8_t hash[mdlen];
+       uint8_t *hash = alloca(mdlen);
 
        while(outlen > 0) {
                /* Inner HMAC */
-               if(!hmac_sha512(secret, secretlen, data, sizeof(data), data)) {
+               if(!hmac_sha512(secret, secretlen, data, datalen, data)) {
                        return false;
                }
 
                /* Outer HMAC */
                if(outlen >= mdlen) {
-                       if(!hmac_sha512(secret, secretlen, data, sizeof(data), out)) {
+                       if(!hmac_sha512(secret, secretlen, data, datalen, out)) {
                                return false;
                        }
 
                        out += mdlen;
                        outlen -= mdlen;
                } else {
-                       if(!hmac_sha512(secret, secretlen, data, sizeof(data), hash)) {
+                       if(!hmac_sha512(secret, secretlen, data, datalen, hash)) {
                                return false;
                        }
 
index 04aa358..292c773 100644 (file)
@@ -111,8 +111,12 @@ static bool ber_read_mpi(unsigned char **p, size_t *buflen, gcry_mpi_t *mpi) {
        return mpi ? !err : true;
 }
 
+rsa_t *rsa_new(void) {
+       return xzalloc(sizeof(rsa_t));
+}
+
 rsa_t *rsa_set_hex_public_key(const char *n, const char *e) {
-       rsa_t *rsa = xzalloc(sizeof(rsa_t));
+       rsa_t *rsa = rsa_new();
 
        gcry_error_t err = gcry_mpi_scan(&rsa->n, GCRYMPI_FMT_HEX, n, 0, NULL);
 
@@ -130,7 +134,7 @@ rsa_t *rsa_set_hex_public_key(const char *n, const char *e) {
 }
 
 rsa_t *rsa_set_hex_private_key(const char *n, const char *e, const char *d) {
-       rsa_t *rsa = xzalloc(sizeof(rsa_t));
+       rsa_t *rsa = rsa_new();
 
        gcry_error_t err = gcry_mpi_scan(&rsa->n, GCRYMPI_FMT_HEX, n, 0, NULL);
 
@@ -162,7 +166,7 @@ rsa_t *rsa_read_pem_public_key(FILE *fp) {
                return NULL;
        }
 
-       rsa_t *rsa = xzalloc(sizeof(rsa_t));
+       rsa_t *rsa = rsa_new();
 
        if(!ber_skip_sequence(&derp, &derlen)
                        || !ber_read_mpi(&derp, &derlen, &rsa->n)
@@ -185,7 +189,7 @@ rsa_t *rsa_read_pem_private_key(FILE *fp) {
                return NULL;
        }
 
-       rsa_t *rsa = xzalloc(sizeof(rsa_t));
+       rsa_t *rsa = rsa_new();
 
        if(!ber_skip_sequence(&derp, &derlen)
                        || !ber_read_mpi(&derp, &derlen, NULL)
index e6bb5ec..14f0381 100644 (file)
@@ -22,6 +22,9 @@
 
 #include <gcrypt.h>
 
+#include "../rsa.h"
+#include "../xalloc.h"
+
 #define TINC_RSA_INTERNAL
 typedef struct rsa {
        gcry_mpi_t n;
@@ -29,4 +32,6 @@ typedef struct rsa {
        gcry_mpi_t d;
 } rsa_t;
 
+extern rsa_t *rsa_new(void) ATTR_MALLOC ATTR_DEALLOCATOR(rsa_free);
+
 #endif
index b89225d..3469c33 100644 (file)
@@ -297,7 +297,7 @@ rsa_t *rsa_generate(size_t bits, unsigned long exponent) {
                return NULL;
        }
 
-       rsa_t *rsa = xzalloc(sizeof(*rsa));
+       rsa_t *rsa = rsa_new();
 
        rsa->n = find_mpi(s_rsa, "n");
        rsa->e = find_mpi(s_rsa, "e");
index f4ce313..d4f1386 100644 (file)
@@ -105,6 +105,7 @@ void ifconfig_address(FILE *out, const char *value) {
                ipv6 = address;
                break;
 
+       case SUBNET_MAC:
        default:
                return;
        }
@@ -208,6 +209,7 @@ void ifconfig_route(FILE *out, const char *value) {
                        fprintf(out, "ip route add %s via %s dev \"$INTERFACE\" onlink\n", subnet_str, gateway_str);
                        break;
 
+               case SUBNET_MAC:
                default:
                        return;
                }
@@ -221,6 +223,7 @@ void ifconfig_route(FILE *out, const char *value) {
                        fprintf(out, "ip route add %s dev \"$INTERFACE\"\n", subnet_str);
                        break;
 
+               case SUBNET_MAC:
                default:
                        return;
                }
@@ -238,6 +241,7 @@ void ifconfig_route(FILE *out, const char *value) {
                        fprintf(out, "netsh interface ipv6 add route %s \"%%INTERFACE%%\" %s\n", subnet_str, gateway_str);
                        break;
 
+               case SUBNET_MAC:
                default:
                        return;
                }
@@ -251,6 +255,7 @@ void ifconfig_route(FILE *out, const char *value) {
                        fprintf(out, "netsh interface ipv6 add route %s \"%%INTERFACE%%\"\n", subnet_str);
                        break;
 
+               case SUBNET_MAC:
                default:
                        return;
                }
@@ -278,6 +283,7 @@ void ifconfig_route(FILE *out, const char *value) {
                        net2str(gateway_str, sizeof(gateway_str), &ipv6);
                        break;
 
+               case SUBNET_MAC:
                default:
                        return;
                }
@@ -298,6 +304,7 @@ void ifconfig_route(FILE *out, const char *value) {
                fprintf(out, "route add -inet6 %s %s\n", subnet_str, gateway_str);
                break;
 
+       case SUBNET_MAC:
        default:
                return;
        }
index 3e630f2..8650a1c 100644 (file)
@@ -239,6 +239,7 @@ void logger(debug_t level, int priority, const char *format, ...) {
        real_logger(level, priority, message);
 }
 
+static void sptps_logger(sptps_t *s, int s_errno, const char *format, va_list ap) ATTR_FORMAT(printf, 3, 0);
 static void sptps_logger(sptps_t *s, int s_errno, const char *format, va_list ap) {
        (void)s_errno;
        char message[1024];
index 28459ce..6fea959 100644 (file)
@@ -1643,6 +1643,7 @@ void broadcast_packet(const node_t *from, vpn_packet_t *packet) {
 
                break;
 
+       case BMODE_NONE:
        default:
                break;
        }
index d7cbbd4..7a3e6dd 100644 (file)
@@ -82,6 +82,7 @@ static bool send_proxyrequest(connection_t *c) {
        case PROXY_EXEC:
                return true;
 
+       case PROXY_NONE:
        default:
                logger(DEBUG_ALWAYS, LOG_ERR, "Unknown proxy type");
                return false;
index 381f952..4d10d2e 100644 (file)
@@ -29,8 +29,8 @@ typedef struct environment {
        char **entries;
 } environment_t;
 
-extern int environment_add(environment_t *env, const char *format, ...);
-extern void environment_update(environment_t *env, int pos, const char *format, ...);
+extern int environment_add(environment_t *env, const char *format, ...) ATTR_FORMAT(printf, 2, 3);
+extern void environment_update(environment_t *env, int pos, const char *format, ...) ATTR_FORMAT(printf, 3, 4);
 extern void environment_init(environment_t *env);
 extern void environment_exit(environment_t *env);
 
index 42eaba0..35c68bc 100644 (file)
@@ -68,6 +68,7 @@ void sptps_log_stderr(sptps_t *s, int s_errno, const char *format, va_list ap) {
 void (*sptps_log)(sptps_t *s, int s_errno, const char *format, va_list ap) = sptps_log_stderr;
 
 // Log an error message.
+static bool error(sptps_t *s, int s_errno, const char *format, ...) ATTR_FORMAT(printf, 3, 4);
 static bool error(sptps_t *s, int s_errno, const char *format, ...) {
        (void)s;
        (void)s_errno;
@@ -83,6 +84,7 @@ static bool error(sptps_t *s, int s_errno, const char *format, ...) {
        return false;
 }
 
+static void warning(sptps_t *s, const char *format, ...) ATTR_FORMAT(printf, 2, 3);
 static void warning(sptps_t *s, const char *format, ...) {
        va_list ap;
        va_start(ap, format);
@@ -640,7 +642,7 @@ size_t sptps_receive_data(sptps_t *s, const void *vdata, size_t len) {
                s->inbuf = realloc(s->inbuf, s->reclen + 19UL);
 
                if(!s->inbuf) {
-                       return error(s, errno, strerror(errno));
+                       return error(s, errno, "%s", strerror(errno));
                }
 
                // Exit early if we have no more data to process.
@@ -718,7 +720,7 @@ bool sptps_start(sptps_t *s, void *handle, bool initiator, bool datagram, ecdsa_
                s->late = malloc(s->replaywin);
 
                if(!s->late) {
-                       return error(s, errno, strerror(errno));
+                       return error(s, errno, "%s", strerror(errno));
                }
 
                memset(s->late, 0, s->replaywin);
@@ -727,14 +729,14 @@ bool sptps_start(sptps_t *s, void *handle, bool initiator, bool datagram, ecdsa_
        s->label = malloc(labellen);
 
        if(!s->label) {
-               return error(s, errno, strerror(errno));
+               return error(s, errno, "%s", strerror(errno));
        }
 
        if(!datagram) {
                s->inbuf = malloc(7);
 
                if(!s->inbuf) {
-                       return error(s, errno, strerror(errno));
+                       return error(s, errno, "%s", strerror(errno));
                }
 
                s->buflen = 0;
index 95209e5..633eb81 100644 (file)
@@ -104,9 +104,9 @@ typedef struct sptps {
 } sptps_t;
 
 extern unsigned int sptps_replaywin;
-extern void sptps_log_quiet(sptps_t *s, int s_errno, const char *format, va_list ap);
-extern void sptps_log_stderr(sptps_t *s, int s_errno, const char *format, va_list ap);
-extern void (*sptps_log)(sptps_t *s, int s_errno, const char *format, va_list ap);
+extern void sptps_log_quiet(sptps_t *s, int s_errno, const char *format, va_list ap) ATTR_FORMAT(printf, 3, 0);
+extern void sptps_log_stderr(sptps_t *s, int s_errno, const char *format, va_list ap) ATTR_FORMAT(printf, 3, 0);
+extern void (*sptps_log)(sptps_t *s, int s_errno, const char *format, va_list ap) ATTR_FORMAT(printf, 3, 0);
 extern bool sptps_start(sptps_t *s, void *handle, bool initiator, bool datagram, ecdsa_t *mykey, ecdsa_t *hiskey, const void *label, size_t labellen, send_data_t send_data, receive_record_t receive_record);
 extern bool sptps_stop(sptps_t *s);
 extern bool sptps_send_record(sptps_t *s, uint8_t type, const void *data, uint16_t len);
index b26615e..38e8b50 100644 (file)
@@ -154,7 +154,7 @@ static struct option const long_options[] = {
 };
 
 static void usage(void) {
-       static const char *message =
+       fprintf(stderr,
                "Usage: %s [options] my_ed25519_key_file his_ed25519_key_file [host] port\n"
                "\n"
                "Valid options are:\n"
@@ -172,9 +172,8 @@ static void usage(void) {
                "  -4                      Use IPv4.\n"
                "  -6                      Use IPv6.\n"
                "\n"
-               "Report bugs to tinc@tinc-vpn.org.\n";
-
-       fprintf(stderr, message, program_name);
+               "Report bugs to tinc@tinc-vpn.org.\n",
+               program_name);
 }
 
 #ifdef HAVE_WINDOWS
index b2d98f6..afe19cd 100644 (file)
@@ -108,7 +108,7 @@ static struct option const long_options[] = {
 };
 
 static void version(void) {
-       static const char *message =
+       fprintf(stdout,
                "%s version %s (built %s %s, protocol %d.%d)\n"
                "Features:"
 #ifdef HAVE_READLINE
@@ -126,16 +126,15 @@ static void version(void) {
                "\n"
                "tinc comes with ABSOLUTELY NO WARRANTY.  This is free software,\n"
                "and you are welcome to redistribute it under certain conditions;\n"
-               "see the file COPYING for details.\n";
-
-       printf(message, PACKAGE, BUILD_VERSION, BUILD_DATE, BUILD_TIME, PROT_MAJOR, PROT_MINOR);
+               "see the file COPYING for details.\n",
+               PACKAGE, BUILD_VERSION, BUILD_DATE, BUILD_TIME, PROT_MAJOR, PROT_MINOR);
 }
 
 static void usage(bool status) {
        if(status) {
                fprintf(stderr, "Try `%s --help\' for more information.\n", program_name);
        } else {
-               static const char *message =
+               fprintf(stdout,
                        "Usage: %s [options] command\n"
                        "\n"
                        "Valid options are:\n"
@@ -194,9 +193,8 @@ static void usage(bool status) {
                        "  sign [FILE]                Generate a signed version of a file.\n"
                        "  verify NODE [FILE]         Verify that a file was signed by the given NODE.\n"
                        "\n"
-                       "Report bugs to tinc@tinc-vpn.org.\n";
-
-               printf(message, program_name);
+                       "Report bugs to tinc@tinc-vpn.org.\n",
+                       program_name);
        }
 }
 
index 9bacc46..94df21d 100644 (file)
@@ -49,7 +49,7 @@ extern const var_t variables[];
 extern size_t rstrip(char *value);
 extern char *get_my_name(bool verbose) ATTR_MALLOC;
 extern bool connect_tincd(bool verbose);
-extern bool sendline(int fd, const char *format, ...);
+extern bool sendline(int fd, const char *format, ...) ATTR_FORMAT(printf, 2, 3);
 extern bool recvline(int fd, char *line, size_t len);
 extern int check_port(const char *name);
 
index 06e6713..edb03f5 100644 (file)
@@ -130,7 +130,7 @@ static void usage(bool status) {
                fprintf(stderr, "Try `%s --help\' for more information.\n",
                        program_name);
        else {
-               static const char *message =
+               fprintf(stdout,
                        "Usage: %s [option]...\n"
                        "\n"
                        "  -c, --config=DIR              Read configuration options from DIR.\n"
@@ -152,9 +152,8 @@ static void usage(bool status) {
                        "      --help                    Display this help and exit.\n"
                        "      --version                 Output version information and exit.\n"
                        "\n"
-                       "Report bugs to tinc@tinc-vpn.org.\n";
-
-               printf(message, program_name);
+                       "Report bugs to tinc@tinc-vpn.org.\n",
+                       program_name);
        }
 }
 
@@ -419,7 +418,7 @@ int main(int argc, char **argv) {
        }
 
        if(show_version) {
-               static const char *message =
+               fprintf(stdout,
                        "%s version %s (built %s %s, protocol %d.%d)\n"
                        "Features:"
 #ifdef HAVE_OPENSSL
@@ -461,9 +460,8 @@ int main(int argc, char **argv) {
                        "\n"
                        "tinc comes with ABSOLUTELY NO WARRANTY.  This is free software,\n"
                        "and you are welcome to redistribute it under certain conditions;\n"
-                       "see the file COPYING for details.\n";
-
-               printf(message, PACKAGE, BUILD_VERSION, BUILD_DATE, BUILD_TIME, PROT_MAJOR, PROT_MINOR);
+                       "see the file COPYING for details.\n",
+                       PACKAGE, BUILD_VERSION, BUILD_DATE, BUILD_TIME, PROT_MAJOR, PROT_MINOR);
                return 0;
        }
 
index c8cfbeb..2b217e8 100644 (file)
@@ -68,6 +68,7 @@ static inline char *xstrdup(const char *s) {
        return p;
 }
 
+static inline int xvasprintf(char **strp, const char *fmt, va_list ap) ATTR_FORMAT(printf, 2, 0);
 static inline int xvasprintf(char **strp, const char *fmt, va_list ap) {
 #ifdef HAVE_WINDOWS
        char buf[1024];