From 5eae9423cb67e1f3a67f46249bc71769f81d5e1a Mon Sep 17 00:00:00 2001 From: Ana Bobillo Date: Tue, 20 Oct 2026 04:14:56 +0200 Subject: [PATCH] Fix cupsOAuthGetUserId cache key, lifetime, and status checks. - oauth_make_path: hash the whole access token for "urn:token:" URIs instead of the text before the first '/' (and at most 1023 bytes), so different tokens no longer share a cached user ID. - oauth_save_value: add an optional expiration time that is written on a second line after the value; cupsOAuthSaveTokens now uses it for the access token instead of formatting the token in a 16k buffer. - cupsOAuthGetUserId: save an expiration time with the cached user ID (the access token's expiration time from cupsOAuthSaveTokens, otherwise five minutes) and get it again from the userinfo endpoint once it expires; only use a 200 OK response containing a "sub" claim; cache the userinfo claims as JSON since cupsJWTExportString no longer exports an unsigned JWT; close the userinfo connection. --- cups/oauth.c | 171 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 117 insertions(+), 54 deletions(-) diff --git a/cups/oauth.c b/cups/oauth.c index c66711d28..0856a5bb6 100644 --- a/cups/oauth.c +++ b/cups/oauth.c @@ -128,6 +128,9 @@ typedef enum _cups_otype_e // OAuth data type #define _CUPS_OAUTH_REDIRECT_PATHLEN 2 // Redirect URI request path length +#define _CUPS_OAUTH_USERID_TIMEOUT 300 + // Default user ID cache lifetime in seconds + #ifdef DEBUG static const char * const cups_otypes[_CUPS_OTYPE_MAX] = { // OAuth data types... @@ -169,7 +172,7 @@ static char *oauth_make_access_uri(const char *access_token); static char *oauth_make_path(char *buffer, size_t bufsize, const char *auth_uri, const char *secondary_uri, _cups_otype_t otype, bool use_sysconfig); static char *oauth_make_software_id(char *buffer, size_t bufsize); static bool oauth_metadata_contains(cups_json_t *metadata, const char *parameter, const char *value); -static void oauth_save_value(const char *auth_uri, const char *secondary_uri, _cups_otype_t otype, const char *value); +static void oauth_save_value(const char *auth_uri, const char *secondary_uri, _cups_otype_t otype, const char *value, time_t expires); static bool oauth_set_error(cups_json_t *json, size_t num_form, cups_option_t *form); @@ -184,11 +187,11 @@ cupsOAuthClearTokens( const char *auth_uri, // I - Authorization server URI const char *resource_uri) // I - Resource server URI { - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_ACCESS, /*value*/NULL); - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_CODE_VERIFIER, NULL); - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_USER_ID, /*value*/NULL); - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_NONCE, NULL); - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_REFRESH, /*value*/NULL); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_ACCESS, /*value*/NULL, /*expires*/0); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_CODE_VERIFIER, NULL, /*expires*/0); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_USER_ID, /*value*/NULL, /*expires*/0); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_NONCE, NULL, /*expires*/0); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_REFRESH, /*value*/NULL, /*expires*/0); } @@ -440,7 +443,7 @@ cupsOAuthGetAuthorizationCode( resourcelen = strlen(resource); // Point redirection to the local port... - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_REDIRECT_URI, redirect_uri); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_REDIRECT_URI, redirect_uri, /*expires*/0); // Make state and code verification strings... if (oauth_metadata_contains(metadata, "code_challenge_methods_supported", "S256")) @@ -776,8 +779,8 @@ cupsOAuthGetClientId( if ((client_id = strdup(value)) != NULL) { // Save client_id and optional client_secret... - oauth_save_value(auth_uri, redirect_uri, _CUPS_OTYPE_CLIENT_ID, value); - oauth_save_value(auth_uri, redirect_uri, _CUPS_OTYPE_CLIENT_SECRET, cupsJSONGetString(cupsJSONFind(response, "client_secret"))); + oauth_save_value(auth_uri, redirect_uri, _CUPS_OTYPE_CLIENT_ID, value, /*expires*/0); + oauth_save_value(auth_uri, redirect_uri, _CUPS_OTYPE_CLIENT_SECRET, cupsJSONGetString(cupsJSONFind(response, "client_secret")), /*expires*/0); } } @@ -1015,7 +1018,7 @@ cupsOAuthGetJWKS(const char *auth_uri, // I - Authorization server URI char *s = cupsJSONExportString(jwks); // JSON string - oauth_save_value(auth_uri, /*secondary_uri*/NULL, _CUPS_OTYPE_JWKS, s); + oauth_save_value(auth_uri, /*secondary_uri*/NULL, _CUPS_OTYPE_JWKS, s, /*expires*/0); free(s); } @@ -1458,8 +1461,8 @@ cupsOAuthGetTokens( done: // Clear code_verifier and nonce values... - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_CODE_VERIFIER, /*value*/NULL); - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_NONCE, /*value*/NULL); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_CODE_VERIFIER, /*value*/NULL, /*expires*/0); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_NONCE, /*value*/NULL, /*expires*/0); cupsJSONDelete(response); cupsJWTDelete(jwt); @@ -1474,8 +1477,11 @@ cupsOAuthGetTokens( // 'cupsOAuthGetUserId()' - Get the user ID token associated with the given access token. // // This function retrieves the user ID token associated with a given access -// token. The user ID information is cached until the token expires to minimize -// the overhead of communicating with the Authorization Server. +// token. The user ID information is cached until the access token expires, as +// saved by @link cupsOAuthSaveTokens@, or for five minutes after it is +// retrieved from the Authorization Server's "userinfo_endpoint", to minimize +// the overhead of communicating with the Authorization Server. Only user +// information containing a subject ("sub") is returned or cached. // cups_jwt_t * // O - Identification information or `NULL` if none @@ -1485,9 +1491,11 @@ cupsOAuthGetUserId( const char *access_token) // I - Access (Bearer) token { char *access_uri, // Access token URI - *user_id_value; // String value of user ID + *user_id_value, // String value of user ID + *expptr; // Pointer to expiration time cups_jwt_t *user_id = NULL; // User ID JWT const char *userinfo_ep; // userinfo_endpoint value + http_t *http = NULL; // Connection to endpoint // Range check input... @@ -1499,10 +1507,49 @@ cupsOAuthGetUserId( return (NULL); // See if we have this token cached already... - if ((user_id_value = oauth_load_value(auth_uri, access_uri, _CUPS_OTYPE_USER_ID, /*try_sysconfig*/false)) == NULL && (userinfo_ep = cupsJSONGetString(cupsJSONFind(metadata, "userinfo_endpoint"))) != NULL) + if ((user_id_value = oauth_load_value(auth_uri, access_uri, _CUPS_OTYPE_USER_ID, /*try_sysconfig*/false)) != NULL) + { + // Yes, use it if it hasn't expired... + if ((expptr = strchr(user_id_value, '\n')) != NULL && strtol(expptr + 1, NULL, 10) > time(NULL)) + { + *expptr = '\0'; + + if (*user_id_value == '{') + { + // Claims from the userinfo endpoint... + cups_json_t *claims = cupsJSONImportString(user_id_value); + // Cached claims + + if (claims && (user_id = cupsJWTNew(/*type*/NULL, claims)) == NULL) + cupsJSONDelete(claims); + } + else + { + // ID token from cupsOAuthSaveTokens... + user_id = cupsJWTImportString(user_id_value, CUPS_JWS_FORMAT_COMPACT); + } + + if (user_id && !cupsJWTGetClaimString(user_id, CUPS_JWT_SUB)) + { + // No subject... + cupsJWTDelete(user_id); + user_id = NULL; + } + } + + free(user_id_value); + user_id_value = NULL; + + if (user_id) + goto done; + + // Remove the expired or unusable user ID... + oauth_save_value(auth_uri, access_uri, _CUPS_OTYPE_USER_ID, /*value*/NULL, /*expires*/0); + } + + if ((userinfo_ep = cupsJSONGetString(cupsJSONFind(metadata, "userinfo_endpoint"))) != NULL) { // No, try to get the information from the Authorization Server... - http_t *http; // Connection to endpoint char host[256], // Endpoint hostname resource[1024]; // Endpoint resource int port; // Endpoint port @@ -1513,7 +1560,7 @@ cupsOAuthGetUserId( // Connect to the endpoint... if ((http = httpConnectURI(userinfo_ep, host, sizeof(host), &port, resource, sizeof(resource), /*blocking*/true, /*msec*/30000, /*cancel*/NULL, /*require_ca*/true)) == NULL) - return (NULL); + goto done; // Send a GET request with the access token... httpSetAuthString(http, "Bearer", access_token); @@ -1539,25 +1586,36 @@ cupsOAuthGetUserId( user_id_claims = cupsJSONImportString(user_id_value); // Verify that the access token hasn't expired... - if (cupsJSONFind(user_id_claims, "error")) + if (status != HTTP_STATUS_OK || cupsJSONFind(user_id_claims, "error")) { - // Expired or bad access token, get rid of this one... + // Expired or bad access token... const char *error_desc = cupsJSONGetString(cupsJSONFind(user_id_claims, "error_description")); // Description of error - DEBUG_printf("1cupsOAuthGetUserId: Error from userinfo - %s, %s.", cupsJSONGetString(cupsJSONFind(user_id_claims, "error")), error_desc); + DEBUG_printf("1cupsOAuthGetUserId: Error from userinfo - %d, %s, %s.", status, cupsJSONGetString(cupsJSONFind(user_id_claims, "error")), error_desc); + + if (!error_desc && status != HTTP_STATUS_OK) + error_desc = httpStatusString(status); + _cupsSetError(IPP_STATUS_ERROR_INTERNAL, error_desc, false); cupsJSONDelete(user_id_claims); - oauth_save_value(auth_uri, access_uri, _CUPS_OTYPE_USER_ID, /*value*/NULL); + } + else if (!cupsJSONGetString(cupsJSONFind(user_id_claims, "sub"))) + { + // No subject in the user information... + _cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Missing subject in user information."), true); + + cupsJSONDelete(user_id_claims); } else if ((user_id = cupsJWTNew(/*type*/NULL, user_id_claims)) != NULL) { - // Created a new JWT with the JSON user information, save it for future use... + // Created a new JWT with the JSON user information, save the claims for + // future use... free(user_id_value); - user_id_value = cupsJWTExportString(user_id, CUPS_JWS_FORMAT_COMPACT); - oauth_save_value(auth_uri, access_uri, _CUPS_OTYPE_USER_ID, user_id_value); + user_id_value = cupsJSONExportString(user_id_claims); + oauth_save_value(auth_uri, access_uri, _CUPS_OTYPE_USER_ID, user_id_value, time(NULL) + _CUPS_OAUTH_USERID_TIMEOUT); } else { @@ -1565,15 +1623,11 @@ cupsOAuthGetUserId( cupsJSONDelete(user_id_claims); } } - else if (user_id_value) - { - // Convert the string to a JWT... - user_id = cupsJWTImportString(user_id_value, CUPS_JWS_FORMAT_COMPACT); - } done: // Free strings and return... + httpClose(http); free(access_uri); free(user_id_value); @@ -1667,14 +1721,14 @@ cupsOAuthMakeAuthorizationURL( num_vars = cupsAddOption("code_challenge", code_challenge, num_vars, &vars); num_vars = cupsAddOption("code_challenge_method", "S256", num_vars, &vars); - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_CODE_VERIFIER, code_verifier); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_CODE_VERIFIER, code_verifier, /*expires*/0); } if (nonce && oauth_metadata_contains(metadata, "scopes_supported", "openid")) { num_vars = cupsAddOption("nonce", nonce, num_vars, &vars); - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_NONCE, nonce); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_NONCE, nonce, /*expires*/0); } if (resource_uri) @@ -1761,8 +1815,8 @@ cupsOAuthSaveClientData( const char *client_id, // I - client_id or `NULL` to delete const char *client_secret) // I - client_secret value or `NULL` for none { - oauth_save_value(auth_uri, redirect_uri, _CUPS_OTYPE_CLIENT_ID, client_id); - oauth_save_value(auth_uri, redirect_uri, _CUPS_OTYPE_CLIENT_SECRET, client_secret); + oauth_save_value(auth_uri, redirect_uri, _CUPS_OTYPE_CLIENT_ID, client_id, /*expires*/0); + oauth_save_value(auth_uri, redirect_uri, _CUPS_OTYPE_CLIENT_SECRET, client_secret, /*expires*/0); } @@ -1784,9 +1838,6 @@ cupsOAuthSaveTokens( const char *user_id, // I - User ID or `NULL` to delete const char *refresh_token) // I - Refresh token or `NULL` to delete { - char temp[16384]; // Temporary string - - DEBUG_printf("cupsOAuthSaveTokens(auth_uri=\"%s\", resource_uri=\"%s\", access_token=\"%s\", access_expires=%ld, user_id=\"%s\", refresh_token=\"%s\")", auth_uri, resource_uri, access_token, (long)access_expires, user_id, refresh_token); // Access token... @@ -1795,26 +1846,27 @@ cupsOAuthSaveTokens( // Save access token... char *access_uri; // Access token URI - snprintf(temp, sizeof(temp), "%s\n%ld\n", access_token, (long)access_expires); - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_ACCESS, temp); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_ACCESS, access_token, access_expires); if ((access_uri = oauth_make_access_uri(access_token)) != NULL) { - oauth_save_value(auth_uri, access_uri, _CUPS_OTYPE_USER_ID, user_id); + // Save the user ID for the access token until the access token expires, + // or for five minutes if the expiration time is not known... + oauth_save_value(auth_uri, access_uri, _CUPS_OTYPE_USER_ID, user_id, access_expires > 0 ? access_expires : time(NULL) + _CUPS_OAUTH_USERID_TIMEOUT); free(access_uri); } } else { // Remove access token - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_ACCESS, NULL); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_ACCESS, NULL, /*expires*/0); } // User ID... - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_USER_ID, user_id); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_USER_ID, user_id, /*expires*/0); // Refresh token... - oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_REFRESH, refresh_token); + oauth_save_value(auth_uri, resource_uri, _CUPS_OTYPE_REFRESH, refresh_token, /*expires*/0); } @@ -2231,6 +2283,9 @@ oauth_make_path( if (secondary_uri) { + const char *hashdata = secondary_temp; + // Data to hash + if (!strncmp(secondary_uri, "http://", 7)) { // HTTP URI @@ -2251,15 +2306,12 @@ oauth_make_path( } else { - // IPPS URI - cupsCopyString(secondary_temp, secondary_uri + 7, sizeof(secondary_temp)); - if ((ptr = strchr(secondary_temp, '/')) != NULL) - *ptr = '\0'; // Strip resource path - if (!strchr(secondary_temp, ':')) // Add :631 if no port is present - cupsConcatString(secondary_temp, ":631", sizeof(secondary_temp)); + // Access token URN - hash the whole token since it can contain '/' and + // be longer than secondary_temp... + hashdata = secondary_uri; } - cupsHashData("sha2-256", secondary_temp, strlen(secondary_temp), secondary_hash, sizeof(secondary_hash)); + cupsHashData("sha2-256", hashdata, strlen(hashdata), secondary_hash, sizeof(secondary_hash)); cupsHashString(secondary_hash, sizeof(secondary_hash), secondary_temp, sizeof(secondary_temp)); } else @@ -2375,15 +2427,19 @@ oauth_metadata_contains( // // 'oauth_save_value()' - Save a value string to the OAuth store. // +// If "expires" is non-zero, it is written on a second line after the value. +// static void oauth_save_value( const char *auth_uri, // I - Authorization Server URI const char *secondary_uri, // I - Resource or redirect URI _cups_otype_t otype, // I - Type (_CUPS_OTYPE_xxx) - const char *value) // I - Value string or `NULL` to remove + const char *value, // I - Value string or `NULL` to remove + time_t expires) // I - Expiration time or `0` for none { - char filename[1024]; // Filename + char filename[1024], // Filename + expstr[32]; // Expiration time string int fd; // File descriptor static const int modes[_CUPS_OTYPE_MAX] = { // File permissions for the various types @@ -2400,7 +2456,7 @@ oauth_save_value( }; - DEBUG_printf("3oauth_save_value(auth_uri=\"%s\", secondary_uri=\"%s\", otype=%s, value=\"%s\")", auth_uri, secondary_uri, cups_otypes[otype], value); + DEBUG_printf("3oauth_save_value(auth_uri=\"%s\", secondary_uri=\"%s\", otype=%s, value=\"%s\", expires=%ld)", auth_uri, secondary_uri, cups_otypes[otype], value, (long)expires); // Try making the filename... if (!oauth_make_path(filename, sizeof(filename), auth_uri, secondary_uri, otype, /*use_sysconfig*/false)) @@ -2411,8 +2467,15 @@ oauth_save_value( // Create the file... if ((fd = open(filename, O_CREAT | O_WRONLY | O_TRUNC | O_NOFOLLOW, modes[otype])) >= 0) { - // Write the value and close... + // Write the value, any expiration time, and close... write(fd, value, strlen(value)); + + if (expires) + { + snprintf(expstr, sizeof(expstr), "\n%ld\n", (long)expires); + write(fd, expstr, strlen(expstr)); + } + close(fd); } }