URI signing support for JWS to be passed as path parameter #4540
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alficles
suggested changes
Nov 12, 2018
| char termination_symbol; | ||
| size_t termination_ct; | ||
| if (strchr(reserved_string, paramName[strlen(paramName) - 1])) { | ||
| termination_symbol = paramName[strlen(paramName) - 1]; |
Contributor
There was a problem hiding this comment.
Repeated length checking of the paramName is inefficient. It can't change, so it should only be checked once.
| * '='. */ | ||
| char termination_symbol; | ||
| size_t termination_ct; | ||
| if (strchr(reserved_string, paramName[strlen(paramName) - 1])) { |
Contributor
There was a problem hiding this comment.
Possible invalid access if paramName is zero-length.
| return NULL; | ||
| const char *reserved_string = ":/?#[]@!$&\'()*+,;="; | ||
|
|
||
| /* If param name ends in reserved character this will be treated as the termination sybmol when parsing for package. Default is |
|
|
||
| if (!strncmp(paramName, key, (size_t)(key_end - key))) { | ||
| /* If the Parameter key is our target parameter name, attempt to import a JWS from the value. */ | ||
| if (!strncmp(paramName, key, (size_t)(key_end - key)) && (size_t)(key_end - key) == termination_ct) { |
Contributor
There was a problem hiding this comment.
These two tests can be swapped. The second test is much faster, so if it fails it will do so much more quickly.
dsouza93
force-pushed
the
uri_signing_updates
branch
from
9d2dcd4 to
aa5337e
Compare
Contributor
|
[approve ci] |
alficles
previously approved these changes
Dec 5, 2018
dsouza93
force-pushed
the
uri_signing_updates
branch
from
aa5337e to
aec3dca
Compare
Contributor
|
[approve ci] |
ezelkow1
pushed a commit
to ezelkow1/trafficserver
that referenced
this pull request
Apr 6, 2020
List of included PRs: - apache#6363 (partial pick) - apache#6420 - apache#6419 - apache#6354 - apache#6252 - apache#4513 - apache#4603 - apache#4750 (partial pick) - apache#4604 - apache#4540 - apache#4777 - apache#4862 - apache#4814 - apache#4802 - apache#4897 - apache#4988 - apache#5034 - apache#5140 - apache#5112 - apache#4895 - apache#5834 (partial pick) - apache#6061 - apache#6210 (partial pick) - apache#6265 (partial pick) - apache#6282 (partial pick) Updating uri_signing docs to reflect new RFC changes (cherry picked from commit 90e51a2) Add normalization the URI before cdniuc validation (cherry picked from commit b39b0f7) JWT Parser strips token from URI and places in buffer (cherry picked from commit 5f9d358) Use POSIX ERE for uri signing regex evaluation (cherry picked from commit be56b3a) Implement nbf claim in Uri Signing Plugin (cherry picked from commit d9dc0f4) Implement aud claim in Uri Signing Plugin The Aud claim is implemented as per the RFC version 16 that can be found here:https://tools.ietf.org/html/draft-ietf-cdni-uri-signing-16 As per the specification, the aud claim can be either a JSON array or a string. The aud claim is stored as raw json in the jwt class in this implementation. It is converted either to an array or a string at validation time. This commit also expands the unit tests quite a bit. Test configs can be provided in the unit_tests directory and parsed in the test framework. JWS validation is also testable now. This commit also fixes two memory leaks 1. Issuers were never being freed on configuration cleanup. 2. Token renewal allocates a tmp json_object without freeing. (cherry picked from commit 012d437) cdniuc is not a manditory claim With Internet Draft 16 for uri signing, the cdniuc claim is not manditory. It took the place of the manditory sub claim in draft 12, and the manditory nature of the sub claim was still in effect. This change allows for tokens to not contain the cdniuc claim and also renews the cdniuc and cdnistd claim on token renewal. (cherry picked from commit fa53771) add --with-jansson and --with-cjose options, document sample commands for building and configuring both locally (cherry picked from commit 0cce83c) Strip token from upstream if conifigured and dynamically allocate string buffers Adds a configuration option to strip uri signing tokens from both the cache key URL and the upstream URL. Additionally it was pointed out that some statically allocated buffers were too small in some of the string manipulating functions (normalize and strip token). These buffers are now dynamically allocated since the maximum buffer size is known for these. (cherry picked from commit 192dc83) Cherry-pick from commit 4cfd5a7 Add Example URI Signer Python Script Provide an example script to be used in conjunction with the uri signing plugin. This script is meant to serve as an example of how to get started with uri signing and could be useful in testing various configs. (cherry picked from commit 3632eb7) Cherry-pick from commit 9c1b88a Cherry-pick from commit a139fd1 Cherry-pick from commit c07474d Add simple autest and subsequent fixes (cherry picked from commit ea3aa04) Cherry-pick from commit 6d64842 URI Sig Null Check for Clang Warning (apache#6419) This commit adds a missing null check in the uri normalization function. This was caught by the clang analyzer. (cherry picked from commit 2de1c35) Syntax Error fixed in URI sig Plugin (apache#6420) (cherry picked from commit c154d40) Change gold files to be less restrictive since some of the headers include can be in a different order (apache#6410) (cherry picked from commit 4bdde5d) Add a dummy cachekey usage to handle the effective vs pristine url issue that exists in 8x where the first plugin gets a different url then subsequent ones. This is not needed on 9x+
zwoop
pushed a commit
that referenced
this pull request
Apr 7, 2020
List of included PRs: - #6363 (partial pick) - #6420 - #6419 - #6354 - #6252 - #4513 - #4603 - #4750 (partial pick) - #4604 - #4540 - #4777 - #4862 - #4814 - #4802 - #4897 - #4988 - #5034 - #5140 - #5112 - #4895 - #5834 (partial pick) - #6061 - #6210 (partial pick) - #6265 (partial pick) - #6282 (partial pick) Updating uri_signing docs to reflect new RFC changes (cherry picked from commit 90e51a2) Add normalization the URI before cdniuc validation (cherry picked from commit b39b0f7) JWT Parser strips token from URI and places in buffer (cherry picked from commit 5f9d358) Use POSIX ERE for uri signing regex evaluation (cherry picked from commit be56b3a) Implement nbf claim in Uri Signing Plugin (cherry picked from commit d9dc0f4) Implement aud claim in Uri Signing Plugin The Aud claim is implemented as per the RFC version 16 that can be found here:https://tools.ietf.org/html/draft-ietf-cdni-uri-signing-16 As per the specification, the aud claim can be either a JSON array or a string. The aud claim is stored as raw json in the jwt class in this implementation. It is converted either to an array or a string at validation time. This commit also expands the unit tests quite a bit. Test configs can be provided in the unit_tests directory and parsed in the test framework. JWS validation is also testable now. This commit also fixes two memory leaks 1. Issuers were never being freed on configuration cleanup. 2. Token renewal allocates a tmp json_object without freeing. (cherry picked from commit 012d437) cdniuc is not a manditory claim With Internet Draft 16 for uri signing, the cdniuc claim is not manditory. It took the place of the manditory sub claim in draft 12, and the manditory nature of the sub claim was still in effect. This change allows for tokens to not contain the cdniuc claim and also renews the cdniuc and cdnistd claim on token renewal. (cherry picked from commit fa53771) add --with-jansson and --with-cjose options, document sample commands for building and configuring both locally (cherry picked from commit 0cce83c) Strip token from upstream if conifigured and dynamically allocate string buffers Adds a configuration option to strip uri signing tokens from both the cache key URL and the upstream URL. Additionally it was pointed out that some statically allocated buffers were too small in some of the string manipulating functions (normalize and strip token). These buffers are now dynamically allocated since the maximum buffer size is known for these. (cherry picked from commit 192dc83) Cherry-pick from commit 4cfd5a7 Add Example URI Signer Python Script Provide an example script to be used in conjunction with the uri signing plugin. This script is meant to serve as an example of how to get started with uri signing and could be useful in testing various configs. (cherry picked from commit 3632eb7) Cherry-pick from commit 9c1b88a Cherry-pick from commit a139fd1 Cherry-pick from commit c07474d Add simple autest and subsequent fixes (cherry picked from commit ea3aa04) Cherry-pick from commit 6d64842 URI Sig Null Check for Clang Warning (#6419) This commit adds a missing null check in the uri normalization function. This was caught by the clang analyzer. (cherry picked from commit 2de1c35) Syntax Error fixed in URI sig Plugin (#6420) (cherry picked from commit c154d40) Change gold files to be less restrictive since some of the headers include can be in a different order (#6410) (cherry picked from commit 4bdde5d) Add a dummy cachekey usage to handle the effective vs pristine url issue that exists in 8x where the first plugin gets a different url then subsequent ones. This is not needed on 9x+
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the URI signing plugin only supported passing JWTs as URI query string parameters. This implements the latest URI Signing Internet draft's logic for parsing signed JWTs from URIs. The latest logic allows for JWTs to be parsed from either path parameters or query parameters.
This also fixes a small bug where keys that were substrings of the target parameter name where being accepted as valid JWT parameters.
Latest Internet Draft: https://tools.ietf.org/html/draft-ietf-cdni-uri-signing-16