Merge pull request #238 from socketpair/chain

SSTP: load certificate chain instead of single one

2 files changed, 59 insertions, 36 deletions

diff --git a/accel-pppd/accel-ppp.conf.5 b/accel-pppd/accel-ppp.conf.5
index 8860d520..01e7ac07 100644
--- a/accel-pppd/accel-ppp.conf.5
+++ b/accel-pppd/accel-ppp.conf.5
@@ -842,14 +842,18 @@ is greater of zero then server ciphers should be preferred over client ciphers.
 Default is 0.
 .TP
 .BI "ssl-pemfile=" pemfile
-Specifies a file with the certificate in the PEM format for sstp server.
-Certificate is also used to compute initial SHA1 and SHA256 certificate hash.
+Specifies a file with the chain of certificates in the PEM format for sstp server.
+The certificates must be in PEM format and must be sorted starting with the
+subject's certificate (actual server certificate), followed by intermediate CA
+certificates if applicable, and ending at the highest level (root) CA.
+Leaf (the first) certificate is also used to compute initial SHA1 and SHA256
+certificate hash.
 .TP
 .BI "ssl-keyfile=" keyfile
 Specifies a file with the secret key in the PEM format for sstp server.
 If not set, secret key will be loaded from the
 .BI pemfile
-certificate.
+parameter.
 .TP
 .BI "cert-hash-proto=" sha1,sha256
 Specifies hashing methods that can be used to compute the Compound MAC in the Crypto Binding attribute.
diff --git a/accel-pppd/ctrl/sstp/sstp.c b/accel-pppd/ctrl/sstp/sstp.c
index 2a9ddae3..d63caa32 100644
--- a/accel-pppd/ctrl/sstp/sstp.c
+++ b/accel-pppd/ctrl/sstp/sstp.c
@@ -2488,6 +2488,13 @@ static void ssl_info_cb(const SSL *ssl, int where, int ret)
 #endif
 #endif
 
+static void ssl_set_cert_hashes(const X509 *cert) {
+	if (conf_hash_protocol & CERT_HASH_PROTOCOL_SHA1)
+		X509_digest(cert, EVP_sha1(), conf_hash_sha1.hash, &conf_hash_sha1.len);
+	if (conf_hash_protocol & CERT_HASH_PROTOCOL_SHA256)
+		X509_digest(cert, EVP_sha256(), conf_hash_sha256.hash, &conf_hash_sha256.len);
+}
+
 static void ssl_load_config(struct sstp_serv_t *serv, const char *servername)
 {
 	SSL_CTX *old_ctx, *ssl_ctx = NULL;
@@ -2495,26 +2502,6 @@ static void ssl_load_config(struct sstp_serv_t *serv, const char *servername)
 	BIO *in = NULL;
 	char *opt;
 
-	opt = conf_get_opt("sstp", "ssl-pemfile");
-	if (opt) {
-		in = BIO_new(BIO_s_file());
-		if (!in) {
-			log_error("sstp: %s error: %s\n", "ssl-pemfile", ERR_error_string(ERR_get_error(), NULL));
-			goto error;
-		}
-
-		if (BIO_read_filename(in, opt) <= 0) {
-			log_error("sstp: %s error: %s\n", "ssl-pemfile", ERR_error_string(ERR_get_error(), NULL));
-			goto error;
-		}
-
-		cert = PEM_read_bio_X509(in, NULL, NULL, NULL);
-		if (!cert) {
-			log_error("sstp: %s error: %s\n", "ssl-pemfile", ERR_error_string(ERR_get_error(), NULL));
-			goto error;
-		}
-	}
-
 	opt = conf_get_opt("sstp", "accept");
 	if (opt && strhas(opt, "ssl", ',')) {
 	legacy_ssl:
@@ -2607,6 +2594,8 @@ static void ssl_load_config(struct sstp_serv_t *serv, const char *servername)
 #else
 			DH *dh;
 
+			in = BIO_new(BIO_s_file());
+
 			if (BIO_read_filename(in, opt) <= 0) {
 				log_error("sstp: %s error: %s\n", "ssl-dhparam", ERR_error_string(ERR_get_error(), NULL));
 				goto error;
@@ -2618,6 +2607,10 @@ static void ssl_load_config(struct sstp_serv_t *serv, const char *servername)
 				goto error;
 			}
 
+			if (!BIO_free(in))
+				abort();
+			in = NULL;
+
 			SSL_CTX_set_tmp_dh(ssl_ctx, dh);
 			DH_free(dh);
 #endif
@@ -2670,12 +2663,21 @@ static void ssl_load_config(struct sstp_serv_t *serv, const char *servername)
 		if (opt && atoi(opt))
 			SSL_CTX_set_options(ssl_ctx, SSL_OP_CIPHER_SERVER_PREFERENCE);
 
-		if (cert && SSL_CTX_use_certificate(ssl_ctx, cert) != 1) {
-			log_error("sstp: %s error: %s\n", "ssl-pemfile", ERR_error_string(ERR_get_error(), NULL));
-			goto error;
+		opt = conf_get_opt("sstp", "ssl-pemfile");
+		if (opt) {
+			if (SSL_CTX_use_certificate_chain_file(ssl_ctx, opt) != 1) {
+				log_error("sstp: %s error: %s\n", "ssl-pemfile", ERR_error_string(ERR_get_error(), NULL));
+				goto error;
+			}
+			// cert is a reference. Do not free it.
+			X509 *cert_ref = SSL_CTX_get0_certificate(ssl_ctx);
+			if (!cert_ref) {
+				log_error("sstp: %s error: %s\n", "ssl-pemfile", ERR_error_string(ERR_get_error(), NULL));
+				goto error;
+			}
+			ssl_set_cert_hashes(cert_ref);
 		}
-
-		opt = conf_get_opt("sstp", "ssl-keyfile") ? : conf_get_opt("sstp", "ssl-pemfile");
+		opt = conf_get_opt("sstp", "ssl-keyfile") ? : opt;
 		if ((opt && SSL_CTX_use_PrivateKey_file(ssl_ctx, opt, SSL_FILETYPE_PEM) != 1) ||
 		    SSL_CTX_check_private_key(ssl_ctx) != 1) {
 			log_error("sstp: %s error: %s\n", "ssl-keyfile", ERR_error_string(ERR_get_error(), NULL));
@@ -2703,13 +2705,30 @@ static void ssl_load_config(struct sstp_serv_t *serv, const char *servername)
 		opt = conf_get_opt("sstp", "ssl");
 		if (opt && atoi(opt) > 0)
 			goto legacy_ssl;
-	}
 
-	if (cert) {
-		if (conf_hash_protocol & CERT_HASH_PROTOCOL_SHA1)
-			X509_digest(cert, EVP_sha1(), conf_hash_sha1.hash, &conf_hash_sha1.len);
-		if (conf_hash_protocol & CERT_HASH_PROTOCOL_SHA256)
-			X509_digest(cert, EVP_sha256(), conf_hash_sha256.hash, &conf_hash_sha256.len);
+		opt = conf_get_opt("sstp", "ssl-pemfile");
+		if (opt) {
+			in = BIO_new(BIO_s_file());
+			if (!in) {
+				log_error("sstp: %s error: %s\n", "ssl-pemfile", ERR_error_string(ERR_get_error(), NULL));
+				goto error;
+			}
+
+			if (BIO_read_filename(in, opt) <= 0) {
+				log_error("sstp: %s error: %s\n", "ssl-pemfile", ERR_error_string(ERR_get_error(), NULL));
+				goto error;
+			}
+
+			cert = PEM_read_bio_X509(in, NULL, NULL, NULL);
+			if (!cert) {
+				log_error("sstp: %s error: %s\n", "ssl-pemfile", ERR_error_string(ERR_get_error(), NULL));
+				goto error;
+			}
+			if (!BIO_free(in))
+				abort();
+			in = NULL;
+			ssl_set_cert_hashes(cert);
+		}
 	}
 
 	old_ctx = serv->ssl_ctx;
@@ -2721,8 +2740,8 @@ error:
 		SSL_CTX_free(ssl_ctx);
 	if (cert)
 		X509_free(cert);
-	if (in)
-		BIO_free(in);
+	if (in && !BIO_free(in))
+		abort();
 }
 #endif