Work on #261: Make comprehensive input san possible.

Also fix bug when emitting messages to log wrt fkey problems.

This required some re-shuffling of the tcms mainloop.

lib/TCMS.pm

@@ -23,6 +23,7 @@ use Time::HiRes qw{gettimeofday tv_interval};
 use HTTP::Parser::XS qw{HEADERS_AS_HASHREF};
 use List::Util;
 use URI();
+use Ref::Util qw{is_coderef is_hashref is_arrayref}; 
 
 #Grab our custom routes
 use FindBin::libs;
@@ -109,6 +110,14 @@ sub _app {
     # sigdie can now "do the right thing"
     $cur_query = { route => $path, fullpath => $path, method => $method };
 
+    # Set the IP of the request so we can fail2ban
+    $Trog::Log::ip = $env->{HTTP_X_FORWARDED_FOR} || $env->{REMOTE_ADDR};
+
+    # Set the referer & ua to go into DB logs, but not logs in general.
+    # The referer/ua largely has no importance beyond being a proto bug report for log messages.
+    $Trog::Log::DBI::referer = $env->{HTTP_REFERER};
+    $Trog::Log::DBI::ua      = $env->{HTTP_UA};
+
     # Check eTags.  If we don't know about it, just assume it's good and lazily fill the cache
     # XXX yes, this allows cache poisoning...but only for logged in users!
     if ( $env->{HTTP_IF_NONE_MATCH} ) {
@@ -123,17 +132,19 @@ sub _app {
     #TODO: Actually do something with the acceptable output formats in the renderer
     my $accept = $env->{HTTP_ACCEPT};
 
+    # Figure out if we want compression or not
+    my $alist = $env->{HTTP_ACCEPT_ENCODING} || '';
+    $alist =~ s/\s//g;
+    my @accept_encodings;
+    @accept_encodings = split( /,/, $alist );
+    my $deflate = grep { 'gzip' eq $_ } @accept_encodings;
+
     # NOTE These two parameters are entirely academic, as we don't use ad tracking cookies, but the UTM parameters.
     # UTMs are actually fully sufficient to get you what you want -- e.g. keywords, audience groups, a/b testing, etc.
     # and you need to put up cookie consent banners if you bother using tracking cookies, which are horrific UX.
     #my $no_track = $env->{HTTP_DNT};
     #my $no_sell_info = $env->{HTTP_SEC_GPC};
 
-    # Set the referer & ua to go into DB logs, but not logs in general.
-    # The referer/ua largely has no importance beyond being a proto bug report for log messages.
-    $Trog::Log::DBI::referer = $env->{HTTP_REFERER};
-    $Trog::Log::DBI::ua      = $env->{HTTP_UA};
-
     # We generally prefer this to be handled at the reverse proxy level.
     #my $prefer_ssl = $env->{HTTP_UPGRADE_INSECURE_REQUESTS};
 
@@ -162,9 +173,6 @@ sub _app {
         @$query{ keys( %{ $body->upload } ) } = values( %{ $body->upload } );
     }
 
-    # Grab the list of ACLs we want to add to a post, if any.
-    $query->{acls} = [ $query->{acls} ] if ( $query->{acls} && ref $query->{acls} ne 'ARRAY' );
-
     # It's mod_rewrite!
     $path = '/index' if $path eq '/';
 
@@ -174,66 +182,39 @@ sub _app {
     # Translate alias paths into their actual path
     $path = $aliases{$path} if exists $aliases{$path};
 
-    # Figure out if we want compression or not
-    my $alist = $env->{HTTP_ACCEPT_ENCODING} || '';
-    $alist =~ s/\s//g;
-    my @accept_encodings;
-    @accept_encodings = split( /,/, $alist );
-    my $deflate = grep { 'gzip' eq $_ } @accept_encodings;
-
     # Collapse multiple slashes in the path
     $path =~ s/[\/]+/\//g;
 
-    # Let's open up our default route before we bother to see if users even exist
-    return $routes{default}{callback}->($query) unless -f "config/setup";
-
-    my $cookies = {};
-    if ( $env->{HTTP_COOKIE} ) {
-        $cookies = CGI::Cookie->parse( $env->{HTTP_COOKIE} );
-    }
-
-    # Set the IP of the request so we can fail2ban
-    $Trog::Log::ip = $env->{HTTP_X_FORWARDED_FOR} || $env->{REMOTE_ADDR};
+    #Handle regex/capture routes
+    if ( !exists $routes{$path} ) {
+        my @captures;
 
-    my $active_user = '';
-    $Trog::Log::user = 'nobody';
-    if ( exists $cookies->{tcmslogin} ) {
-        $active_user     = Trog::Auth::session2user( $cookies->{tcmslogin}->value );
-        $Trog::Log::user = $active_user if $active_user;
+        # TODO can optimize by having separate hashes for capture/non-capture routes
+        foreach my $pattern ( keys(%routes) ) {
+            @captures = $path =~ m/^$pattern$/;
+            if (@captures) {
+                $path = $pattern;
+                foreach my $field ( @{ $routes{$path}{captures} } ) {
+                    $routes{$path}{data} //= {};
+                    $routes{$path}{data}{$field} = shift @captures;
+                }
+                last;
+            }
+        }
     }
-    $query->{user_acls} = [];
-    $query->{user_acls} = Trog::Auth::acls4user($active_user) // [] if $active_user;
 
-    # Filter out passed ACLs which are naughty
-    my $is_admin = grep { $_ eq 'admin' } @{ $query->{user_acls} };
-    @{ $query->{acls} } = grep { $_ ne 'admin' } @{ $query->{acls} } unless $is_admin;
-
-    # Ensure any short-circuit routes can log the request
-    $query->{method} = $method;
-    $query->{route}  = $path;
-
-    # Set the urchin parameters if necessary.
-    %$Trog::Log::DBI::urchin = map { $_ => $query->{$_} } qw{utm_source utm_medium utm_campaign utm_term utm_content};
+    # Ensure any short-circuit routes can log the request, and return the server-timing headers properly
+    $query->{method}   = $method;
+    $query->{route}    = $path;
+    $query->{fullpath} = $fullpath;
+    $query->{start}    = $start;
 
-    # Disallow any paths that are naughty ( starman auto-removes .. up-traversal)
-    if ( index( $path, '/templates' ) == 0 || index( $path, '/statics' ) == 0 || $path =~ m/.*(\.psgi|\.pm)$/i ) {
-        return _forbidden($query);
-    }
+    # Handle HTTP range/streaming requests
+    my $range = $env->{HTTP_RANGE} || "bytes=0-" if $env->{HTTP_RANGE} || $env->{HTTP_IF_RANGE};
 
     my $streaming = $env->{'psgi.streaming'};
     $query->{streaming} = $streaming;
 
-    # If we have a static render, just use it instead (These will ALWAYS be correct, data saves invalidate this)
-    # TODO: make this key on admin INSTEAD of active user when we add non-admin users.
-    $query->{start} = $start;
-    if ( !$active_user && !$has_query ) {
-        return _static( $fullpath, "$path.z", $start, $streaming ) if -f "www/statics/$path.z" && $deflate;
-        return _static( $fullpath, $path,     $start, $streaming ) if -f "www/statics/$path";
-    }
-
-    # Handle HTTP range/streaming requests
-    my $range = $env->{HTTP_RANGE} || "bytes=0-" if $env->{HTTP_RANGE} || $env->{HTTP_IF_RANGE};
-
     my @ranges;
     if ($range) {
         $range =~ s/bytes=//g;
@@ -248,35 +229,84 @@ sub _app {
         );
     }
 
+	# If it's a file, just serve it
     return Trog::FileHandler::serve( $fullpath, "www/$path",  $start, $streaming, \@ranges, $last_fetch, $deflate ) if -f "www/$path";
-    return Trog::FileHandler::serve( $fullpath, "totp/$path", $start, $streaming, \@ranges, $last_fetch, $deflate ) if -f "totp/$path" && $active_user;
 
-    #Handle regex/capture routes
-    if ( !exists $routes{$path} ) {
-        my @captures;
+	# Figure out if we have a logged in user, so we can serve them user-specific files
+    my $cookies = {};
+    if ( $env->{HTTP_COOKIE} ) {
+        $cookies = CGI::Cookie->parse( $env->{HTTP_COOKIE} );
+    }
 
-        # TODO can optimize by having separate hashes for capture/non-capture routes
-        foreach my $pattern ( keys(%routes) ) {
-            @captures = $path =~ m/^$pattern$/;
-            if (@captures) {
-                $path = $pattern;
-                foreach my $field ( @{ $routes{$path}{captures} } ) {
-                    $routes{$path}{data} //= {};
-                    $routes{$path}{data}{$field} = shift @captures;
-                }
-                last;
-            }
-        }
+    my $active_user = '';
+    $Trog::Log::user = 'nobody';
+    if ( exists $cookies->{tcmslogin} ) {
+        $active_user     = Trog::Auth::session2user( $cookies->{tcmslogin}->value );
+        $Trog::Log::user = $active_user if $active_user;
     }
 
-    $query->{fullpath} = $fullpath;
-    $query->{deflate}  = $deflate;
-    $query->{user}     = $active_user;
+    return Trog::FileHandler::serve( $fullpath, "totp/$path", $start, $streaming, \@ranges, $last_fetch, $deflate ) if -f "totp/$path" && $active_user;
 
+	# Now that we have firmed up the actual routing, let's validate.
     return _forbidden($query) if exists $routes{$path}{auth} && !$active_user;
     return _notfound($query) unless exists $routes{$path} && ref $routes{$path} eq 'HASH' && keys( %{ $routes{$path} } );
     return _badrequest($query) unless grep { $env->{REQUEST_METHOD} eq $_ } ( $routes{$path}{method} || '', 'HEAD' );
 
+    # Disallow any paths that are naughty ( starman auto-removes .. up-traversal)
+    if ( index( $path, '/templates' ) == 0 || index( $path, '/statics' ) == 0 || $path =~ m/.*(\.psgi|\.pm)$/i ) {
+        return _forbidden($query);
+    }
+
+    # Set the urchin parameters if necessary.
+    %$Trog::Log::DBI::urchin = map { $_ => delete $query->{$_} } qw{utm_source utm_medium utm_campaign utm_term utm_content};
+
+	# Now that we've parsed the query and know where we want to go, we should murder everything the route does not explicitly want, and validate what it does
+	my $parameters = $routes{$path}{parameters};
+	if ($parameters) {
+		die "invalid route definition for $path: bad parameters" unless is_hashref($parameters);
+		my @known_params = keys(%$parameters);
+		for my $param (@known_params) {
+			die "Invalid route definition for $path: parameter $param must correspond to a validation CODEREF." unless is_coderef($parameters->{$param});
+			# A missing parameter is not necessarily a problem.
+			next unless $query->{$param};
+			# But if we have it, and it's bad, nack it, so that scanners get fail2banned.
+			DEBUG("Rejected $fullpath for bad query param $param");
+			return _badrequest($query) unless $parameters->{$param}->($query->{$param});
+		}
+
+		# Smack down passing of unnecessary fields
+		foreach my $field (keys(%$query)) {
+			next if List::Util::any { $field eq $_ } @known_params;
+			next if List::Util::any { $field eq $_ } qw{start route streaming method fullpath};
+			DEBUG("Rejected $fullpath for query param $field");
+			return _badrequest($query);
+		}
+	}
+
+    # Let's open up our default route before we bother thinking about routing any harder
+    return $routes{default}{callback}->($query) unless -f "config/setup";
+
+    $query->{user_acls} = [];
+    $query->{user_acls} = Trog::Auth::acls4user($active_user) // [] if $active_user;
+
+    # Grab the list of ACLs we want to add to a post, if any.
+    $query->{acls} = [ $query->{acls} ] if ( $query->{acls} && ref $query->{acls} ne 'ARRAY' );
+
+    # Filter out passed ACLs which are naughty
+    my $is_admin = grep { $_ eq 'admin' } @{ $query->{user_acls} };
+    @{ $query->{acls} } = grep { $_ ne 'admin' } @{ $query->{acls} } unless $is_admin;
+
+    # If we have a static render, just use it instead (These will ALWAYS be correct, data saves invalidate this)
+    # TODO: make this key on admin INSTEAD of active user when we add non-admin users.
+    if ( !$active_user && !$has_query ) {
+        return _static( $fullpath, "$path.z", $start, $streaming ) if -f "www/statics/$path.z" && $deflate;
+        return _static( $fullpath, $path,     $start, $streaming ) if -f "www/statics/$path";
+    }
+
+    $query->{deflate}  = $deflate;
+    $query->{user}     = $active_user;
+
+	# Set the 'data' in the query that the route specifically overrides
     @{$query}{ keys( %{ $routes{$path}{'data'} } ) } = values( %{ $routes{$path}{'data'} } ) if ref $routes{$path}{'data'} eq 'HASH' && %{ $routes{$path}{'data'} };
 
     #Set various things we don't want overridden

lib/Trog/Log/DBI.pm

@@ -72,6 +72,7 @@ sub log_message {
         $self->{sth2}->bind_param_array(2, $buffer{$uuid});
         $self->{sth2}->execute_array({});
         delete $buffer{$uuid};
+
     }
 
     # Record urchin data if there is any.

lib/Trog/Routes/HTML.pm

@@ -1132,7 +1132,8 @@ sub posts ( $query, $direct = 0 ) {
     $query->{title} ||= @$tags && $query->{domain} ? "$query->{domain} : @$tags" : undef;
 
     #Handle paginator vars
-    my $limit       = int( $query->{limit} ) || 25;
+	$query->{limit} ||= 25;
+    my $limit       = int( $query->{limit} );
     my $now_year    = ( localtime(time) )[5] + 1900;
     my $oldest_year = $now_year - 20;                  #XXX actually find oldest post year
 
@@ -1259,11 +1260,14 @@ sub _post_helper ( $query, $tags, $acls ) {
     state $data;
     $data //= Trog::Data->new($conf);
 
+	$query->{page} ||= 1;
+	$query->{limit} ||= 25;
+
     return $data->get(
         older        => $query->{older},
         newer        => $query->{newer},
-        page         => int( $query->{page} ) || 1,
-        limit        => int( $query->{limit} ) || 25,
+        page         => int( $query->{page} ),
+        limit        => int( $query->{limit} ),
         tags         => $tags,
         exclude_tags => $query->{exclude_tags},
         acls         => $acls,

lib/Trog/Routes/JSON.pm

@@ -9,6 +9,8 @@ use feature qw{signatures state};
 use Clone qw{clone};
 use JSON::MaybeXS();
 
+use Scalar::Util();
+
 use Trog::Utils();
 use Trog::Config();
 use Trog::Auth();
@@ -48,7 +50,12 @@ our %routes = (
     '/api/requests_per' => {
         method => 'GET',
         auth   => 1,
-        parameters => [qw{period num_periods before code}],
+        parameters => {
+            period      => sub { grep { my $valid=$_; List::Util::any { $_ eq $valid } @_ } qw{second minute hour day week month year} },
+            num_periods => \&Scalar::Util::looks_like_number,
+            before      => \&Scalar::Util::looks_like_number,
+            code        => \&Scalar::Util::looks_like_number,
+        },
         callback => \&requests_per,
     },
 );
@@ -104,6 +111,9 @@ sub process_auth_change_request ($query) {
 }
 
 sub requests_per($query) {
+    use Data::Dumper;
+    print Dumper($query);
+
     my $code = Trog::Utils::coerce_array($query->{code});
     return _render(
         200, undef, 

schema/log.schema

@@ -178,6 +178,6 @@ END;
 
 /* This is just to store various messages associated with requests, which are usually errors. */
 CREATE TABLE IF NOT EXISTS messages (
-    uuid TEXT NOT NULL REFERENCES requests ON DELETE NO ACTION,
+    uuid TEXT NOT NULL REFERENCES requests(uuid) ON DELETE NO ACTION,
     message TEXT NOT NULL
 );

www/templates/html/sysbar.tx

@@ -4,7 +4,7 @@
         <a href="/"            title="Back home"     class="topbar">Home</a>
         <a href="/config"      title="Configuration" class="topbar">Settings</a>
         <a href="/manual"      title="Manual"        class="topbar">Manual</a>
-        <a href="/metrics"     title="Metrics"       class="topbar">Metrics</a>
+        <a href="/metrics?period=hour&num_periods=24"     title="Metrics"       class="topbar">Metrics</a>
         <a href="/totp"        title="TOTP"          class="topbar">Enable/View TOTP 2fa</a>
         <a href="/password_reset" title="Reset Auth" class="topbar">Reset Password/TOTP</a>
         <a href="/logout"      title="Logout"        class="topbar">🚪</a>