Skip to content

Commit c94df68

Browse files
gary-archergary-archer
authored
Merge branch 'master' into feature/nginx_plus_updates
2 parents bc0554c + 6ad4947 commit c94df68

File tree

2 files changed

+168
-25
lines changed

2 files changed

+168
-25
lines changed

‎phantom_token.c

+88-21
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ static char *merge_location_configuration(ngx_conf_t *main_config, void *parent,
5353
static ngx_int_t introspection_response_handler(ngx_http_request_t *request, void *data,
5454
ngx_int_t introspection_subrequest_status_code);
5555

56+
static ngx_int_t write_error_response(ngx_http_request_t *request, ngx_int_t status, phantom_token_configuration_t *module_location_config);
57+
5658
/**
5759
* Adds a WWW-Authenticate header to the given request's output headers that conforms to <a href="https://tools.ietf.org/html/rfc6750">RFC 6750</>.
5860
*
@@ -75,8 +77,7 @@ static ngx_int_t introspection_response_handler(ngx_http_request_t *request, voi
7577
*
7678
* @see <a href="https://tools.ietf.org/html/rfc6750">RFC 6750</a>
7779
*/
78-
static ngx_int_t set_www_authenticate_header(ngx_http_request_t *request, ngx_str_t realm,
79-
ngx_str_t space_separated_scopes, char *error_code);
80+
static ngx_int_t set_www_authenticate_header(ngx_http_request_t *request, phantom_token_configuration_t *module_location_config, char *error_code);
8081

8182
/**
8283
* Sets the base-64-encoded client ID and secret in the module's configuration setting structure.
@@ -287,20 +288,19 @@ static ngx_int_t handler(ngx_http_request_t *request)
287288
}
288289
else if (module_context->status == NGX_HTTP_NO_CONTENT)
289290
{
290-
return set_www_authenticate_header(request, module_location_config->realm,
291-
module_location_config->space_separated_scopes, NULL);
291+
return set_www_authenticate_header(request, module_location_config, NULL);
292292
}
293293
else if (module_context->status == NGX_HTTP_SERVICE_UNAVAILABLE)
294294
{
295-
return NGX_HTTP_SERVICE_UNAVAILABLE;
295+
return write_error_response(request, NGX_HTTP_SERVICE_UNAVAILABLE, module_location_config);
296296
}
297297
else if (module_context->status >= NGX_HTTP_INTERNAL_SERVER_ERROR || module_context->status == NGX_HTTP_NOT_FOUND
298298
|| module_context->status == NGX_HTTP_UNAUTHORIZED || module_context->status == NGX_HTTP_FORBIDDEN)
299299
{
300-
return NGX_HTTP_BAD_GATEWAY;
300+
return write_error_response(request, NGX_HTTP_BAD_GATEWAY, module_location_config);
301301
}
302302

303-
return NGX_HTTP_INTERNAL_SERVER_ERROR;
303+
return write_error_response(request, NGX_HTTP_INTERNAL_SERVER_ERROR, module_location_config);
304304
}
305305

306306
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, request->connection->log, 0,
@@ -314,8 +314,7 @@ static ngx_int_t handler(ngx_http_request_t *request)
314314
{
315315
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, request->connection->log, 0, "Authorization header not present");
316316

317-
return set_www_authenticate_header(request, module_location_config->realm,
318-
module_location_config->space_separated_scopes, NULL);
317+
return set_www_authenticate_header(request, module_location_config, NULL);
319318
}
320319

321320
u_char *bearer_token_pos;
@@ -328,8 +327,7 @@ static ngx_int_t handler(ngx_http_request_t *request)
328327
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, request->connection->log, 0,
329328
"Authorization header does not contain a bearer token");
330329

331-
return set_www_authenticate_header(request, module_location_config->realm,
332-
module_location_config->space_separated_scopes, NULL);
330+
return set_www_authenticate_header(request, module_location_config, NULL);
333331
}
334332

335333
bearer_token_pos += BEARER_SIZE;
@@ -361,7 +359,7 @@ static ngx_int_t handler(ngx_http_request_t *request)
361359
if (ngx_http_subrequest(request, &module_location_config->introspection_endpoint, NULL, &introspection_request,
362360
introspection_request_callback, NGX_HTTP_SUBREQUEST_WAITED) != NGX_OK)
363361
{
364-
return NGX_HTTP_INTERNAL_SERVER_ERROR;
362+
write_error_response(request, NGX_HTTP_INTERNAL_SERVER_ERROR, module_location_config);
365363
}
366364

367365
// extract access token from header
@@ -490,8 +488,7 @@ static ngx_int_t handler(ngx_http_request_t *request)
490488
return NGX_AGAIN;
491489
}
492490

493-
static ngx_int_t set_www_authenticate_header(ngx_http_request_t *request, ngx_str_t realm,
494-
ngx_str_t space_separated_scopes, char *error_code)
491+
static ngx_int_t set_www_authenticate_header(ngx_http_request_t *request, phantom_token_configuration_t *module_location_config, char *error_code)
495492
{
496493
request->headers_out.www_authenticate = ngx_list_push(&request->headers_out.headers);
497494

@@ -516,20 +513,20 @@ static ngx_int_t set_www_authenticate_header(ngx_http_request_t *request, ngx_st
516513
static const size_t ERROR_CODE_PREFIX_SIZE = sizeof(ERROR_CODE_PREFIX) - 1;
517514

518515
size_t bearer_data_size = BEARER_SIZE + sizeof('\0'); // Add one for the nul byte
519-
bool realm_provided = realm.len > 0;
520-
bool scopes_provided = space_separated_scopes.len > 0;
516+
bool realm_provided = module_location_config->realm.len > 0;
517+
bool scopes_provided = module_location_config->space_separated_scopes.len > 0;
521518
bool error_code_provided = error_code != NULL;
522519
bool append_one_comma = false, append_two_commas = false;
523520
size_t error_code_len = 0;
524521

525522
if (realm_provided)
526523
{
527-
bearer_data_size += REALM_PREFIX_SIZE + realm.len + TOKEN_SUFFIX_SIZE;
524+
bearer_data_size += REALM_PREFIX_SIZE + module_location_config->realm.len + TOKEN_SUFFIX_SIZE;
528525
}
529526

530527
if (scopes_provided)
531528
{
532-
bearer_data_size += SCOPE_PREFIX_SIZE + space_separated_scopes.len + TOKEN_SUFFIX_SIZE;
529+
bearer_data_size += SCOPE_PREFIX_SIZE + module_location_config->space_separated_scopes.len + TOKEN_SUFFIX_SIZE;
533530
}
534531

535532
if (error_code_provided)
@@ -562,7 +559,7 @@ static ngx_int_t set_www_authenticate_header(ngx_http_request_t *request, ngx_st
562559
if (realm_provided)
563560
{
564561
p = ngx_cpymem(p, REALM_PREFIX, REALM_PREFIX_SIZE);
565-
p = ngx_cpymem(p, realm.data, realm.len);
562+
p = ngx_cpymem(p, module_location_config->realm.data, module_location_config->realm.len);
566563
p = ngx_cpymem(p, TOKEN_SUFFIX, TOKEN_SUFFIX_SIZE);
567564

568565
if (append_one_comma)
@@ -574,7 +571,7 @@ static ngx_int_t set_www_authenticate_header(ngx_http_request_t *request, ngx_st
574571
if (scopes_provided)
575572
{
576573
p = ngx_cpymem(p, SCOPE_PREFIX, SCOPE_PREFIX_SIZE);
577-
p = ngx_cpymem(p, space_separated_scopes.data, space_separated_scopes.len);
574+
p = ngx_cpymem(p, module_location_config->space_separated_scopes.data, module_location_config->space_separated_scopes.len);
578575
p = ngx_cpymem(p, TOKEN_SUFFIX, TOKEN_SUFFIX_SIZE);
579576

580577
if (append_one_comma || append_two_commas)
@@ -607,7 +604,7 @@ static ngx_int_t set_www_authenticate_header(ngx_http_request_t *request, ngx_st
607604

608605
assert(request->headers_out.www_authenticate->value.len <= bearer_data_size);
609606

610-
return NGX_HTTP_UNAUTHORIZED;
607+
return write_error_response(request, NGX_HTTP_UNAUTHORIZED, module_location_config);
611608
}
612609

613610
static ngx_int_t introspection_response_handler(ngx_http_request_t *request, void *data,
@@ -811,3 +808,73 @@ static char* set_client_credential_configuration_slot(ngx_conf_t *config_setting
811808

812809
return "invalid_client_credential";
813810
}
811+
812+
/*
813+
* Add the error response as a JSON object that is easier to handle than the default HTML response that NGINX returns
814+
* http://nginx.org/en/docs/dev/development_guide.html#http_response_body
815+
*/
816+
static ngx_int_t write_error_response(ngx_http_request_t *request, ngx_int_t status, phantom_token_configuration_t *module_location_config)
817+
{
818+
ngx_int_t rc;
819+
ngx_str_t code;
820+
ngx_str_t message;
821+
u_char json_error_data[256];
822+
ngx_chain_t output;
823+
ngx_buf_t *body = NULL;
824+
const char *error_format = NULL;
825+
size_t error_len = 0;
826+
827+
if (request->method == NGX_HTTP_HEAD)
828+
{
829+
return status;
830+
}
831+
832+
body = ngx_calloc_buf(request->pool);
833+
if (body == NULL)
834+
{
835+
ngx_log_error(NGX_LOG_WARN, request->connection->log, 0, "Failed to allocate memory for error body");
836+
return NGX_HTTP_INTERNAL_SERVER_ERROR;
837+
}
838+
else
839+
{
840+
if (status == NGX_HTTP_UNAUTHORIZED)
841+
{
842+
ngx_str_set(&code, "unauthorized_request");
843+
ngx_str_set(&message, "Access denied due to missing, invalid or expired credentials");
844+
}
845+
else
846+
{
847+
ngx_str_set(&code, "server_error");
848+
ngx_str_set(&message, "Problem encountered processing the request");
849+
}
850+
851+
/* The string length calculation replaces the two '%V' markers with their actual values */
852+
error_format = "{\"code\":\"%V\",\"message\":\"%V\"}";
853+
error_len = ngx_strlen(error_format) + code.len + message.len - 4;
854+
ngx_snprintf(json_error_data, sizeof(json_error_data) - 1, error_format, &code, &message);
855+
json_error_data[error_len] = 0;
856+
857+
request->headers_out.status = status;
858+
request->headers_out.content_length_n = error_len;
859+
ngx_str_set(&request->headers_out.content_type, "application/json");
860+
861+
rc = ngx_http_send_header(request);
862+
if (rc == NGX_ERROR || rc > NGX_OK || request->header_only) {
863+
return rc;
864+
}
865+
866+
body->pos = json_error_data;
867+
body->last = json_error_data + error_len;
868+
body->memory = 1;
869+
body->last_buf = 1;
870+
body->last_in_chain = 1;
871+
output.buf = body;
872+
output.next = NULL;
873+
874+
/* Return an error result, which also requires finalize_request to be called, to prevent a 'header already sent' warning in logs
875+
https://forum.nginx.org/read.php?29,280514,280521#msg-280521 */
876+
rc = ngx_http_output_filter(request, &output);
877+
ngx_http_finalize_request(request, rc);
878+
return NGX_DONE;
879+
}
880+
}

‎t/curity.t

+80-4
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,13 @@ GET /t
100100
101101
--- error_code: 401
102102
103+
--- response_headers
104+
content-type: application/json
105+
WWW-Authenticate: Bearer realm="api"
106+
107+
--- response_body_like chomp
108+
{"code":"unauthorized_request","message":"Access denied due to missing, invalid or expired credentials"}
109+
103110
=== Test 3: The wrong kind of HTTP method is used results in an access denied error
104111
105112
--- config
@@ -143,7 +150,14 @@ GET /t
143150
144151
--- error_code: 401
145152
146-
=== Test 4: A valid token with trash after results in an access denied error
153+
--- response_headers
154+
content-type: application/json
155+
WWW-Authenticate: Bearer realm="api"
156+
157+
--- response_body_like chomp
158+
{"code":"unauthorized_request","message":"Access denied due to missing, invalid or expired credentials"}
159+
160+
=== Test 5: A valid token with trash after results in an access denied error
147161
148162
--- config
149163
location tt {
@@ -166,7 +180,11 @@ GET /t
166180
167181
--- error_code: 401
168182
169-
=== Test 5: The bearer HTTP method can be in upper case
183+
--- response_headers
184+
content-type: application/json
185+
WWW-Authenticate: Bearer realm="api"
186+
187+
=== Test 6: The bearer HTTP method can be in upper case
170188
171189
--- config
172190
location tt {
@@ -194,7 +212,7 @@ main::process_json_from_backend()
194212
195213
--- response_body: GOOD
196214
197-
=== Test 6: The bearer HTTP method can be in mixed case
215+
=== Test 7: The bearer HTTP method can be in mixed case
198216
199217
--- config
200218
location tt {
@@ -222,7 +240,7 @@ main::process_json_from_backend()
222240
223241
--- response_body: GOOD
224242
225-
=== Test 6: The bearer HTTP method can have > 1 space before it
243+
=== Test 8: The bearer HTTP method can have > 1 space before it
226244
227245
--- config
228246
location tt {
@@ -249,3 +267,61 @@ GET /t
249267
main::process_json_from_backend()
250268
251269
--- response_body: GOOD
270+
271+
=== Test 9: A misconfigured client secret results in a 502 error
272+
273+
--- config
274+
location tt {
275+
proxy_pass "http://localhost:8443/oauth/v2/oauth-introspect";
276+
}
277+
278+
location /t {
279+
proxy_pass "http://localhost:8080/anything";
280+
281+
phantom_token on;
282+
phantom_token_client_credential "test-nginx" "incorrect_secret";
283+
phantom_token_introspection_endpoint tt;
284+
}
285+
286+
--- error_code: 502
287+
288+
--- request
289+
GET /t
290+
291+
--- more_headers eval
292+
"Authorization: bearer " . $main::token
293+
294+
--- response_headers
295+
content-type: application/json
296+
297+
--- response_body_like chomp
298+
{"code":"server_error","message":"Problem encountered processing the request"}
299+
300+
=== Test 10: An unreachable authorization server results in a 502 error
301+
302+
--- config
303+
location tt {
304+
proxy_pass "http://localhost:9443/oauth/v2/oauth-introspect";
305+
}
306+
307+
location /t {
308+
proxy_pass "http://localhost:8080/anything";
309+
310+
phantom_token on;
311+
phantom_token_client_credential "test-nginx" "secret2";
312+
phantom_token_introspection_endpoint tt;
313+
}
314+
315+
--- error_code: 502
316+
317+
--- request
318+
GET /t
319+
320+
--- more_headers eval
321+
"Authorization: bearer " . $main::token
322+
323+
--- response_headers
324+
content-type: application/json
325+
326+
--- response_body_like chomp
327+
{"code":"server_error","message":"Problem encountered processing the request"}

0 commit comments

Comments
 (0)