Address gemini-code-assist review comments for SSL keylog documentation

This commit addresses all review comments from gemini-code-assist on PR #5279:

1. Fixed FLUSH LOGS documentation - clarified that file is reopened for
   appending, not truncating, and updated the note about preserving contents

2. Fixed callback documentation - clarified that the callback attaches to
   all frontend connections, not just admin connections

3. Updated security warning - focused on passive eavesdropping and offline
   decryption as the primary threats

4. Fixed typo: proxyql_ip -> proxysql_ip in tcpdump example

5. Removed misleading @see HPKP link - HPKP is unrelated to NSS Key Log
   Format and is a deprecated feature

6. Updated NSS Key Log Format URL to use official MDN link instead of
   unofficial mirror

7. Fixed buffer size comment to accurately reflect 256-byte buffer and
   254-byte line length validation

8. Clarified fputs comment to emphasize the read lock's role in allowing
   concurrent writes from multiple threads
pull/5279/head
Rene Cannao 4 months ago
parent 442635b721
commit 8c90bda52a

@ -203,8 +203,8 @@ void proxysql_keylog_attach_callback(SSL_CTX* ssl_ctx);
- Uses `SSL_CTX_set_keylog_callback()` (OpenSSL 1.1.1+)
**Called from:**
- `MySQL_Session::handler()` for MySQL admin connections
- `PgSQL_Session::handler()` for PostgreSQL admin connections
- `MySQL_Session::handler()` for frontend MySQL connections
- `PgSQL_Session::handler()` for frontend PostgreSQL connections
---
@ -342,9 +342,7 @@ The keylog file contains **cryptographic secrets** that can decrypt ALL TLS traf
- Enough to decrypt past, present, and future connections
2. **Attack scenarios if compromised:**
- Passive eavesdropping on all TLS traffic
- Decryption of captured packets
- Man-in-the-middle attacks with captured packets
- Decryption of captured network traffic, exposing all plaintext data, including credentials, queries, and results.
3. **Recommended safeguards:**
- **File permissions:** `0600` (owner read/write only)

@ -188,9 +188,9 @@ PROXYSQL FLUSH LOGS;
This command:
1. Closes the current key log file
2. Reopens the file (truncating it if it exists)
2. Reopens the file for appending
**Note:** This is different from typical log rotation. The old file contents are **not preserved**. If you want to archive old key log files, rename/move them manually before running `FLUSH LOGS`.
**Note:** The file is reopened in append mode, so existing contents will be preserved. If you want to start with a fresh file, rename/move the old file manually before running `FLUSH LOGS`.
### Manual Log Rotation Example
@ -227,7 +227,7 @@ On the ProxySQL server, capture network traffic to a pcap file:
sudo tcpdump -i eth0 -w /tmp/proxysql_debug.pcap port 6033
# Or capture traffic between specific hosts
sudo tcpdump -i eth0 -w /tmp/proxysql_debug.pcap host client_ip and host proxyql_ip
sudo tcpdump -i eth0 -w /tmp/proxysql_debug.pcap host client_ip and host proxysql_ip
# Run for a specific duration
sudo timeout 60 tcpdump -i eth0 -w /tmp/proxysql_debug.pcap port 6033

@ -100,7 +100,6 @@ void proxysql_keylog_attach_callback(SSL_CTX* ssl_ctx);
*
* @note Line length is validated (max 254 bytes)
* @note Lines are newline-terminated if not already present
* @see https://developer.mozilla.org/en-US/docs/Web/HTTP/HPKP
*/
void proxysql_keylog_write_line_callback(const SSL* ssl, const char* line);

@ -25,7 +25,7 @@
#include "proxysql_sslkeylog.h"
// NSS Key Log Format reference:
// http://udn.realityripple.com/docs/Mozilla/Projects/NSS/Key_Log_Format
// https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/Key_Log_Format
#define KEYLOG_LABEL_MAXLEN (sizeof("CLIENT_HANDSHAKE_TRAFFIC_SECRET") - 1)
@ -176,7 +176,10 @@ void proxysql_keylog_write_line_callback(const SSL *ssl, const char *line)
// The check is repeated after acquiring the lock for correctness
if (!keylog_file_fp) return;
/* The current maximum valid keylog line length including LF and NUL is 195. */
/* The line is written to a 256-byte buffer. The maximum line length
* from OpenSSL is validated to not exceed 254 bytes to allow for a
* newline and null terminator.
*/
size_t linelen;
char buf[256];
@ -197,7 +200,7 @@ void proxysql_keylog_write_line_callback(const SSL *ssl, const char *line)
}
buf[linelen] = '\0';
/* fputs is used because we're using rwlock (multiple readers allowed) */
/* The read lock allows multiple threads to write to the file concurrently. */
fputs(buf, keylog_file_fp);
__exit:

Loading…
Cancel
Save