Skip to content

Commit

Permalink
ISSUE-1722: expand sslProfile mgmt API for ordinal value updates
Browse files Browse the repository at this point in the history
  • Loading branch information
kgiusti committed Jan 28, 2025
1 parent d6cf8f2 commit 42cbf56
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 36 deletions.
21 changes: 15 additions & 6 deletions include/qpid/dispatch/tls_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,29 +84,38 @@ struct qd_ssl2_profile_t {
char *uid_name_mapping_file;

/**
* version: Version assigned to the current configuration
* oldest_valid_version: Previous sslProfile updates with versions values < oldest_valid_version have expired.
* Certificate Rotation
* ordinal: Identifies the current configuration's revision
* oldest_valid_ordinal: Previous configurations with ordinal values < oldest_valid_ordinal have expired.
*/
long version;
long oldest_valid_version;
uint64_t ordinal;
uint64_t oldest_valid_ordinal;
};

/**
* Create a new TLS qd_tls_config_t instance with the given configuration
* Create a new TLS qd_tls_config_t instance with the given configuration. This is called when a listener/connector
* record is created.
*
* @param ssl_profile_name the name of the sslProfile configuration to use
* @param p_type protocol type for the child connections (TCP or AMQP)
* @param mode the operational use case (TLS Server or Client)
* @param verify_hostname enforce host name checking (Client mode)
* @param authenticate_peer validate peer's certificate (Server mode)
* @param update_cb_context Opaque handle passed to ordinal update callback.
* @param callback Optional callback invoked when ordinal or last_valid_ordinal
* is modified by management.
*
* @return a new qd_tls_config_t instance or 0 on error. qd_error() set if error.
*/

typedef void (*qd_tls_ordinal_update_cb_t)(uint64_t ordinal, uint64_t oldest_valid_ordinal, void *context);
qd_tls_config_t *qd_tls_config(const char *ssl_profile_name,
qd_tls_type_t p_type,
qd_tls_config_mode_t mode,
bool verify_hostname,
bool authenticate_peer);
bool authenticate_peer,
void *update_cb_context,
qd_tls_ordinal_update_cb_t callback);


/**
Expand Down
8 changes: 4 additions & 4 deletions python/skupper_router/management/skrouter.json
Original file line number Diff line number Diff line change
Expand Up @@ -725,15 +725,15 @@
"description": "The absolute path to the file containing the unique id to display name mapping",
"create": true
},
"version": {
"ordinal": {
"type": "integer",
"description": "RESERVED FOR FUTURE USE",
"description": "Indicates the revision of the TLS certificates provided. Used by certificate rotation. Each time the TLS certificates associated with this profile are updated the ordinal value will be increased. The most recent version of the TLS credentials will have the highest numerical ordinal value.",
"create": true,
"update": true
},
"oldestValidVersion": {
"oldestValidOrdinal": {
"type": "integer",
"description": "RESERVED FOR FUTURE USE",
"description": "Used by certificate rotation to remove connections that were created using TLS certificates that are no longer considered valid. Any active connections based on TLS certificates an ordinal value less than oldesValidOrdinal will be immediately terminated by the router.",
"create": true,
"update": true
}
Expand Down
9 changes: 6 additions & 3 deletions src/adaptors/amqp/connection_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ QD_EXPORT qd_listener_t *qd_dispatch_configure_listener(qd_dispatch_t *qd, qd_en
QD_TLS_TYPE_PROTON_AMQP,
QD_TLS_CONFIG_SERVER_MODE,
li->config.verify_host_name,
li->config.ssl_require_peer_authentication);
li->config.ssl_require_peer_authentication,
0, 0);
if (!li->tls_config) {
// qd_tls_config() sets qd_error_message():
qd_log(LOG_CONN_MGR, QD_LOG_ERROR, "Failed to configure TLS for Listener %s: %s",
Expand Down Expand Up @@ -302,7 +303,8 @@ QD_EXPORT qd_connector_t *qd_dispatch_configure_connector(qd_dispatch_t *qd, qd_
QD_TLS_TYPE_PROTON_AMQP,
QD_TLS_CONFIG_CLIENT_MODE,
ct->config.verify_host_name,
ct->config.ssl_require_peer_authentication);
ct->config.ssl_require_peer_authentication,
0, 0);
if (!ct->tls_config) {
// qd_tls2_config() has set the qd_error_message(), which is logged below
goto error;
Expand Down Expand Up @@ -358,7 +360,8 @@ QD_EXPORT qd_connector_t *qd_dispatch_configure_connector(qd_dispatch_t *qd, qd_
QD_TLS_TYPE_PROTON_AMQP,
QD_TLS_CONFIG_CLIENT_MODE,
ct->config.verify_host_name,
ct->config.ssl_require_peer_authentication);
ct->config.ssl_require_peer_authentication,
0, 0);
if (!dc->tls_config) {
// qd_tls2_config() has set the qd_error_message(), which is logged below
qd_connector_decref(dc);
Expand Down
6 changes: 4 additions & 2 deletions src/adaptors/tcp/tcp_adaptor.c
Original file line number Diff line number Diff line change
Expand Up @@ -2384,7 +2384,8 @@ QD_EXPORT void *qd_dispatch_configure_tcp_listener(qd_dispatch_t *qd, qd_entity_
QD_TLS_TYPE_PROTON_RAW,
QD_TLS_CONFIG_SERVER_MODE,
listener->adaptor_config->verify_host_name,
listener->adaptor_config->authenticate_peer);
listener->adaptor_config->authenticate_peer,
0, 0);
if (!listener->tls_config) {
qd_log(LOG_TCP_ADAPTOR, QD_LOG_ERROR, "tcpListener %s TLS configuration failed: %s",
listener->adaptor_config->name, qd_error_message());
Expand Down Expand Up @@ -2590,7 +2591,8 @@ qd_tcp_connector_t *qd_dispatch_configure_tcp_connector(qd_dispatch_t *qd, qd_en
QD_TLS_TYPE_PROTON_RAW,
QD_TLS_CONFIG_CLIENT_MODE,
connector->adaptor_config->verify_host_name,
connector->adaptor_config->authenticate_peer);
connector->adaptor_config->authenticate_peer,
0, 0);
if (!connector->tls_config) {
qd_log(LOG_TCP_ADAPTOR, QD_LOG_ERROR, "tcpConnector %s TLS configuration failed: %s",
connector->adaptor_config->name, qd_error_message());
Expand Down
7 changes: 5 additions & 2 deletions src/tls/private.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ struct qd_tls_session_t {
// copies from parent qd_tls_config_t to avoid locking during I/O:
char *ssl_profile_name;
char *uid_format;
long version;
uint64_t ordinal;

bool tls_error;
bool output_eos; // pn_tls_close_output() called
Expand Down Expand Up @@ -97,7 +97,10 @@ struct qd_tls_config_t {
qd_proton_config_t *proton_tls_cfg; // lock must be held
qd_tls_type_t p_type;
sys_atomic_t ref_count;
long version; // lock must be held
uint64_t ordinal; // lock must be held
uint64_t oldest_valid_ordinal; // lock must be held
qd_tls_ordinal_update_cb_t ordinal_update_callback;
void *ordinal_update_context;

bool authenticate_peer;
bool verify_hostname;
Expand Down
49 changes: 35 additions & 14 deletions src/tls/tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,9 @@ qd_tls_config_t *qd_tls_config(const char *ssl_profile_name,
qd_tls_type_t p_type,
qd_tls_config_mode_t mode,
bool verify_hostname,
bool authenticate_peer)
bool authenticate_peer,
void *update_cb_context,
qd_tls_ordinal_update_cb_t callback)
{
ASSERT_MGMT_THREAD; // called from listener/connector create callback

Expand Down Expand Up @@ -283,12 +285,16 @@ qd_tls_config_t *qd_tls_config(const char *ssl_profile_name,

tls_config->ssl_profile_name = qd_strdup(ssl_profile_name);
tls_config->uid_format = CHECKED_STRDUP(tls_context->profile.uid_format);
tls_config->version = tls_context->profile.version;
tls_config->ordinal = tls_context->profile.ordinal;
tls_config->oldest_valid_ordinal = tls_context->profile.oldest_valid_ordinal;
tls_config->authenticate_peer = authenticate_peer;
tls_config->verify_hostname = verify_hostname;
tls_config->is_listener = is_listener;
tls_config->p_type = p_type;
tls_config->proton_tls_cfg = qd_proton_config(pn_raw_config, pn_amqp_config);
tls_config->ordinal_update_callback = callback;
tls_config->ordinal_update_context = update_cb_context;

DEQ_INSERT_TAIL(tls_context->tls_configs, tls_config);

return tls_config;
Expand Down Expand Up @@ -339,7 +345,7 @@ qd_tls_session_t *qd_tls_session_raw(qd_tls_config_t *tls_config, const char *pe
assert(p_cfg);
sys_atomic_inc(&p_cfg->ref_count); // prevents free after we drop lock
tls_session->uid_format = CHECKED_STRDUP(tls_config->uid_format); // may be changed by mgmt thread
tls_session->version = tls_config->version; // may be changed by mgmt thread
tls_session->ordinal = tls_config->ordinal; // may be changed by mgmt thread
sys_mutex_unlock(&tls_config->lock);

tls_session->proton_tls_cfg = p_cfg;
Expand Down Expand Up @@ -434,7 +440,7 @@ qd_tls_session_t *qd_tls_session_amqp(qd_tls_config_t *tls_config, pn_transport_
assert(p_cfg);
sys_atomic_inc(&p_cfg->ref_count); // prevents free after we drop lock
tls_session->uid_format = CHECKED_STRDUP(tls_config->uid_format); // may be changed by mgmt thread
tls_session->version = tls_config->version; // may be changed by mgmt thread
tls_session->ordinal = tls_config->ordinal; // may be changed by mgmt thread
sys_mutex_unlock(&tls_config->lock);

tls_session->proton_tls_cfg = p_cfg;
Expand Down Expand Up @@ -607,8 +613,8 @@ qd_ssl2_profile_t *qd_tls_read_ssl_profile(const char *ssl_profile_name, qd_ssl2
profile->private_key_file = CHECKED_STRDUP(tls_context->profile.private_key_file);
profile->uid_name_mapping_file = CHECKED_STRDUP(tls_context->profile.uid_name_mapping_file);
profile->trusted_certificate_db = CHECKED_STRDUP(tls_context->profile.trusted_certificate_db);
profile->version = tls_context->profile.version;
profile->oldest_valid_version = tls_context->profile.oldest_valid_version;
profile->ordinal = tls_context->profile.ordinal;
profile->oldest_valid_ordinal = tls_context->profile.oldest_valid_ordinal;

return profile;
}
Expand Down Expand Up @@ -637,6 +643,8 @@ static qd_error_t _read_tls_profile(qd_entity_t *entity, qd_ssl2_profile_t *prof
{
ZERO(profile);

long ordinal;
long oldest_valid_ordinal;
char *name = 0;
name = qd_entity_opt_string(entity, "name", "<NONE>");
if (qd_error_code()) goto error;
Expand All @@ -657,9 +665,9 @@ static qd_error_t _read_tls_profile(qd_entity_t *entity, qd_ssl2_profile_t *prof
if (qd_error_code()) goto error;
profile->uid_name_mapping_file = qd_entity_opt_string(entity, "uidNameMappingFile", 0);
if (qd_error_code()) goto error;
profile->version = qd_entity_opt_long(entity, "version", 0);
ordinal = qd_entity_opt_long(entity, "ordinal", 0);
if (qd_error_code()) goto error;
profile->oldest_valid_version = qd_entity_opt_long(entity, "oldestValidVersion", 0);
oldest_valid_ordinal = qd_entity_opt_long(entity, "oldestValidOrdinal", 0);
if (qd_error_code()) goto error;

if (profile->uid_format) {
Expand Down Expand Up @@ -695,16 +703,19 @@ static qd_error_t _read_tls_profile(qd_entity_t *entity, qd_ssl2_profile_t *prof
}
}

// simple validation of version fields:
if (profile->version < 0 || profile->oldest_valid_version < 0) {
qd_error(QD_ERROR_CONFIG, "Negative version field values are invalid (sslProfile '%s')", name);
// simple validation of ordinal fields:
if (ordinal < 0 || oldest_valid_ordinal < 0) {
qd_error(QD_ERROR_CONFIG, "Negative ordinal field values are invalid (sslProfile '%s')", name);
goto error;
}
if (profile->version < profile->oldest_valid_version) {
qd_error(QD_ERROR_CONFIG, "version must be >= oldestValidVersion (sslProfile '%s')", name);
if (ordinal < oldest_valid_ordinal) {
qd_error(QD_ERROR_CONFIG, "ordinal must be >= oldestValidOrdinal (sslProfile '%s')", name);
goto error;
}

profile->ordinal = (uint64_t) ordinal;
profile->oldest_valid_ordinal = (uint64_t) oldest_valid_ordinal;

free(name);
return QD_ERROR_NONE;

Expand Down Expand Up @@ -778,6 +789,9 @@ static qd_error_t _update_tls_config(qd_tls_config_t *tls_config, const qd_ssl2_
break;
}

bool ordinal_updated = profile->ordinal != tls_config->ordinal
|| profile->oldest_valid_ordinal != tls_config->oldest_valid_ordinal;

qd_proton_config_t *new_cfg = qd_proton_config(pn_raw_config, pn_amqp_config);
qd_proton_config_t *old_cfg = 0;

Expand All @@ -788,11 +802,18 @@ static qd_error_t _update_tls_config(qd_tls_config_t *tls_config, const qd_ssl2_
old_cfg = tls_config->proton_tls_cfg;
tls_config->proton_tls_cfg = new_cfg;
// And refresh any parameters that must be used when creating new sessions:
tls_config->version = profile->version;
tls_config->ordinal = profile->ordinal;
tls_config->oldest_valid_ordinal = profile->oldest_valid_ordinal;
free(tls_config->uid_format);
tls_config->uid_format = CHECKED_STRDUP(profile->uid_format);
sys_mutex_unlock(&tls_config->lock);

if (tls_config->ordinal_update_callback && ordinal_updated) {
tls_config->ordinal_update_callback(tls_config->ordinal,
tls_config->oldest_valid_ordinal,
tls_config->ordinal_update_context);
}

// no need to hold the lock here because the decref is atomic and if this
// is the last reference then by definition there are no other threads involved.
qd_proton_config_decref(old_cfg);
Expand Down
10 changes: 5 additions & 5 deletions tests/system_tests_one_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ def setUpClass(cls):
'certFile': SERVER_CERTIFICATE,
'privateKeyFile': SERVER_PRIVATE_KEY,
'password': "server-password",
'version': -1})
'ordinal': -1})
])
cls.routers.append(cls.tester.qdrouterd(name, config_23, wait=False, expect=Process.EXIT_FAIL))

Expand All @@ -504,8 +504,8 @@ def setUpClass(cls):
'certFile': SERVER_CERTIFICATE,
'privateKeyFile': SERVER_PRIVATE_KEY,
'password': "server-password",
'version': 1,
'oldestValidVersion': 42})
'ordinal': 1,
'oldestValidOrdinal': 42})
])
cls.routers.append(cls.tester.qdrouterd(name, config_24, wait=False, expect=Process.EXIT_FAIL))

Expand Down Expand Up @@ -668,10 +668,10 @@ def test_48_router_in_error(self):
err = "Failed to configure TLS Ciphers 'Blah-Blah-Blabbity-Blab' for sslProfile 'BadCipherProfile'"
self.routers[22].wait_log_message(err, timeout=1.0)

err = "Negative version field values are invalid"
err = "Negative ordinal field values are invalid"
self.routers[23].wait_log_message(err, timeout=1.0)

err = "version must be >= oldestValidVersion"
err = "ordinal must be >= oldestValidOrdinal"
self.routers[24].wait_log_message(err, timeout=1.0)

with open(self.routers[25].outfile + '.out', 'r') as out_file:
Expand Down

0 comments on commit 42cbf56

Please sign in to comment.