Challenge Extension

[felix-schwarz]

The Challenge Extension to the tus protocol proposed here allows clients to authenticate requests targeting an upload resource following the resource's creation through creation or creation-with-upload.

This extension originates from the original Client-Tag PR and has been factored out here to allow separate discussion.

Compared with its original version, the proposal here includes various improvements:

• it makes changes to the computation of Upload-Challenge to extend it to more HTTP methods and block re-use for different purposes (i.e. to prevent the reuse of a captured Upload-Challenge from a HEAD request for a DELETE request)
• it allows the use of Upload-Challenge for requests referencing more than one upload resource (to support the Concat extension)
• it adds a section with examples

Looking forward to your feedback 🙂

[Acconut]

Can we explicitly state here again why we are sending a hashed version of the Upload-Secret in the requests instead of just the Upload-Secret? Maybe that was already discussed somewhere else but I would like to bring the reason up, so we think about which security goals we try to reach. Is the hash intended to avoid tampering by a MITM? Or is the hash intended so that the Upload-Challenge cannot be reused for other requests?

Also, do we know of a similar security mechanism in other applications or protocols? This would allow us to compare and see if we are missing anything.

[felix-schwarz]

Can we explicitly state here again why we are sending a hashed version of the Upload-Secret in the requests instead of just the Upload-Secret? Maybe that was already discussed somewhere else but I would like to bring the reason up, so we think about which security goals we try to reach. Is the hash intended to avoid tampering by a MITM? Or is the hash intended so that the Upload-Challenge cannot be reused for other requests?

Think of it as a shared secret:

• the request that initiates an upload needs no protection, for it initiates the upload in the first place. Therefore, it can also send the Upload-Secret along in plain text.
• all follow-up requests do need protection, so the Upload-Challenge is computed from the shared secret and the variables Upload-Offset, Content-Length and HTTP Method that should differ for every follow-up request. The idea here is that:
  • only someone in possession of the Upload-Secret will be able to compute a valid Upload-Challenge
  • the Upload-Challenge can't be used to recover the Upload-Secret, keeping anyone but the original uploader out

Also, do we know of a similar security mechanism in other applications or protocols? This would allow us to compare and see if we are missing anything.

PKCE is using a similar approach, but computes the Challenge as a simple SHA256 of the Secret. Which is ok, because it only needs the Challenge for a single request.

However, for multiple requests, a Challenge value should not simply be reusable. To protect against simply reusing a single hash value, I added Upload-Offset, Content-Length and HTTP Method to the computation.

[Acconut]

@felix-schwarz Thank you very much for the additional explanations!

[felix-schwarz]

@smatsson @Acconut Thanks for your comments and suggestions! Once we agree on the still discussed points, I'd update the PR accordingly.

[Acconut]

FYI, I have a few commits lined up for this PR and will push them in a few hours!

[Acconut]

I pushed a few changes to the PR:

• The server can now support multiple algorithms using the Tus-Challenge-Algorithm header
• The Upload-Secret header is now ASCII encoded, just as the Upload-Tag header, so clients can reuse the code for generating random strings
• The Upload-Challenge header is now Base64 encoded and has the same format as the Upload-Checksum header, so servers can reuse their parsing code
• The Upload-Challenge header does not include the Content-Length header in its hash anymore since this information is not always available
• Include a list of requests that currently would require an Upload-Challenge header to make it easier for implementations

Let me know if you have any feedback! Otherwise, I see this as ready to be merged into the 1.1 branch.

[smatsson]

@Acconut Looks good to me. Before merging I think it would be beneficial to implement a POC, similar to the one for Upload-Tag, so that we do not miss anything. I'll see if I can get one up and running this week.

[smatsson]

POC is now available at https://tustemp.azurewebsites.net/files/

EDIT: Source: https://github.com/tusdotnet/tusdotnet/tree/POC/Challenge

To calculate the checksums I used this bash command (also works on Windows):

openssl dgst -sha256 -binary challenge.txt | openssl enc -base64 | clip
# challenge.txt should contain the data to hash, e.g. DELETE#R290Y2hhISBVc2UgYW4gYWN0dWFsIGhpR290Y2hhISBVc2UgYW4gYWN0dWFsIGhp

Restrictions:

• One can use basic auth (any username/password is accepted) to lock an upload tag to a specific client
• Unauthenticated users must provide a Upload-Secret to be able to use upload tag. The implementation is very naive and will just accept the initial POST and then return 404 for the HEAD request if no Upload-Secret was set.
• Client-Tags can be resolved for any user, anonymous or not, regardless if they own the file or not, if the correct Upload-Challenge is provided. This is probably less than ideal and I will change this for the final release.
• File sizes are limited to 1 MB
• All files not written to in 30 minutes will be deleted (regardless of if it's completed or not)
• No check that Upload-Secret is not sent with other requests than POST.
• Downloads does not implement Upload-Challenge.

Feel free to try it out 😊

Issues I found while implementing (in no particual order):

• Examples are still using zero instead of # in a lot of places
• We should specify what casing of http method to use ("patch" vs "PATCH"). tusdotnet uses lower case internally which did not work very well with the examples given in the spec.
• Concatenation feels off. I have written down the issues below.

Issues with concatenation

1. The algorithm does not feel as spot on as for other requests:

   "sha256" + " " + SHA256("sha256 sXfhFCwyWMjnH1DMPkArsByfa4FEGtpf3LsAt6uDkTU=" + "sha256 JbVm0kH59MDQfGtzjJ3s9oBjzHp+Yqtv7O2/OzYTUqg=")

Do we need "sha256" for each of the partial file checksums? Since this checksum is calculated on each request we use the provided Upload-Challenge to determine the algorithm (i.e. we take "sha256" from the Upload-Challenge header). Would it be better to have something like this?

"sha256" + " " + SHA256("sXfhFCwyWMjnH1DMPkArsByfa4FEGtpf3LsAt6uDkTU=JbVm0kH59MDQfGtzjJ3s9oBjzHp+Yqtv7O2/OzYTUqg=")

2. What status code to return when Upload-Challenge is wrong when doing a POST with Upload-Concat: final /files/a;/files/b? It seems odd to return 404 as this would indicate that the actual creation endpoint does not exist. 400? 403? Not sure.

3. Is the client allowed to use Upload-Secret for when doing a POST with Upload-Concat: final /files/a;/files/b together with the Upload-Challenge? If yes, we should probably document it.

4. What if one of the partial files used does not contain an upload secret? E.g. file A uses an upload secret but file B does not. How would the final checksum be calculated? The POC just adds an empty string as checksum for file B but I think this needs to be documented in the spec.

[felix-schwarz]

@Acconut @smatsson Thanks for the updates and the POC implementation!

Issues I found while implementing (in no particual order):

• Examples are still using zero instead of # in a lot of places

Fixed that with the latest commit.

• We should specify what casing of http method to use ("patch" vs "PATCH"). tusdotnet uses lower case internally which did not work very well with the examples given in the spec.

Since the HTTP methods are specified in upper case in RFC 2616, which also states that The method is case-sensitive., I was always under the assumption that these are only used in upper-case.

But in fact get / HTTP/1.1 and GET / HTTP/1.1 both seem to work (get / http/1.1 returns an error, though). TIL! 🤯

The PR didn't assume any case, so if the server used the method as received from the client for computation, that should also work. That assumes, however, that proxies in-between would pass the method on unaltered - which can't be taken for granted. I therefore added a clarification.

Out of interest: is there any particular reason why you're using lower case HTTP methods in tusdotnet?

• Concatenation feels off. I have written down the issues below.

Issues with concatenation

1. The algorithm does not feel as spot on as for other requests:

"sha256" + " " + SHA256("sha256 sXfhFCwyWMjnH1DMPkArsByfa4FEGtpf3LsAt6uDkTU=" + "sha256 JbVm0kH59MDQfGtzjJ3s9oBjzHp+Yqtv7O2/OzYTUqg=")

Do we need "sha256" for each of the partial file checksums? Since this checksum is calculated on each request we use the provided Upload-Challenge to determine the algorithm (i.e. we take "sha256" from the Upload-Challenge header). Would it be better to have something like this?

"sha256" + " " + SHA256("sXfhFCwyWMjnH1DMPkArsByfa4FEGtpf3LsAt6uDkTU=JbVm0kH59MDQfGtzjJ3s9oBjzHp+Yqtv7O2/OzYTUqg=")

I agree the latter looks better.

However, I think it is more consistent to keep the sha256 portion even here. That way, the same piece of code that computes Upload-Challenge for all other requests for that upload can just be used, without special case handling.

2. What status code to return when Upload-Challenge is wrong when doing a POST with Upload-Concat: final /files/a;/files/b? It seems odd to return 404 as this would indicate that the actual creation endpoint does not exist. 400? 403? Not sure.

Semantically, I agree. The idea of the 404 is to not give any hints to the existence of a resource to anyone without the Upload-Secret. And going by that, I'd keep the 404.

3. Is the client allowed to use Upload-Secret for when doing a POST with Upload-Concat: final /files/a;/files/b together with the Upload-Challenge? If yes, we should probably document it.

Good catch! Since Concatenation also returns an upload resource, I feel it should be included as well. I just added it next to Create and Create-with-Upload in the latest commit.

4. What if one of the partial files used does not contain an upload secret? E.g. file A uses an upload secret but file B does not. How would the final checksum be calculated? The POC just adds an empty string as checksum for file B but I think this needs to be documented in the spec.

Good point! I've added a paragraph on this to the PR, but wrote that the path to the resource should be used instead of an empty string. Otherwise, the Upload-Challenge header of the request could be reused while inserting any number of additional uploads in-between the ones secured with Upload-Secret. Or it would be possible to replace those resources without Upload-Secret with any other resource:

For upload resources that were created without an `Upload-Secret`, the path to the upload resource
MUST be used as `Upload-Challenge` for the purpose of computing the `Upload-Challenge` for
requests that reference multiple upload resources - of which at least one MUST have been created with
an `Upload-Secret`.

[felix-schwarz]

@Acconut @smatsson From my side, the PR is ready to be merged at this point.

[smatsson]

Out of interest: is there any particular reason why you're using lower case HTTP methods in tusdotnet?

Mostly chance I think. I needed to support X-Http-Method-Override that could be in any casing I just needed to go with one casing and it ended up as lower case. ASP.NET supports any casing (pAtCH) and will interpret it correctly anyway.

However, I think it is more consistent to keep the sha256 portion even here. That way, the same piece of code that computes Upload-Challenge for all other requests for that upload can just be used, without special case handling.

I was actually thinking that we should simplify it even more as we still would need special casing for the cases where we do not have an upload secret for a file. I think it's a fair point to re-use code though.

Semantically, I agree. The idea of the 404 is to not give any hints to the existence of a resource to anyone without the Upload-Secret. And going by that, I'd keep the 404.

I understand why we want to return 404 but since I as a client would know that the /files endpoint does exist, it is still clear that I have the wrong Upload-Challenge. This would probably be OK if we updated the concatenation spec to include that the server should return 404 if one of the partial files does not exist (tusdotnet already does this).

Good catch! Since Concatenation also returns an upload resource, I feel it should be included as well. I just added it next to Create and Create-with-Upload in the latest commit.

Perfect! 👍

Good point! I've added a paragraph on this to the PR, but wrote that the path to the resource should be used instead of an empty string. Otherwise, the Upload-Challenge header of the request could be reused while inserting any number of additional uploads in-between the ones secured with Upload-Secret. Or it would be possible to replace those resources without Upload-Secret with any other resource.

Fair point 👍 Could we add an example of how to use this? The path is the same value included in the Upload-Concat header, e.g. /files/file1?

EDIT:

of which at least one MUST have been created with an Upload-Secret.

What should be returned if this is not fulfilled? Do we need to require this or could we just ignoring the Upload-Challenge header in that case? I mean technically it does nothing but I think that we should either define the error code (e.g. 400 Bad Request) or just remove the requirement that a file must have an Upload-Secret.

[felix-schwarz]

Semantically, I agree. The idea of the 404 is to not give any hints to the existence of a resource to anyone without the Upload-Secret. And going by that, I'd keep the 404.

I understand why we want to return 404 but since I as a client would know that the /files endpoint does exist, it is still clear that I have the wrong Upload-Challenge. This would probably be OK if we updated the concatenation spec to include that the server should return 404 if one of the partial files does not exist (tusdotnet already does this).

Good point. Right now I can't seem to find any direction in the spec on what to do when a non-existent file is referenced. And this would clarify that case.

I'm not sure where best to add that, however, below the Concatenation heading or below the Upload-Concat header section.

Good point! I've added a paragraph on this to the PR, but wrote that the path to the resource should be used instead of an empty string. Otherwise, the Upload-Challenge header of the request could be reused while inserting any number of additional uploads in-between the ones secured with Upload-Secret. Or it would be possible to replace those resources without Upload-Secret with any other resource.

Fair point 👍 Could we add an example of how to use this? The path is the same value included in the Upload-Concat header, e.g. /files/file1?

Yes, it'd be /files/file1. I've added an example.

of which at least one MUST have been created with an Upload-Secret.

What should be returned if this is not fulfilled? Do we need to require this or could we just ignoring the Upload-Challenge header in that case? I mean technically it does nothing but I think that we should either define the error code (e.g. 400 Bad Request) or just remove the requirement that a file must have an Upload-Secret.

My intention there was more to clarify that Upload-Challenge headers should not be used if none of the files actually was uploaded with an Upload-Secret, because it provides no security benefit.

I've changed that sentence now to (hopefully) make that clearer.

[smatsson]

@felix-schwarz Thanks for the updates! Looks good to me and I think this PR is ready for merge. @Acconut Any input on where to put the error handling examples for concatenation? Concretely to respond 404 when a partial file does not exist when creating a final file?

@Acconut @nigoroll Any more input on this extension? :)

[Acconut]

@felix-schwarz @smatsson Thank you very much for the updates in the last two weeks! I went over all changes and they all look good :+1:

Any input on where to put the error handling examples for concatenation? Concretely to respond 404 when a partial file does not exist when creating a final file?

A 404 sounds good to me. The response body can be used to describe the error in more detail (i.e. that one of the uploads to be concatenated wasn't found)

I think this PR is ready for merge.

I agree! Let's merge it! If one of you (or @nigoroll) has more input, we can open another PR but let's close this one for now :)