You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@httpd.apache.org by GitBox <gi...@apache.org> on 2021/04/20 09:56:35 UTC
[GitHub] [httpd] rpluem commented on a change in pull request #179: core ap_ssl_() support, related changes, latest mod_md
rpluem commented on a change in pull request #179:
URL: https://github.com/apache/httpd/pull/179#discussion_r616479976
##########
File path: server/ssl.c
##########
@@ -0,0 +1,192 @@
+/* Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+ * ssl.c --- routines for SSL/TLS server infrastructure.
+ *
+ */
+
+#include "apr.h"
+#include "apr_strings.h"
+#include "apr_buckets.h"
+#include "apr_lib.h"
+#include "apr_signal.h"
+#include "apr_strmatch.h"
+
+#define APR_WANT_STDIO /* for sscanf */
+#define APR_WANT_STRFUNC
+#define APR_WANT_MEMFUNC
+#include "apr_want.h"
+
+#include "util_filter.h"
+#include "ap_config.h"
+#include "httpd.h"
+#include "http_config.h"
+#include "http_core.h"
+#include "http_protocol.h"
+#include "http_request.h"
+#include "http_main.h"
+#include "http_ssl.h"
+#include "http_log.h" /* For errors detected in basic auth common
+ * support code... */
+#include "mod_core.h"
+
+
+#if APR_HAVE_STDARG_H
+#include <stdarg.h>
+#endif
+#if APR_HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
+/* we know core's module_index is 0 */
+#undef APLOG_MODULE_INDEX
+#define APLOG_MODULE_INDEX AP_CORE_MODULE_INDEX
+
+APR_HOOK_STRUCT(
+ APR_HOOK_LINK(ssl_conn_is_ssl)
+ APR_HOOK_LINK(ssl_var_lookup)
+ APR_HOOK_LINK(ssl_add_cert_files)
+ APR_HOOK_LINK(ssl_add_fallback_cert_files)
+ APR_HOOK_LINK(ssl_answer_challenge)
+ APR_HOOK_LINK(ssl_ocsp_prime_hook)
+ APR_HOOK_LINK(ssl_ocsp_get_resp_hook)
+)
+
+APR_DECLARE_OPTIONAL_FN(int, ssl_is_https, (conn_rec *));
+static APR_OPTIONAL_FN_TYPE(ssl_is_https) *module_ssl_is_https;
+
+static int ssl_is_https(conn_rec *c)
+{
+ /* Someone retrieved the optional function., not knowning about the
+ * new API. We redirect them to what they should have inoked. */
+ return ap_ssl_conn_is_ssl(c);
+}
+
+AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
+{
+ int r = (ap_run_ssl_conn_is_ssl(c) == OK);
+ if (r == 0 && module_ssl_is_https) {
+ r = module_ssl_is_https(c);
+ }
+ return r;
+}
+
+APR_DECLARE_OPTIONAL_FN(const char *, ssl_var_lookup,
+ (apr_pool_t *p, server_rec *s,
+ conn_rec *c, request_rec *r,
+ const char *name));
+static APR_OPTIONAL_FN_TYPE(ssl_var_lookup) *module_ssl_var_lookup;
+
+static const char *ssl_var_lookup(apr_pool_t *p, server_rec *s,
+ conn_rec *c, request_rec *r,
+ const char *name)
+{
+ /* Someone retrieved the optional function., not knowning about the
+ * new API. We redirect them to what they should have inoked. */
+ return ap_ssl_var_lookup(p, s, c, r, name);
+}
+
+AP_DECLARE(const char *) ap_ssl_var_lookup(apr_pool_t *p, server_rec *s,
+ conn_rec *c, request_rec *r,
+ const char *name)
+{
+ const char *val = ap_run_ssl_var_lookup(p, s, c, r, name);
+ if (val == NULL && module_ssl_is_https) {
Review comment:
I guess should be `module_ssl_var_lookup` instead of `module_ssl_is_https`.
##########
File path: modules/lua/mod_lua.c
##########
@@ -1688,15 +1685,12 @@ static const char *register_lua_root(cmd_parms *cmd, void *_cfg,
const char *ap_lua_ssl_val(apr_pool_t *p, server_rec *s, conn_rec *c,
request_rec *r, const char *var)
{
- if (lua_ssl_val) {
- return (const char *)lua_ssl_val(p, s, c, r, (char *)var);
- }
- return NULL;
+ return ap_ssl_var_lookup(p, s, c, r, (char *)var);
Review comment:
`ap_ssl_var_lookup` expects `const char *` as last parameter, `var` is `const char *`. What is the purpose of converting to `char *`?
##########
File path: include/ap_mmn.h
##########
@@ -554,14 +554,20 @@
* AP_REQUEST_STRONG_ETAG, AP_REQUEST_GET_BNOTE,
* AP_REQUEST_SET_BNOTE and AP_REQUEST_IS_STRONG_ETAG
* in httpd.h.
+ * 20120211.102 (2.4.47-dev) Add ap_ssl_conn_is_ssl()/ap_ssl_var_lookup() and hooks
+ * 20120211.103 (2.4.47-dev) Add ap_ssl_add_cert_files, ap_ssl_add_fallback_cert_files
+ * and ap_ssl_answer_challenge and hooks.
+ * 20120211.104 (2.4.47-dev) Move ap_ssl_* into new http_ssl.h header file
+ * 20120211.105 (2.4.47-dev) Add `ap_bytes_t` to httpd.h.
+ * Add ap_ssl_ocsp* hooks and functions to http_ssl.h.
*/
Review comment:
Nitpick: Whitespace formating: Before `Add`one space less, before `and` and the last `Add` one space more.
##########
File path: modules/http2/h2_h2.c
##########
@@ -501,7 +475,7 @@ int h2_is_acceptable_connection(conn_rec *c, request_rec *r, int require_all)
/* Check TLS cipher blacklist
*/
- val = opt_ssl_var_lookup(pool, s, c, NULL, (char*)"SSL_CIPHER");
+ val = ap_ssl_var_lookup(pool, s, c, NULL, (char*)"SSL_CIPHER");
Review comment:
`ap_ssl_var_lookup` expects `const char *` as last parameter, `var` is `const char *`. What is the purpose of converting to `char *`?
##########
File path: modules/ssl/ssl_engine_kernel.c
##########
@@ -2316,32 +2316,53 @@ void ssl_callback_Info(const SSL *ssl, int where, int rc)
#ifdef HAVE_TLSEXT
static apr_status_t set_challenge_creds(conn_rec *c, const char *servername,
- SSL *ssl, X509 *cert, EVP_PKEY *key)
+ SSL *ssl, X509 *cert, EVP_PKEY *key,
+ const char *cert_pem, const char *key_pem)
{
SSLConnRec *sslcon = myConnConfig(c);
+ apr_status_t rv = APR_SUCCESS;
+ int our_data = 0;
sslcon->service_unavailable = 1;
+ if (cert_pem) {
+ cert = NULL;
+ key = NULL;
+ our_data = 1;
+
+ rv = modssl_read_cert(c->pool, cert_pem, key_pem, NULL, NULL, &cert, &key);
+ if (rv != APR_SUCCESS) {
+ ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c, APLOGNO(10266)
+ "Failed to parse PEM of challenge certificate %s",
+ servername);
+ goto cleanup;
+ }
+ }
+
if ((SSL_use_certificate(ssl, cert) < 1)) {
ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c, APLOGNO(10086)
"Failed to configure challenge certificate %s",
servername);
- return APR_EGENERAL;
+ rv = APR_EGENERAL; goto cleanup;
}
if (!SSL_use_PrivateKey(ssl, key)) {
ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c, APLOGNO(10087)
"error '%s' using Challenge key: %s",
ERR_error_string(ERR_peek_last_error(), NULL),
servername);
- return APR_EGENERAL;
+ rv = APR_EGENERAL; goto cleanup;
Review comment:
Nitpick: Style: Can we have two lines here?
##########
File path: modules/proxy/mod_proxy.c
##########
@@ -2866,23 +2860,14 @@ PROXY_DECLARE(int) ap_proxy_ssl_engine(conn_rec *c,
PROXY_DECLARE(int) ap_proxy_conn_is_https(conn_rec *c)
{
- if (proxy_is_https) {
- return proxy_is_https(c);
- }
- else
- return 0;
+ return ap_ssl_conn_is_ssl(c);
}
PROXY_DECLARE(const char *) ap_proxy_ssl_val(apr_pool_t *p, server_rec *s,
conn_rec *c, request_rec *r,
const char *var)
{
- if (proxy_ssl_val) {
- /* XXX Perhaps the casting useless */
- return (const char *)proxy_ssl_val(p, s, c, r, (char *)var);
- }
- else
- return NULL;
+ return ap_ssl_var_lookup(p, s, c, r, (char *)var);
Review comment:
`ap_ssl_var_lookup` expects `const char *` as last parameter, `var` is `const char *`. What is the purpose of converting to `char *`?
##########
File path: modules/ssl/ssl_engine_kernel.c
##########
@@ -2316,32 +2316,53 @@ void ssl_callback_Info(const SSL *ssl, int where, int rc)
#ifdef HAVE_TLSEXT
static apr_status_t set_challenge_creds(conn_rec *c, const char *servername,
- SSL *ssl, X509 *cert, EVP_PKEY *key)
+ SSL *ssl, X509 *cert, EVP_PKEY *key,
+ const char *cert_pem, const char *key_pem)
{
SSLConnRec *sslcon = myConnConfig(c);
+ apr_status_t rv = APR_SUCCESS;
+ int our_data = 0;
sslcon->service_unavailable = 1;
+ if (cert_pem) {
+ cert = NULL;
+ key = NULL;
+ our_data = 1;
+
+ rv = modssl_read_cert(c->pool, cert_pem, key_pem, NULL, NULL, &cert, &key);
+ if (rv != APR_SUCCESS) {
+ ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c, APLOGNO(10266)
+ "Failed to parse PEM of challenge certificate %s",
+ servername);
+ goto cleanup;
+ }
+ }
+
if ((SSL_use_certificate(ssl, cert) < 1)) {
ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c, APLOGNO(10086)
"Failed to configure challenge certificate %s",
servername);
- return APR_EGENERAL;
+ rv = APR_EGENERAL; goto cleanup;
Review comment:
Nitpick: Style: Can we have two lines here?
##########
File path: modules/http2/h2_h2.c
##########
@@ -445,44 +437,26 @@ apr_status_t h2_h2_init(apr_pool_t *pool, server_rec *s)
{
(void)pool;
ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, s, "h2_h2, child_init");
- opt_ssl_is_https = APR_RETRIEVE_OPTIONAL_FN(ssl_is_https);
- opt_ssl_var_lookup = APR_RETRIEVE_OPTIONAL_FN(ssl_var_lookup);
-
- if (!opt_ssl_is_https || !opt_ssl_var_lookup) {
- ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s,
- APLOGNO(02951) "mod_ssl does not seem to be enabled");
- }
-
cipher_init(pool);
return APR_SUCCESS;
}
-int h2_h2_is_tls(conn_rec *c)
-{
- return opt_ssl_is_https && opt_ssl_is_https(c);
-}
-
int h2_is_acceptable_connection(conn_rec *c, request_rec *r, int require_all)
{
- int is_tls = h2_h2_is_tls(c);
+ int is_tls = ap_ssl_conn_is_ssl(c);
if (is_tls && h2_config_cgeti(c, H2_CONF_MODERN_TLS_ONLY) > 0) {
/* Check TLS connection for modern TLS parameters, as defined in
* RFC 7540 and https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility
*/
apr_pool_t *pool = c->pool;
server_rec *s = c->base_server;
- char *val;
+ const char *val;
- if (!opt_ssl_var_lookup) {
- /* unable to check */
- return 0;
- }
-
/* Need Tlsv1.2 or higher, rfc 7540, ch. 9.2
*/
- val = opt_ssl_var_lookup(pool, s, c, NULL, (char*)"SSL_PROTOCOL");
+ val = ap_ssl_var_lookup(pool, s, c, NULL, (char*)"SSL_PROTOCOL");
if (val && *val) {
Review comment:
`ap_ssl_var_lookup` expects `const char *` as last parameter, `var` is `const char *`. What is the purpose of converting to `char *`?
##########
File path: modules/ssl/ssl_engine_kernel.c
##########
@@ -2316,32 +2316,53 @@ void ssl_callback_Info(const SSL *ssl, int where, int rc)
#ifdef HAVE_TLSEXT
static apr_status_t set_challenge_creds(conn_rec *c, const char *servername,
- SSL *ssl, X509 *cert, EVP_PKEY *key)
+ SSL *ssl, X509 *cert, EVP_PKEY *key,
+ const char *cert_pem, const char *key_pem)
{
SSLConnRec *sslcon = myConnConfig(c);
+ apr_status_t rv = APR_SUCCESS;
+ int our_data = 0;
sslcon->service_unavailable = 1;
+ if (cert_pem) {
+ cert = NULL;
+ key = NULL;
+ our_data = 1;
+
+ rv = modssl_read_cert(c->pool, cert_pem, key_pem, NULL, NULL, &cert, &key);
+ if (rv != APR_SUCCESS) {
+ ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c, APLOGNO(10266)
+ "Failed to parse PEM of challenge certificate %s",
+ servername);
+ goto cleanup;
+ }
+ }
+
if ((SSL_use_certificate(ssl, cert) < 1)) {
ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c, APLOGNO(10086)
"Failed to configure challenge certificate %s",
servername);
- return APR_EGENERAL;
+ rv = APR_EGENERAL; goto cleanup;
}
if (!SSL_use_PrivateKey(ssl, key)) {
ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c, APLOGNO(10087)
"error '%s' using Challenge key: %s",
ERR_error_string(ERR_peek_last_error(), NULL),
servername);
- return APR_EGENERAL;
+ rv = APR_EGENERAL; goto cleanup;
}
if (SSL_check_private_key(ssl) < 1) {
ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, c, APLOGNO(10088)
"Challenge certificate and private key %s "
"do not match", servername);
- return APR_EGENERAL;
+ rv = APR_EGENERAL; goto cleanup;
Review comment:
Nitpick: Style: Can we have two lines here?
##########
File path: modules/ssl/ssl_engine_kernel.c
##########
@@ -2376,15 +2394,6 @@ static apr_status_t init_vhost(conn_rec *c, SSL *ssl, const char *servername)
sslcon->vhost_found = +1;
return APR_SUCCESS;
}
- else if (ssl_is_challenge(c, servername, &cert, &key)) {
- /* With ACMEv1 we can have challenge connections to a unknown domains
- * that need to be answered with a special certificate and will
- * otherwise not answer any requests. */
- if (set_challenge_creds(c, servername, ssl, cert, key) != APR_SUCCESS) {
- return APR_EGENERAL;
- }
- SSL_set_verify(ssl, SSL_VERIFY_NONE, ssl_callback_SSLVerify);
- }
Review comment:
Why is this no longer needed?
##########
File path: CHANGES
##########
@@ -45,6 +137,37 @@ Changes with Apache 2.4.47
*) mod_session: Improve session parsing. [Yann Yalvic]
+ *) core: Adding SSL related inquiry functions to the server API.
+ These function are always available, even when no module providing
+ SSL is loaded. They provide their own "shadowing" implementation for
+ the optional functions of similar name that mod_ssl and impersonators
+ of mod_ssl provide.
+ This enables loading of several SSL providing modules when all but
+ one of them registers itself into the new hooks. Two old-style SSL
+ modules will not work, as they replace the others optional functions
+ with their own.
+ Modules using the old-style optional functions will continue to work
+ as core supplies its own versions of those.
+ The following has been added so far:
+ - ap_ssl_conn_is_ssl() to query if a connection is using SSL.
+ - ap_ssl_var_lookup() to query SSL related variables for a
+ server/connection/request.
+ - Hooks for 'ssl_conn_is_ssl' and 'ssl_var_lookup' where modules
+ providing SSL can install their own value supplying functions.
+ - ap_ssl_add_cert_files() to enable other modules like mod_md to provide
+ certificate and keys for an SSL module like mod_ssl.
+ - ap_ssl_add_fallback_cert_files() to enable other modules like mod_md to
+ provide a fallback certificate in case no 'proper' certificate is
+ available for an SSL module like mod_ssl.
+ - ap_ssl_answer_challenge() to enable other modules like mod_md to
+ provide a certificate as used in the RFC 8555 'tls-alpn-01' challenge
+ for the ACME protocol for an SSL module like mod_ssl. The function
+ and its hook provide PEM encoded data instead of file names.
+ - Hooks for 'ssl_add_cert_files', 'ssl_add_fallback_cert_files' and
+ 'ssl_answer_challenge' where modules like mod_md can provide providers
+ to the above mentioned functions.
+ [Stefan Eissing]
+
Review comment:
These changes are added twice.
##########
File path: modules/ssl/ssl_util_ssl.c
##########
@@ -511,3 +511,77 @@ char *modssl_SSL_SESSION_id2sz(IDCONST unsigned char *id, int idlen,
return str;
}
+
+/* _________________________________________________________________
+**
+** Certficate/Key Stuff
+** _________________________________________________________________
+*/
+
+apr_status_t modssl_read_cert(apr_pool_t *p,
+ const char *cert_pem, const char *key_pem,
+ pem_password_cb *cb, void *ud,
+ X509 **pcert, EVP_PKEY **pkey)
+{
+ BIO *in;
+ X509 *x = NULL;
+ EVP_PKEY *key = NULL;
+ apr_status_t rv = APR_SUCCESS;
+
+ in = BIO_new_mem_buf(cert_pem, -1);
+ if (in == NULL) {
+ rv = APR_ENOMEM; goto cleanup;
Review comment:
Nitpick: Style: Can we have two lines here? Here in the remainder of the function?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org