Fix the security audit's findings
ShareDownload::$authenticated was a public property without #[Locked], so a visitor with a share's link but not its password could set it from the browser and see the file list: names, folder paths, sizes and whether the share holds a text. The contents stayed protected; the flag is now locked, and a test tries to set it. Sessions hold the keys of the password shares a recipient unlocked, and were stored unencrypted in the sessions table wherever SESSION_ENCRYPT was not set, which includes every Docker install. They are now encrypted unless SESSION_ENCRYPT=false. Share keys never live in the session, so every share stays readable; people sign in again once. The container generated an APP_KEY when none was set, printed it to the logs and generated another on the next start, which left the shares without a password unreadable. It now refuses to start without one and says how to create it; docker-compose.example.yml requires it for both services. The command it names, and the README's, now pass --entrypoint php: the image's entrypoint ignores its arguments, so "docker run <image> php artisan key:generate --show" never ran it. Every proxy is trusted, so the password-reset mail built its link from a host a forged X-Forwarded-Host could choose. It is now built from APP_URL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
6c3202a0cf
commit
6578335568
@@ -94,7 +94,7 @@ curl -O https://gitea.nonameweb.ch/noNameWEB/SealShare/raw/branch/main/docker-co
|
|||||||
cp docker-compose.example.yml docker-compose.yml
|
cp docker-compose.example.yml docker-compose.yml
|
||||||
|
|
||||||
# Generate an app key and paste it into docker-compose.yml
|
# Generate an app key and paste it into docker-compose.yml
|
||||||
docker run --rm gitea.nonameweb.ch/nonameweb/sealshare:latest php artisan key:generate --show
|
docker run --rm --entrypoint php gitea.nonameweb.ch/nonameweb/sealshare:latest artisan key:generate --show
|
||||||
|
|
||||||
# Edit docker-compose.yml — set APP_KEY and APP_URL, and choose how HTTPS is served (below)
|
# Edit docker-compose.yml — set APP_KEY and APP_URL, and choose how HTTPS is served (below)
|
||||||
# Then start:
|
# Then start:
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ use App\Services\ShareService;
|
|||||||
use Carbon\CarbonInterval;
|
use Carbon\CarbonInterval;
|
||||||
use Illuminate\Support\Facades\RateLimiter;
|
use Illuminate\Support\Facades\RateLimiter;
|
||||||
use Livewire\Attributes\Layout;
|
use Livewire\Attributes\Layout;
|
||||||
|
use Livewire\Attributes\Locked;
|
||||||
use Livewire\Attributes\Renderless;
|
use Livewire\Attributes\Renderless;
|
||||||
use Livewire\Attributes\Validate;
|
use Livewire\Attributes\Validate;
|
||||||
use Livewire\Component;
|
use Livewire\Component;
|
||||||
@@ -16,6 +17,8 @@ class ShareDownload extends Component
|
|||||||
{
|
{
|
||||||
public Share $share;
|
public Share $share;
|
||||||
|
|
||||||
|
/** Set only by mount() and verifyPassword(): the browser must never unlock the file list. */
|
||||||
|
#[Locked]
|
||||||
public bool $authenticated = false;
|
public bool $authenticated = false;
|
||||||
|
|
||||||
#[Validate('required|string')]
|
#[Validate('required|string')]
|
||||||
|
|||||||
@@ -3,6 +3,8 @@
|
|||||||
namespace App\Providers;
|
namespace App\Providers;
|
||||||
|
|
||||||
use App\Actions\Fortify\ResetUserPassword;
|
use App\Actions\Fortify\ResetUserPassword;
|
||||||
|
use App\Models\User;
|
||||||
|
use Illuminate\Auth\Notifications\ResetPassword;
|
||||||
use Illuminate\Cache\RateLimiting\Limit;
|
use Illuminate\Cache\RateLimiting\Limit;
|
||||||
use Illuminate\Http\Request;
|
use Illuminate\Http\Request;
|
||||||
use Illuminate\Support\Facades\RateLimiter;
|
use Illuminate\Support\Facades\RateLimiter;
|
||||||
@@ -28,6 +30,12 @@ class FortifyServiceProvider extends ServiceProvider
|
|||||||
private function configureActions(): void
|
private function configureActions(): void
|
||||||
{
|
{
|
||||||
Fortify::resetUserPasswordsUsing(ResetUserPassword::class);
|
Fortify::resetUserPasswordsUsing(ResetUserPassword::class);
|
||||||
|
|
||||||
|
// From APP_URL, never the request's host: every proxy is trusted, so a forged X-Forwarded-Host would otherwise pick it.
|
||||||
|
ResetPassword::createUrlUsing(fn (User $user, string $token): string => rtrim((string) config('app.url'), '/').route('password.reset', [
|
||||||
|
'token' => $token,
|
||||||
|
'email' => $user->getEmailForPasswordReset(),
|
||||||
|
], false));
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
+8
-2
@@ -8,13 +8,19 @@ use Illuminate\Support\Str;
|
|||||||
|--------------------------------------------------------------------------
|
|--------------------------------------------------------------------------
|
||||||
|
|
|
|
||||||
| Kept from earlier releases, where it differs from the framework's
|
| Kept from earlier releases, where it differs from the framework's
|
||||||
| "<app>_session": renaming the cookie would sign everyone out. Every
|
| "<app>_session": renaming the cookie would sign everyone out.
|
||||||
| other key comes from the framework's defaults.
|
|
|
||||||
|
| Sessions are encrypted unless SESSION_ENCRYPT=false: they hold the keys
|
||||||
|
| of the password shares a recipient unlocked, which must never sit
|
||||||
|
| readable in the sessions table. Every other key comes from the
|
||||||
|
| framework's defaults.
|
||||||
|
|
|
|
||||||
*/
|
*/
|
||||||
|
|
||||||
return [
|
return [
|
||||||
|
|
||||||
|
'encrypt' => env('SESSION_ENCRYPT', true),
|
||||||
|
|
||||||
'cookie' => env(
|
'cookie' => env(
|
||||||
'SESSION_COOKIE',
|
'SESSION_COOKIE',
|
||||||
Str::slug((string) env('APP_NAME', 'laravel')).'-session'
|
Str::slug((string) env('APP_NAME', 'laravel')).'-session'
|
||||||
|
|||||||
@@ -4,12 +4,13 @@
|
|||||||
#
|
#
|
||||||
# Quick start:
|
# Quick start:
|
||||||
# 1. Copy this file: cp docker-compose.example.yml docker-compose.yml
|
# 1. Copy this file: cp docker-compose.example.yml docker-compose.yml
|
||||||
# 2. Edit the settings below (APP_URL is required; uploads need HTTPS, see below)
|
# 2. Edit the settings below (APP_URL and APP_KEY are required; uploads need HTTPS, see below)
|
||||||
# 3. Start: docker compose up -d
|
# 3. Start: docker compose up -d
|
||||||
# 4. Open your browser to your configured domain
|
# 4. Open your browser to your configured domain
|
||||||
#
|
#
|
||||||
# Note: APP_KEY is auto-generated on first start if not set.
|
# APP_KEY is required: the container does not start without it. Create one with
|
||||||
# Copy it from the logs into your docker-compose.yml to persist across restarts.
|
# docker run --rm --entrypoint php gitea.nonameweb.ch/nonameweb/sealshare:latest artisan key:generate --show
|
||||||
|
# and keep it: it decrypts the shares, so a new key makes the existing ones unreadable.
|
||||||
#
|
#
|
||||||
# ============================================
|
# ============================================
|
||||||
|
|
||||||
@@ -32,7 +33,7 @@ services:
|
|||||||
environment:
|
environment:
|
||||||
# --- REQUIRED ---
|
# --- REQUIRED ---
|
||||||
APP_URL: # Your full URL, e.g. https://share.example.com
|
APP_URL: # Your full URL, e.g. https://share.example.com
|
||||||
# APP_KEY: # Auto-generated if not set. Copy from logs to persist across restarts.
|
APP_KEY: # Required. Create one with php artisan key:generate --show (see above)
|
||||||
DB_DATABASE: /app/database/sqlite/database.sqlite # The SQLite file in sealshare_database
|
DB_DATABASE: /app/database/sqlite/database.sqlite # The SQLite file in sealshare_database
|
||||||
|
|
||||||
# --- HTTPS ---
|
# --- HTTPS ---
|
||||||
@@ -90,7 +91,7 @@ services:
|
|||||||
- sealshare_storage:/app/storage/app
|
- sealshare_storage:/app/storage/app
|
||||||
- sealshare_database:/app/database/sqlite
|
- sealshare_database:/app/database/sqlite
|
||||||
environment:
|
environment:
|
||||||
# APP_KEY: # Same key as the app service above (auto-generated if not set)
|
APP_KEY: # Same key as the app service above
|
||||||
APP_URL: # Same URL as the app service above
|
APP_URL: # Same URL as the app service above
|
||||||
DB_DATABASE: /app/database/sqlite/database.sqlite # Same as the app service above
|
DB_DATABASE: /app/database/sqlite/database.sqlite # Same as the app service above
|
||||||
depends_on:
|
depends_on:
|
||||||
|
|||||||
@@ -3,13 +3,14 @@ set -e
|
|||||||
|
|
||||||
cd /app
|
cd /app
|
||||||
|
|
||||||
# Auto-generate APP_KEY if not provided
|
# APP_KEY decrypts the shares, so it must stay the same across restarts: a key made up here would
|
||||||
|
# change on the next start and leave every existing share unreadable.
|
||||||
if [ -z "$APP_KEY" ]; then
|
if [ -z "$APP_KEY" ]; then
|
||||||
echo "[entrypoint] No APP_KEY set, generating one..."
|
echo "[entrypoint] APP_KEY is not set. Create one and set it for the app and the scheduler:" >&2
|
||||||
APP_KEY=$(php artisan key:generate --show)
|
echo "[entrypoint] docker run --rm --entrypoint php <this image> artisan key:generate --show" >&2
|
||||||
export APP_KEY
|
echo "[entrypoint] If this install ran without one before, use the key from its old logs" >&2
|
||||||
echo "[entrypoint] Generated APP_KEY: $APP_KEY"
|
echo "[entrypoint] (\"Generated APP_KEY: ...\") so its shares stay readable." >&2
|
||||||
echo "[entrypoint] WARNING: Set this APP_KEY in your docker-compose.yml to persist across restarts!"
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# A docker-compose.yml from before 2.1.1 mounts the SQLite volume over all of /app/database, so the
|
# A docker-compose.yml from before 2.1.1 mounts the SQLite volume over all of /app/database, so the
|
||||||
|
|||||||
@@ -20,6 +20,20 @@ test('reset password link can be requested', function () {
|
|||||||
Notification::assertSentTo($user, ResetPassword::class);
|
Notification::assertSentTo($user, ResetPassword::class);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('the reset link points at APP_URL, whatever host the request claims', function () {
|
||||||
|
Notification::fake();
|
||||||
|
config(['app.url' => 'https://share.example.com']);
|
||||||
|
|
||||||
|
$user = User::factory()->create();
|
||||||
|
|
||||||
|
$this->withHeader('X-Forwarded-Host', 'attacker.example')
|
||||||
|
->post(route('password.request'), ['email' => $user->email]);
|
||||||
|
|
||||||
|
Notification::assertSentTo($user, ResetPassword::class, function (ResetPassword $notification) use ($user) {
|
||||||
|
return str_starts_with($notification->toMail($user)->actionUrl, 'https://share.example.com/reset-password/');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
test('reset password screen can be rendered', function () {
|
test('reset password screen can be rendered', function () {
|
||||||
Notification::fake();
|
Notification::fake();
|
||||||
|
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ use App\Services\ShareService;
|
|||||||
use Illuminate\Http\UploadedFile;
|
use Illuminate\Http\UploadedFile;
|
||||||
use Illuminate\Support\Facades\RateLimiter;
|
use Illuminate\Support\Facades\RateLimiter;
|
||||||
use Illuminate\Support\Facades\Storage;
|
use Illuminate\Support\Facades\Storage;
|
||||||
|
use Livewire\Features\SupportLockedProperties\CannotUpdateLockedPropertyException;
|
||||||
use Livewire\Livewire;
|
use Livewire\Livewire;
|
||||||
|
|
||||||
// --- Session stores derived key, not plaintext password ---
|
// --- Session stores derived key, not plaintext password ---
|
||||||
@@ -35,6 +36,22 @@ test('session stores derived encryption key instead of plaintext password', func
|
|||||||
expect(strlen(session('share_key_'.$share->token)))->toBe(64);
|
expect(strlen(session('share_key_'.$share->token)))->toBe(64);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// --- The password gate cannot be opened from the browser ---
|
||||||
|
|
||||||
|
test('a visitor without the password cannot unlock a share\'s file list from the browser', function () {
|
||||||
|
Storage::fake('shares');
|
||||||
|
|
||||||
|
$share = app(ShareService::class)->createShare([
|
||||||
|
['file' => UploadedFile::fake()->createWithContent('salary-review.pdf', 'secret'), 'relativePath' => null],
|
||||||
|
], [
|
||||||
|
'password' => 'test-password-secure',
|
||||||
|
]);
|
||||||
|
|
||||||
|
Livewire::test(ShareDownload::class, ['share' => $share])
|
||||||
|
->assertDontSee('salary-review.pdf')
|
||||||
|
->set('authenticated', true);
|
||||||
|
})->throws(CannotUpdateLockedPropertyException::class);
|
||||||
|
|
||||||
// --- Rate limiting on password verification ---
|
// --- Rate limiting on password verification ---
|
||||||
|
|
||||||
test('rate limiting blocks after 5 failed password attempts', function () {
|
test('rate limiting blocks after 5 failed password attempts', function () {
|
||||||
|
|||||||
Reference in New Issue
Block a user