Ref https://www.sqlite.org/c3ref/initialize.html:
The sqlite3_initialize() interface is threadsafe, but
sqlite3_shutdown() is not.
We currently call `sqlite3_shutdown` from all threads as part of
`sqlite_init_multithreaded`, and this has been observed to have
adversarial effects during startup if many threads receive their first
inbound request at the same time. The apparent motivation behind
calling shutdown is to make the subsequent calls to `sqlite3_config`
succeed, since these ordinarily return SQLITE_MISUSE if called
multiple times. However, this function is also documented to not be
thread safe, so introduce a barrier that ensures we only initialize
once over all threads.
If the configuration contains entries such as:
no-tlsv1
no-tlsv1_1
then the SSL context is NULL. The previous code was simple enough that it
handled this case; the new code needs to deal with it explicitly.
[*] https://github.com/coturn/coturn/issues/770
Because:
1. According to sqlite3 docs sqlite3_initialize() and sqlite3_shutdown() are not must to be invoked
2. sqlite3_initialize() is never called explicilty
3. sqlite3_shutdown() is not threadsafe and sqlite_init_multithreaded is not called holding a lock
4. According to docs all connections must be closed before invoking sqlite3_shutdown() but they are not (from the different threads).
Possible issue:
sqlite3_config must be called before sqlite3_initialize() or after sqlite3_shutdown() (and only once?)
When openssl_load_certificates() is called as a result of USR2
signal, it has the effect of SSL_free() on certificates.
But pointers to these certificates are borrowed by the ioa_engines
where they are used for new connections.
The tls_mutex when loading the certificates does not prevent this use
because it's released before despatching asynchronous events to each
ioa_engine asking them to pick up the new SSL context.
So there is a race; if a new connection arrives quickly after
openssl_load_certificates() but before the tls_ctx_update_ev.
This patch resolves this using OpenSSL's own fine grained locking.
The ioa_engines now 'copy' the SSL context (actually a refcounted copy)
When SSL certificates are renewed during runtime (via SIGUSR2),
e->dtls_ctx is replaced with a context based on the new certificate.
But this code continues to operate on its own borrowed pointer.
This is clearly visible using valgrind, but the bug is subtle and not
always noticed at runtime, possibly due to some fortunate re-use of
memory.
At the point of SSL_new():
==28413== Thread 5:
==28413== Invalid read of size 8
==28413== at 0x4F6198F: SSL_new (in /lib/libssl.so.1.1)
==28413== by 0x137A72: dtls_server_input_handler (dtls_listener.c:291)
==28413== by 0x137A72: handle_udp_packet (dtls_listener.c:443)
==28413== by 0x138153: udp_server_input_handler (dtls_listener.c:728)
==28413== by 0x4FC499E: ??? (in /usr/lib/libevent_core-2.1.so.7.0.0)
==28413== by 0x4FC50AF: event_base_loop (in /usr/lib/libevent_core-2.1.so.7.0.0)
==28413== by 0x121F34: run_events (netengine.c:1579)
==28413== by 0x121F34: run_general_relay_thread (netengine.c:1707)
==28413== by 0x40517B6: start (pthread_create.c:195)
==28413== by 0x40538EF: ??? (clone.s:22)
==28413== Address 0x49a75e0 is 0 bytes inside a block of size 1,024 free'd
==28413== at 0x48A074F: free (vg_replace_malloc.c:540)
==28413== by 0x4F5F6F1: SSL_CTX_free (in /lib/libssl.so.1.1)
==28413== by 0x11CEC4: set_ctx (mainrelay.c:3104)
==28413== by 0x11D233: openssl_load_certificates (mainrelay.c:3173)
==28413== by 0x11D328: reload_ssl_certs (mainrelay.c:3190)
==28413== by 0x4FC4601: ??? (in /usr/lib/libevent_core-2.1.so.7.0.0)
==28413== by 0x4FC50AF: event_base_loop (in /usr/lib/libevent_core-2.1.so.7.0.0)
==28413== by 0x122582: run_events (netengine.c:1579)
==28413== by 0x122582: run_listener_server (netengine.c:1603)
==28413== by 0x110BB8: main (mainrelay.c:2536)
==28413== Block was alloc'd at
==28413== at 0x489F72A: malloc (vg_replace_malloc.c:309)
==28413== by 0x4DFA2C6: CRYPTO_zalloc (in /lib/libcrypto.so.1.1)
==28413== by 0x4F5F79E: SSL_CTX_new (in /lib/libssl.so.1.1)
==28413== by 0x11CA80: set_ctx (mainrelay.c:2875)
==28413== by 0x11D233: openssl_load_certificates (mainrelay.c:3173)
==28413== by 0x110A19: openssl_setup (mainrelay.c:3139)
==28413== by 0x110A19: main (mainrelay.c:2396)
==28413==
Multiple DTLS listener servers are created, and server->dtls_ctx is
the same object shared between them.
Set these callbacks once, and logically this is at the point where the
SSL context is created.
This adds the `--new-log-timestamp` and `--new-timestamp-format <value>` options
to the `turnserver` program.
Setting `--new-log-timestamp` on the command line, or `new-log-timestamp` in the
configuration file, will cause all logs to be written with an ISI-8601 timestamp
(`YYYY-MM-DDTHH:MM:SSZZZZZ` with `T` being literal and `ZZZZZ` being `+` or `-`
and the hour and minute offset from GMT for the local timezone). This replaces
the 'number of seconds since daemon was started' format.
Setting the `--new-timestamp-format <format>` option with a given format, or
`new-log-timestamp=<format>` in the configuration file, will use this instead
of the standard timestamp format. Timestamp format strings up to 48 characters
can be accommodated; more will be truncated. This will only be used when the
`--new-log-timestamp` option (above) is set.
Thanks to Hendrik Huels <hendrik.huels@outlook.de> for the idea and some of the
code for setting the log timestamp string.
Signed-off-by: Paul Wayper <paulway@mabula.net>
The function `turn_log_func_default` calls the function `vrtpprintf` to print to syslog
or the log file. The latter does exactly the same string formatting as the former, so
here we merge the two functions into one to do the string formatting once. This also
makes sure that the log line is consistent on all outputs.
Signed-off-by: Paul Wayper <paulway@mabula.net>
This provides the 'use_new_timestamp_log_format' variable in `ns_turn_utils.h`. By
default it is set to 0 and the old 'seconds since daemon was started' timestamp will
be emitted. However, if it is set to 1 or any 'true' number the new date and time
timestamp format will be used instead.
This has also resulted in a small clean-up of some of the string length handling.
Signed-off-by: Paul Wayper <paulway@mabula.net>
As discussed in https://github.com/coturn/coturn/pull/478, if the
parameter only controls whether or not to send the software attribute
and not other production-relevant configurations, it should be named
accordingly.
The old --prod configuration option still works, but is now deprecated
and undocumented.
* Changed type from int to size_t to avoid warning
warning: comparison between signed and unsigned integer expressions
* Fixed string truncation warning
Previously this was being done in stun_attr_get_next_str() to check that the previous attribute didn't exceed the size of the underlying buffer, however by that point any maliciously crafted attributes would have already had their chance to attack the caller.
