Project

General

Profile

Bug #2896

mod_secdownload behaves unexpectedly for timestamps in the future

Added by jupi about 1 month ago. Updated about 1 month ago.

Status:
Invalid
Priority:
Normal
Assignee:
-
Category:
mod_secdownload
Target version:
Start date:
2018-07-15
Due date:
% Done:

0%

Estimated time:
Missing in 1.5.x:

Description

Hallo,
during the implementation of a PHP application which uses mod_secdownload for temporary download links, I stumbled upon an unexpected behaviour of mod_secdownload when timestamps are used which reference to some time in the future.
My application makes use of such timestamps (e.g. timestamp is set to 2h from now). This led to mod_secdownload responding always with "410 - Gone", although the time specified in the timestamp hadn't been reached yet.
Looking in the source I found why that is:

(srv->cur_ts < ts && (unsigned int) (ts - srv->cur_ts) > p->conf.timeout)

Causes mod_secdownload to see any timestamp as timed out which is longer in the servers future than the currently configured timeout. So in fact we don't have a "valid until" anymore, but a "valid from". I don't think that this is the intended behaviour here or that anyone would have use for a "valid from" function for temporary download links.
If it is indeed expected behaviour, it should at least be documented (which it isn't up to now). If it's not intended, the fix would simply be to leave out the given condition and just keep the first one, which is used if the given timestamp is in the server's past. Because if the given timestamp is greater than the server's current time, there is no way the download link could have timed out...

History

#1

Updated by stbuehler about 1 month ago

The way the condition is written should make it clear this is on purpose. Your application is supposed to calculate the checksum for the current timestamp, not for something in the future.

If we'd want to only let the url get invalid at some point, we could drop the timeout parameter and have you sign the timestamp the url gets invalid instead of the current time.

#2

Updated by jupi about 1 month ago

Ok. While I personally still don't see the use in having a temporary download link which becomes valid only some time in the future, I will adapt and supply lighttpd with the timestamp of the time of link generation. But if this behaviour is intended, I'd suggest to describe it on mod_secdownloads's Wiki-Page, so that any developer is aware of this special behaviour concerning future timestamps. I don't know lighttpd's project policy, but if it's ok, I could make this edit to the Wiki page.
But may I suggest an additional feature of mod_secdownload then, where setting a parameter like secdownload.timeout-by-app allows the link generating application to decide when the supplied link should time out? I think there might be other use cases than my own requiring to have different timeouts for different files, and in this cases it would be easiest to supply the timestamp after which the link is invalidated like I tried to do.

#3

Updated by stbuehler about 1 month ago

If we don't want users to edit certain wiki pages we usually lock them :)

I'm certain we are open to patches which allow to configure a separate value for the range a timestamp can extend into the future.

If you then set timeout to 0, and the "allow-future-timestamp-range" to some high value, you effectively leave it up to the signer of the timestamp when the URL should expire.

#4

Updated by gstrauss about 1 month ago

  • Status changed from New to Invalid

I don't think that this is the intended behaviour here

The code is explicit. As stbuehler notes, the behavior is obviously intended, or else the code would not be so explicit with that condition.

This code has been present in mod_secdownload since the first version of lighttpd 1.4.0 when the module was named mod_secure_download.c. (I did not look any further back in the history.)

Docs_ModSecDownload says

mod_secdownload removes this problem by introducing a way to
authenticate a URL for a specified time. The application has
to generate a token and a timestamp which are checked by the
webserver before it allows the file to be downloaded by the
webserver.

lighttpd mod_secdownload could have chosen to reject any timestamps in the future. This is different from your suggestion, which is to assume the timestamp is valid because it is in the future.

Instead, to account for the possibility of clock skew between machines, lighttpd mod_secdownload accepts the URL as valid if the encoded time is in the future, but within the configured time period (the timeout).

.

If the URL should be valid from now until some specific time in the future ("valid until"), then you should configure your timeout as appropriate in lighttpd and you should use an appropriate timestamp in the past when you generate the download link.

If you generate a timestamp in the future, then you should consider waiting to share the generated link until the link is valid.

.

As stbuehler said above, proposed patches (with code and with explanations of use cases) will be considered. However, please be aware that producing a patch does not guarantee that it will be accepted.

#5

Updated by gstrauss about 1 month ago

BTW, the examples in Docs_ModSecDownload all use the current time in the link generation, and mod_secdownload.c contains the comment:

 *   timestamp := [a-f0-9]{8}       # timestamp when the checksum was calculated
 *                                  # to prevent access after timeout (active requests
 *                                  # will finish successfully even after the timeout)

While the wiki page could be more explicit, I have withdrawn this ticket since the current behavior is most definitely intended.

Also available in: Atom