Skip to content

Commit

Permalink
generate or propagate traceparent header
Browse files Browse the repository at this point in the history
- using OIDCTraceParent; closes #1152; thanks @studersi
- include hostname/port/pid in User-Agent header on outgoing requests
- bump to 2.4.15rc12

Signed-off-by: Hans Zandbelt <[email protected]>
  • Loading branch information
zandbelt committed Dec 20, 2023
1 parent 9206c25 commit 0d1e17c
Show file tree
Hide file tree
Showing 11 changed files with 160 additions and 22 deletions.
5 changes: 5 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
12/20/2023
- generate or propagate traceparent header using OIDCTraceParent; closes #1152; thanks @studersi
- include hostname,port and process id in User-Agent header on outgoing requests
- bump to 2.4.15rc12

12/19/2023
- metrics update:
- rename "requests" class to "provider"
Expand Down
8 changes: 7 additions & 1 deletion auth_openidc.conf
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,7 @@
# authn Authentication request creation and response processing.
# authz Authorization errors per OIDCUnAuthzAction (per Require statement, not overall).
# require.claim Match/failure count of Require claim directives (per Require statement, not overall).
# provider Requests to the provider endpoints.
# provider Requests to the provider [token, userinfo, metadata] endpoints.
# session Existing session processing.
# cache Cache read/write timings and errors.
# redirect_uri Requests to the Redirect URI, per type.
Expand All @@ -1036,6 +1036,12 @@
# When not defined, no metrics will be published on the enclosing vhost.
#OIDCMetricsPublish <path>

# Set a traceparent HTTP header on outgoing requests to the provider and proxied requests.
# propagate: propagate any existing traceparent header on requests to the Provider (it's proxied as it is)
# generate: generate a traceparent header, possibly overwriting an existing one
# The default is to not add (or overwrite) a traceparent header.
#OIDCTraceParent generate | propagate

# Specify claims that should be removed from the userinfo and/or id_token before storing them in the session.
# Note that OIDCBlackListedClaims takes precedence over OIDCWhiteListedClaims
# When not defined no claims are blacklisted and all claims are stored except when OIDCWhiteListedClaims is used.
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
AC_INIT([mod_auth_openidc],[2.4.15rc11],[[email protected]])
AC_INIT([mod_auth_openidc],[2.4.15rc12],[[email protected]])

AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION())

Expand Down
14 changes: 14 additions & 0 deletions src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@
#define OIDCXForwardedHeaders "OIDCXForwardedHeaders"
#define OIDCUserInfoClaimsExpr "OIDCUserInfoClaimsExpr"
#define OIDCFilterClaimsExpr "OIDCFilterClaimsExpr"
#define OIDCTraceParent "OIDCTraceParent"

extern module AP_MODULE_DECLARE_DATA auth_openidc_module;

Expand Down Expand Up @@ -1137,6 +1138,12 @@ static const char *oidc_set_metrics_hook_data(cmd_parms *cmd, void *m, const cha
return NULL;
}

static const char *oidc_set_trace_parent(cmd_parms *cmd, void *struct_ptr, const char *arg) {
oidc_cfg *cfg = (oidc_cfg *)ap_get_module_config(cmd->server->module_config, &auth_openidc_module);
const char *rv = oidc_parse_trace_parent(cmd->pool, arg, &cfg->trace_parent);
return OIDC_CONFIG_DIR_RV(cmd, rv);
}

static const char *oidc_set_filtered_claims(cmd_parms *cmd, void *m, const char *arg) {
oidc_cfg *cfg = (oidc_cfg *)ap_get_module_config(cmd->server->module_config, &auth_openidc_module);
int offset = (int)(long)cmd->info;
Expand Down Expand Up @@ -1657,6 +1664,7 @@ void *oidc_create_server_config(apr_pool_t *pool, server_rec *svr) {
c->info_hook_data = NULL;
c->metrics_hook_data = NULL;
c->metrics_path = NULL;
c->trace_parent = OIDC_TRACE_PARENT_OFF;

c->black_listed_claims = NULL;
c->white_listed_claims = NULL;
Expand Down Expand Up @@ -1906,6 +1914,7 @@ void *oidc_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) {
c->info_hook_data = add->info_hook_data != NULL ? add->info_hook_data : base->info_hook_data;
c->metrics_hook_data = add->metrics_hook_data != NULL ? add->metrics_hook_data : base->metrics_hook_data;
c->metrics_path = add->metrics_path != NULL ? add->metrics_path : base->metrics_path;
c->trace_parent = add->trace_parent != OIDC_TRACE_PARENT_OFF ? add->trace_parent : base->trace_parent;

c->black_listed_claims =
add->black_listed_claims != NULL ? add->black_listed_claims : base->black_listed_claims;
Expand Down Expand Up @@ -3409,6 +3418,11 @@ const command_rec oidc_config_cmds[] = {
(void *)APR_OFFSETOF(oidc_cfg, metrics_path),
RSRC_CONF,
"Define the URL where the metrics will be published (e.g.: /metrics)"),
AP_INIT_TAKE1(OIDCTraceParent,
oidc_set_trace_parent,
(void *)APR_OFFSETOF(oidc_cfg, trace_parent),
RSRC_CONF,
"Define to propagagte or generate a traceparent header"),
AP_INIT_ITERATE(OIDCBlackListedClaims,
oidc_set_filtered_claims,
(void *) APR_OFFSETOF(oidc_cfg, black_listed_claims),
Expand Down
4 changes: 3 additions & 1 deletion src/mod_auth_openidc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1919,7 +1919,7 @@ static apr_byte_t oidc_save_in_session(request_rec *r, oidc_cfg *c, oidc_session
const char *original_url, const char *userinfo_jwt) {

/* store the user in the session */
session->remote_user = remoteUser;
session->remote_user = apr_pstrdup(r->pool, remoteUser);

/* set the session expiry to the inactivity timeout */
session->expiry = apr_time_now() + apr_time_from_sec(c->session_inactivity_timeout);
Expand Down Expand Up @@ -4281,6 +4281,8 @@ int oidc_check_user_id(request_rec *r) {
return DECLINED;
}

oidc_util_set_trace_parent(r, c, NULL);

OIDC_METRICS_COUNTER_INC(r, c, OM_AUTHTYPE_MOD_AUTH_OPENIDC);

/* see if we've configured OpenID Connect user authentication for this request */
Expand Down
21 changes: 15 additions & 6 deletions src/mod_auth_openidc.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ APLOG_USE_MODULE(auth_openidc);
#define OIDC_REQUEST_STATE_KEY_DISCOVERY "d"
#define OIDC_REQUEST_STATE_KEY_AUTHN "a"
#define OIDC_REQUEST_STATE_KEY_SAVE "s"
#define OIDC_REQUEST_STATE_TRACE_ID "t"

/* parameter name of the callback URL in the discovery response */
#define OIDC_DISC_CB_PARAM "oidc_callback"
Expand Down Expand Up @@ -285,6 +286,10 @@ APLOG_USE_MODULE(auth_openidc);
#define OIDC_HDR_X_FORWARDED_PROTO 4
#define OIDC_HDR_FORWARDED 8

#define OIDC_TRACE_PARENT_OFF 0
#define OIDC_TRACE_PARENT_PROPAGATE 1
#define OIDC_TRACE_PARENT_GENERATE 2

typedef apr_byte_t (*oidc_proto_pkce_state)(request_rec *r, char **state);
typedef apr_byte_t (*oidc_proto_pkce_challenge)(request_rec *r, const char *state, char **code_challenge);
typedef apr_byte_t (*oidc_proto_pkce_verifier)(request_rec *r, const char *state, char **code_verifier);
Expand Down Expand Up @@ -512,6 +517,7 @@ typedef struct oidc_cfg {
apr_hash_t *info_hook_data;
apr_hash_t *metrics_hook_data;
char *metrics_path;
int trace_parent;

apr_hash_t *black_listed_claims;
apr_hash_t *white_listed_claims;
Expand Down Expand Up @@ -985,6 +991,7 @@ oidc_jwk_t *oidc_util_key_list_first(const apr_array_header_t *key_list, int kty
const char *oidc_util_jq_filter(request_rec *r, const char *input, const char *filter);
char *oidc_util_apr_expr_parse(cmd_parms *cmd, const char *str, oidc_apr_expr_t **expr, apr_byte_t result_is_str);
const char *oidc_util_apr_expr_exec(request_rec *r, const oidc_apr_expr_t *expr, apr_byte_t result_is_str);
void oidc_util_set_trace_parent(request_rec *r, oidc_cfg *c, const char *span);

/* HTTP header constants */
#define OIDC_HTTP_HDR_COOKIE "Cookie"
Expand All @@ -1010,6 +1017,7 @@ const char *oidc_util_apr_expr_exec(request_rec *r, const oidc_apr_expr_t *expr,
#define OIDC_HTTP_HDR_EXPIRES "Expires"
#define OIDC_HTTP_HDR_X_FRAME_OPTIONS "X-Frame-Options"
#define OIDC_HTTP_HDR_WWW_AUTHENTICATE "WWW-Authenticate"
#define OIDC_HTTP_HDR_TRACE_PARENT "traceparent"

#define OIDC_HTTP_HDR_VAL_XML_HTTP_REQUEST "XMLHttpRequest"
#define OIDC_HTTP_HDR_VAL_NAVIGATE "navigate"
Expand All @@ -1032,6 +1040,7 @@ const char *oidc_util_hdr_in_x_forwarded_port_get(const request_rec *r);
const char *oidc_util_hdr_in_x_forwarded_host_get(const request_rec *r);
const char *oidc_util_hdr_in_forwarded_get(const request_rec *r);
const char *oidc_util_hdr_in_host_get(const request_rec *r);
const char *oidc_util_hdr_in_traceparent_get(const request_rec *r);
void oidc_util_hdr_out_location_set(const request_rec *r, const char *value);
const char *oidc_util_hdr_out_location_get(const request_rec *r);
void oidc_util_hdr_err_out_add(const request_rec *r, const char *name, const char *value);
Expand All @@ -1056,15 +1065,15 @@ apr_byte_t oidc_oauth_metadata_provider_parse(request_rec *r, oidc_cfg *c, json_

// oidc_session.c
typedef struct {
char *uuid; /* unique id */
const char *remote_user; /* user who owns this particular session */
json_t *state; /* the state for this session, encoded in a JSON object */
apr_time_t expiry; /* if > 0, the time of expiry of this session */
const char *sid;
char *uuid; /* unique id */
char *remote_user; /* user who owns this particular session */
json_t *state; /* the state for this session, encoded in a JSON object */
apr_time_t expiry; /* if > 0, the time of expiry of this session */
char *sid;
} oidc_session_t;

apr_byte_t oidc_session_load(request_rec *r, oidc_session_t **z);
apr_byte_t oidc_session_get(request_rec *r, oidc_session_t *z, const char *key, const char **value);
apr_byte_t oidc_session_get(request_rec *r, oidc_session_t *z, const char *key, char **value);
apr_byte_t oidc_session_set(request_rec *r, oidc_session_t *z, const char *key, const char *value);
apr_byte_t oidc_session_save(request_rec *r, oidc_session_t *z, apr_byte_t first_time);
apr_byte_t oidc_session_kill(request_rec *r, oidc_session_t *z);
Expand Down
2 changes: 2 additions & 0 deletions src/oauth.c
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,8 @@ int oidc_oauth_check_userid(request_rec *r, oidc_cfg *c, const char *access_toke
}
}

oidc_util_set_trace_parent(r, c, access_token);

/* validate the obtained access token against the OAuth AS validation endpoint */
json_t *token = NULL;
char *s_token = NULL;
Expand Down
21 changes: 21 additions & 0 deletions src/parse.c
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,27 @@ const char *oidc_parse_info_hook_data(apr_pool_t *pool, const char *arg, apr_has
return NULL;
}

#define OIDC_TRACE_PARENT_PROPAGATE_STR "propagate"
#define OIDC_TRACE_PARENT_GENERATE_STR "generate"

const char *valid_trace_parent_value(apr_pool_t *pool, const char *arg) {
static char *options[] = {OIDC_TRACE_PARENT_PROPAGATE_STR, OIDC_TRACE_PARENT_GENERATE_STR, NULL};
return oidc_valid_string_option(pool, arg, options);
}

const char *oidc_parse_trace_parent(apr_pool_t *pool, const char *arg, int *trace_parent) {
const char *rv = valid_trace_parent_value(pool, arg);
if (rv != NULL)
return rv;

if (_oidc_strcmp(arg, OIDC_TRACE_PARENT_PROPAGATE_STR) == 0)
*trace_parent = OIDC_TRACE_PARENT_PROPAGATE;
else if (_oidc_strcmp(arg, OIDC_TRACE_PARENT_GENERATE_STR) == 0)
*trace_parent = OIDC_TRACE_PARENT_GENERATE;

return NULL;
}

#define OIDC_AUTH_REQUEST_METHOD_GET_STR "GET"
#define OIDC_AUTH_REQEUST_METHOD_POST_STR "POST"

Expand Down
1 change: 1 addition & 0 deletions src/parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ const char *oidc_parse_refresh_access_token_before_expiry(apr_pool_t *pool, cons
const char *oidc_parse_set_state_input_headers_as(apr_pool_t *pool, const char *arg, apr_byte_t *state_input_headers);
const char *oidc_parse_x_forwarded_headers(apr_pool_t *pool, const char *arg, apr_byte_t *x_forwarded_headers);
const char *oidc_parse_outgoing_proxy_auth_type(apr_pool_t *pool, const char *arg, unsigned long *auth_type);
const char *oidc_parse_trace_parent(apr_pool_t *pool, const char *arg, int *trace_parent);

typedef const char *(*oidc_valid_int_function_t)(apr_pool_t *, int);
typedef const char *(*oidc_valid_function_t)(apr_pool_t *, const char *);
Expand Down
21 changes: 9 additions & 12 deletions src/session.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,8 @@ void oidc_session_id_new(request_rec *r, oidc_session_t *z) {
* clear contents of a session
*/
static void oidc_session_clear(request_rec *r, oidc_session_t *z) {
z->uuid = NULL;
z->remote_user = NULL;
// NB: don't clear sid
// NB: don't clear sid or uuid
z->expiry = 0;
if (z->state) {
json_decref(z->state);
Expand All @@ -116,7 +115,7 @@ static void oidc_session_clear(request_rec *r, oidc_session_t *z) {
}

apr_byte_t oidc_session_load_cache_by_uuid(request_rec *r, oidc_cfg *c, const char *uuid, oidc_session_t *z) {
const char *stored_uuid = NULL;
char *stored_uuid = NULL;
char *s_json = NULL;
apr_byte_t rc = FALSE;

Expand Down Expand Up @@ -191,13 +190,6 @@ static apr_byte_t oidc_session_save_cache(request_rec *r, oidc_session_t *z, apr

if (z->state != NULL) {

if (z->uuid == NULL) {
/* get a new id for this session */
oidc_session_id_new(r, z);
/* store the session id in the cache value so it allows us to detect cache corruption */
oidc_session_set(r, z, OIDC_SESSION_SESSION_ID, z->uuid);
}

if (z->sid != NULL) {
oidc_cache_set_sid(r, z->sid, z->uuid, z->expiry);
oidc_session_set(r, z, OIDC_SESSION_SID_KEY, z->sid);
Expand Down Expand Up @@ -282,6 +274,7 @@ apr_byte_t oidc_session_extract(request_rec *r, oidc_session_t *z) {

oidc_session_get(r, z, OIDC_SESSION_REMOTE_USER_KEY, &z->remote_user);
oidc_session_get(r, z, OIDC_SESSION_SID_KEY, &z->sid);
oidc_session_get(r, z, OIDC_SESSION_SESSION_ID, &z->uuid);

rc = TRUE;

Expand All @@ -301,6 +294,7 @@ apr_byte_t oidc_session_load(request_rec *r, oidc_session_t **zz) {
/* allocate space for the session object and fill it */
oidc_session_t *z = (*zz = apr_pcalloc(r->pool, sizeof(oidc_session_t)));
oidc_session_clear(r, z);
oidc_session_id_new(r, z);
z->sid = NULL;

if (c->session_type == OIDC_SESSION_TYPE_SERVER_CACHE)
Expand All @@ -316,6 +310,8 @@ apr_byte_t oidc_session_load(request_rec *r, oidc_session_t **zz) {
if (rc == TRUE)
rc = oidc_session_extract(r, z);

oidc_util_set_trace_parent(r, c, z->uuid);

return rc;
}

Expand All @@ -330,6 +326,7 @@ apr_byte_t oidc_session_save(request_rec *r, oidc_session_t *z, apr_byte_t first
if (z->state != NULL) {
oidc_session_set(r, z, OIDC_SESSION_REMOTE_USER_KEY, z->remote_user);
json_object_set_new(z->state, OIDC_SESSION_EXPIRY_KEY, json_integer(apr_time_sec(z->expiry)));
oidc_session_set(r, z, OIDC_SESSION_SESSION_ID, z->uuid);
}

if (c->session_type == OIDC_SESSION_TYPE_SERVER_CACHE)
Expand Down Expand Up @@ -369,7 +366,7 @@ apr_byte_t oidc_session_kill(request_rec *r, oidc_session_t *z) {
/*
* get a value from the session based on the name from a name/value pair
*/
apr_byte_t oidc_session_get(request_rec *r, oidc_session_t *z, const char *key, const char **value) {
apr_byte_t oidc_session_get(request_rec *r, oidc_session_t *z, const char *key, char **value) {

/* just return the value for the key */
oidc_json_object_get_string(r->pool, z->state, key, (char **)value, NULL);
Expand Down Expand Up @@ -450,7 +447,7 @@ static json_t *oidc_session_get_str2json(request_rec *r, oidc_session_t *z,
}

static const char *oidc_session_get_key2string(request_rec *r, oidc_session_t *z, const char *key) {
const char *s_value = NULL;
char *s_value = NULL;
oidc_session_get(r, z, key, &s_value);
return s_value;
}
Expand Down
Loading

0 comments on commit 0d1e17c

Please sign in to comment.