Skip to content

Commit 78666a2

Browse files
lthfacebook-github-bot
authored andcommitted
Prevent cert/key mismatch is combined file replaced between cert load and key load
Summary: MySQL was using the functions that take a file as input for setting the ssl cert and key. There is a race condition if the cert file gets replaced between the moment the cert is loaded and the key is loaded. Fix: open file once and load cert and key. Used BIO functions that work on file descriptors. Reference patch: 53d0673 Reviewed By: abal147 Differential Revision: D7299948 fbshipit-source-id: 96022f3
1 parent ce7b2a1 commit 78666a2

File tree

4 files changed

+152
-31
lines changed

4 files changed

+152
-31
lines changed
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
-----BEGIN RSA PRIVATE KEY-----
2+
MIIEpQIBAAKCAQEA6SDnBVEHOki54mKiJEUf8VQ0IlxihpsB6MJFkACNTuSi6unX
3+
tZUlzhisGk+z5jBGryATQITfIdzfCeWgf4ESbh6EWFyhEduqtgTg/B4MES7zMGIa
4+
+e7f/qPT1oNureiNmImxaWO4cvRa5iJec2SV71aSkgvhk6XXTEFH5zHtCWizxW7B
5+
GwE5u/GPv7r0AuLl5Z631pqylHabSNgnGA6fMJj1mj4j5jtKSO6lJuiAlDfkSrr/
6+
n0K5Mt16nWMRqCWZsR+G5XyxMeMSEQzr9h0CSx00y3Qdfy9AwIHiBNSO6pb4IjWO
7+
q7aZMzbvtYMRiG0GD3ZNv9uo32w8keK6c6BzRQIDAQABAoIBAQDUbdzVJV6Wp4pq
8+
VUI2Fp7iwr22ycQlr71voQbODxK0XvZtZKPgnIWUZTr9xr7A9CCUl3+zfN/t9Vtv
9+
o0Q6qxxmJ3ylH9LNeQL3VT7FvYN1bPjAj8TRFfAaEqKHh8AkzBGqe12kEPAUH8Fs
10+
jsjOEUvmiVaJqjXk2mty2tFwRDggJwCrN5bXkhkzwhDcMfH2Wgc4c4XkyUrciJQU
11+
ua4d0L354B3UmRYtrzwPr6WHLXCGPGhyWvXYpDjjdUGMVf2YcBSQdABF+mhCEb2b
12+
NP4dYUqKHjKn6p4B1/qfJtf0c9Lz229nz0WTzanmpXaNxQVce0sTbktp5A3itT+m
13+
NlQfDNoBAoGBAPvqSK7NqCrvFYEo+Cvl6fOhq9li2zAeaYO7D+AKiWSJzG9KK/Ts
14+
F+28nnWkBEdzAnmgWZ7UZlUwHqF6DNIGn+RLHDJ2MVRrZug2irCR8g2mxcHk2dss
15+
DcmUtsatjCbjLqVCcyuuQylP2GWK60JmRbdKEOfpHLntzGStpOhn5FPBAoGBAOzo
16+
okk2FWZlymJTkN2HYTqvUCYINDciTDm/ms7YGC6YKdDJ8PUVq6qJ2GO/M+zGPQtV
17+
A+qFWqx1kk3K5uLPnZLCPLORXPIm0X1ZGreG+rHsrJTnP6uh9OxrTyLNkvt6xcm7
18+
yA51QOWTuRbYhPwy05IqT3Z88HkHByMKr4xafPCFAoGADff1w8ufkZHkTV8qM7Tx
19+
/hJu5wT2RnrJOwa6YJ/08mA5t8oTGeelhAc7eiZ4HkYgUwIzNf1tFzgt2qJb56F6
20+
aDxJ+fpXzeiOsj2j/xp4o40l1hSMh/yvXwgiAm5JITbjtUI0BK4LB1VoGGlVlj75
21+
iqpOua1RbHXlKYf/Zuur24ECgYEAqXDFSWmGKsOY2XR9QwQltUxYHat2dQxxykfR
22+
GCmUOhcYqT0VuqSyL/oBK25AXBN465b1gxG3xWsdpcf+FLB7OdD0i1XnTUYYRPeq
23+
1SKUQRdOY/11G3Ntcn5ZjkHL41NvDRbiQfz42noqQj2/94T+rybVyKAZeeZd42Es
24+
J0082OUCgYEAnguGJxhfSryD3a2kAQ/6s+L303rgXkRt+/luoopdm7vu2AcnaP7L
25+
aK4dCDusp/DZyGn8/ebDCGNIaVEMJVHAPLFbhJA9E9HCjfC33RdklCO/aGDtXsiq
26+
kzg3mqPHTCPCpmpr5YAtuLONczP1qgB04/vqb2S5eANC+5k6mEifu2U=
27+
-----END RSA PRIVATE KEY-----
28+
-----BEGIN CERTIFICATE-----
29+
MIIDyzCCArOgAwIBAgIJAOG0pVw936YUMA0GCSqGSIb3DQEBCwUAMGMxCzAJBgNV
30+
BAYTAlNFMRIwEAYDVQQIDAlTdG9ja2hvbG0xEjAQBgNVBAcMCVN0b2NraG9sbTEP
31+
MA0GA1UECgwGT3JhY2xlMQ4wDAYDVQQLDAVNeVNRTDELMAkGA1UEAwwCQ0EwHhcN
32+
MTQxMjA1MDQ0ODQwWhcNMjkxMjAxMDQ0ODQwWjBqMQswCQYDVQQGEwJTRTESMBAG
33+
A1UECAwJU3RvY2tob2xtMRIwEAYDVQQHDAlTdG9ja2hvbG0xDzANBgNVBAoMBk9y
34+
YWNsZTEOMAwGA1UECwwFTXlTUUwxEjAQBgNVBAMMCWxvY2FsaG9zdDCCASIwDQYJ
35+
KoZIhvcNAQEBBQADggEPADCCAQoCggEBAOkg5wVRBzpIueJioiRFH/FUNCJcYoab
36+
AejCRZAAjU7kourp17WVJc4YrBpPs+YwRq8gE0CE3yHc3wnloH+BEm4ehFhcoRHb
37+
qrYE4PweDBEu8zBiGvnu3/6j09aDbq3ojZiJsWljuHL0WuYiXnNkle9WkpIL4ZOl
38+
10xBR+cx7Qlos8VuwRsBObvxj7+69ALi5eWet9aaspR2m0jYJxgOnzCY9Zo+I+Y7
39+
SkjupSbogJQ35Eq6/59CuTLdep1jEaglmbEfhuV8sTHjEhEM6/YdAksdNMt0HX8v
40+
QMCB4gTUjuqW+CI1jqu2mTM277WDEYhtBg92Tb/bqN9sPJHiunOgc0UCAwEAAaN7
41+
MHkwCQYDVR0TBAIwADAsBglghkgBhvhCAQ0EHxYdT3BlblNTTCBHZW5lcmF0ZWQg
42+
Q2VydGlmaWNhdGUwHQYDVR0OBBYEFOQx2NUG6sazovYBOY9YCDYsez/bMB8GA1Ud
43+
IwQYMBaAFJRloaOHz7/BdLvYhJe2a+6ykHOyMA0GCSqGSIb3DQEBCwUAA4IBAQBG
44+
782/we82qcuZtb7ip7ppDfWbYzl4MjUBqLnxcA610Y+ULnrLZdTQtK1SuFFfZC6o
45+
CKVx/sI1ig0oJuW+ytf0eVThJ4+HktMEchvMxH+LJgmSLchvJ9qXMYAPg4Sc4KSI
46+
yeOPNefevTHi+lKD3u5cYG6PpY1eU0EYQvMDCwULWQlCLhsLKbP+ETvVrT9NJOjA
47+
2kwNk5TszRgPZs0D0+6gsn9k1zlmGXKfZEM4tLaz6m855wmYkJ9s9eizLgmNe3Zl
48+
MPTEm44QTpuMk2NEnSuK8/DP+HtllTj7tJLobBEDoqIv6uAit83PDaWRIxRHDDSP
49+
8RHM6B43U+yhAYEgwCz1
50+
-----END CERTIFICATE-----

mysql-test/t/openssl_1.test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ drop table t1;
121121
# Test that we can't open connection to server if we are using
122122
# a blank client-key
123123
#
124+
--replace_regex /Unable to get private key from '.*'/Unable to get private key from ''/
124125
--error 1
125126
--exec $MYSQL_TEST --ssl-mode=REQUIRED --ssl-key= --max-connect-retries=1 < $MYSQLTEST_VARDIR/tmp/test.sql 2>&1
126127

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--ssl=1
22
--ssl-ca=$MYSQL_TEST_DIR/std_data/cacert.pem
3-
--ssl-cert=$MYSQL_TEST_DIR/std_data/server-cert.pem
4-
--ssl-key=$MYSQL_TEST_DIR/std_data/server-key.pem
3+
--ssl-cert=$MYSQL_TEST_DIR/std_data/server-combined.pem
4+
--ssl-key=$MYSQL_TEST_DIR/std_data/server-combined.pem
55
--max_allowed_packet=64M

vio/viosslfactories.cc

Lines changed: 99 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
#include "m_ctype.h"
2929
#include "my_dbug.h"
30+
#include "my_dir.h"
3031
#include "my_inttypes.h"
3132
#include "my_loglevel.h"
3233
#if !defined(HAVE_WOLFSSL) && !defined(HAVE_PSI_INTERFACE)
@@ -189,48 +190,117 @@ const char *sslGetErrString(enum enum_ssl_init_error e) {
189190
return ssl_error_string[e];
190191
}
191192

193+
static bool check_same_file(const char *cert_file, const char *key_file) {
194+
MY_STAT cert_stat;
195+
MY_STAT key_stat;
196+
197+
if (!my_stat(cert_file, &cert_stat, MYF(0)) ||
198+
!my_stat(key_file, &key_stat, MYF(0))) {
199+
DBUG_PRINT("error", ("Unable to stat the files."));
200+
return false;
201+
}
202+
203+
return cert_stat.st_ino == key_stat.st_ino &&
204+
cert_stat.st_dev == key_stat.st_dev;
205+
}
206+
207+
static int vio_load_cert(SSL_CTX *ctx, const char *cert_file, BIO *bio) {
208+
X509 *cert = NULL;
209+
210+
if (BIO_read_filename(bio, cert_file) != 1 ||
211+
!PEM_read_bio_X509(bio, &cert, NULL, NULL)) {
212+
return 1;
213+
}
214+
215+
int ret = SSL_CTX_use_certificate(ctx, cert);
216+
X509_free(cert);
217+
218+
return ret <= 0;
219+
}
220+
221+
static int vio_load_key(SSL_CTX *ctx, const char *key_file, bool same_file,
222+
BIO *bio) {
223+
// read key file if not in same file as cert
224+
if (!same_file) {
225+
if (BIO_read_filename(bio, key_file) != 1) {
226+
DBUG_PRINT("error", ("BIO_WRITE for key %s failed", key_file));
227+
return 1;
228+
}
229+
} else {
230+
if (BIO_reset(bio) < 0) {
231+
DBUG_PRINT("error", ("BIO_reset failed ret"));
232+
return 1;
233+
}
234+
}
235+
236+
EVP_PKEY *key = NULL;
237+
if (!PEM_read_bio_PrivateKey(bio, &key, NULL, NULL)) return 1;
238+
239+
int ret = SSL_CTX_use_PrivateKey(ctx, key);
240+
EVP_PKEY_free(key);
241+
242+
return ret <= 0;
243+
}
244+
192245
static int vio_set_cert_stuff(SSL_CTX *ctx, const char *cert_file,
193246
const char *key_file,
194247
enum enum_ssl_init_error *error) {
195248
DBUG_ENTER("vio_set_cert_stuff");
196249
DBUG_PRINT("enter", ("ctx: %p cert_file: %s key_file: %s", ctx, cert_file,
197250
key_file));
198251

199-
if (!cert_file && key_file) cert_file = key_file;
252+
*error = SSL_INITERR_NOERROR;
253+
bool same_file = false;
200254

201-
if (!key_file && cert_file) key_file = cert_file;
255+
if (cert_file == NULL) {
256+
if (key_file == NULL) DBUG_RETURN(0);
202257

203-
if (cert_file &&
204-
SSL_CTX_use_certificate_file(ctx, cert_file, SSL_FILETYPE_PEM) <= 0) {
205-
*error = SSL_INITERR_CERT;
206-
DBUG_PRINT("error",
207-
("%s from file '%s'", sslGetErrString(*error), cert_file));
208-
DBUG_EXECUTE("error", ERR_print_errors_fp(DBUG_FILE););
209-
my_message_local(ERROR_LEVEL, EE_SSL_ERROR_FROM_FILE,
210-
sslGetErrString(*error), cert_file);
211-
DBUG_RETURN(1);
212-
}
258+
cert_file = key_file;
259+
same_file = true;
260+
} else if (key_file == NULL) {
261+
key_file = cert_file;
262+
same_file = true;
263+
} else
264+
same_file = check_same_file(key_file, cert_file);
213265

214-
if (key_file &&
215-
SSL_CTX_use_PrivateKey_file(ctx, key_file, SSL_FILETYPE_PEM) <= 0) {
216-
*error = SSL_INITERR_KEY;
217-
DBUG_PRINT("error",
218-
("%s from file '%s'", sslGetErrString(*error), key_file));
219-
DBUG_EXECUTE("error", ERR_print_errors_fp(DBUG_FILE););
220-
my_message_local(ERROR_LEVEL, EE_SSL_ERROR_FROM_FILE,
221-
sslGetErrString(*error), key_file);
222-
DBUG_RETURN(1);
266+
const char *error_file = NULL;
267+
BIO *bio = BIO_new(BIO_s_file());
268+
if (bio == NULL)
269+
*error = SSL_INITERR_MEMFAIL;
270+
else {
271+
// load the cert
272+
if (vio_load_cert(ctx, cert_file, bio)) {
273+
*error = SSL_INITERR_CERT;
274+
error_file = cert_file;
275+
}
276+
// load the key
277+
else if (vio_load_key(ctx, key_file, same_file, bio)) {
278+
*error = SSL_INITERR_KEY;
279+
error_file = key_file;
280+
}
281+
282+
/*
283+
If we are using DSA, we can copy the parameters from the private key
284+
Now we know that a key and cert have been set against the SSL context
285+
*/
286+
else if (!SSL_CTX_check_private_key(ctx)) {
287+
*error = SSL_INITERR_NOMATCH;
288+
}
289+
290+
BIO_free_all(bio);
223291
}
224292

225-
/*
226-
If we are using DSA, we can copy the parameters from the private key
227-
Now we know that a key and cert have been set against the SSL context
228-
*/
229-
if (cert_file && !SSL_CTX_check_private_key(ctx)) {
230-
*error = SSL_INITERR_NOMATCH;
231-
DBUG_PRINT("error", ("%s", sslGetErrString(*error)));
293+
if (*error != SSL_INITERR_NOERROR) {
294+
char err_string[MYSQL_ERRMSG_SIZE];
295+
if (error_file)
296+
snprintf(err_string, sizeof(err_string), "%s from '%s'",
297+
sslGetErrString(*error), error_file);
298+
else
299+
snprintf(err_string, sizeof(err_string), "%s", sslGetErrString(*error));
300+
DBUG_PRINT("error", ("%s", err_string));
232301
DBUG_EXECUTE("error", ERR_print_errors_fp(DBUG_FILE););
233-
my_message_local(ERROR_LEVEL, EE_SSL_ERROR, sslGetErrString(*error));
302+
my_message_local(ERROR_LEVEL, EE_SSL_ERROR_FROM_FILE,
303+
sslGetErrString(*error), cert_file);
234304
DBUG_RETURN(1);
235305
}
236306

0 commit comments

Comments
 (0)