mirror of
https://github.com/DorkSquadRR/DorkNet.git
synced 2026-10-25 15:08:47 -07:00
fix(auth): encrypt stored cookie keys; tighten the link and sign-out flows
- Data-protection keys are encrypted (AES-256-GCM, key derived from the server secret) before they reach the DataProtectionKeys table. - /link only follows local return paths and keeps the form when a signed-in visitor has an identity waiting to be linked. - A failed sign-out says so instead of reloading as if it worked.
This commit is contained in:
@@ -147,6 +147,37 @@ public sealed class ExternalSignInTests : IClassFixture<DorkNetServerFactory>
|
|||||||
Assert.Equal(HttpStatusCode.Forbidden, relink.StatusCode);
|
Assert.Equal(HttpStatusCode.Forbidden, relink.StatusCode);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Cookie_keys_are_stored_encrypted()
|
||||||
|
{
|
||||||
|
var (player, password) = await PlayerAsync();
|
||||||
|
using var site = Client();
|
||||||
|
using var link = await PostJsonAsync(site, "/api/site/v1/account/link",
|
||||||
|
new { username = player.Username, password }, ExternalCookie(Guid.NewGuid().ToString("N"), ""));
|
||||||
|
Assert.Equal(HttpStatusCode.OK, link.StatusCode);
|
||||||
|
|
||||||
|
await using var scope = _factory.Services.CreateAsyncScope();
|
||||||
|
var db = scope.ServiceProvider.GetRequiredService<DorkNetDbContext>();
|
||||||
|
var stored = await db.DataProtectionKeys.Select(k => k.Xml).ToListAsync();
|
||||||
|
Assert.NotEmpty(stored);
|
||||||
|
Assert.All(stored, xml =>
|
||||||
|
{
|
||||||
|
Assert.Contains("dorknetEncryptedKey", xml);
|
||||||
|
Assert.DoesNotContain("masterKey", xml);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Key_encryption_round_trips_and_rejects_another_secret()
|
||||||
|
{
|
||||||
|
var element = new System.Xml.Linq.XElement("key", new System.Xml.Linq.XElement("masterKey", "secret-bytes"));
|
||||||
|
var encrypted = new DataProtectionKeyEncryption("first-secret").Encrypt(element).EncryptedElement;
|
||||||
|
Assert.DoesNotContain("secret-bytes", encrypted.ToString());
|
||||||
|
Assert.Equal(element.ToString(), new DataProtectionKeyEncryption("first-secret").DecryptElement(encrypted).ToString());
|
||||||
|
Assert.ThrowsAny<System.Security.Cryptography.CryptographicException>(
|
||||||
|
() => new DataProtectionKeyEncryption("other-secret").DecryptElement(encrypted));
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task Downloads_can_require_sign_in()
|
public async Task Downloads_can_require_sign_in()
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -19,13 +19,18 @@ public class AuthService(IConfiguration config, DorkNetDbContext db)
|
|||||||
// configs) and leave Jwt:Secret as the appsettings.json placeholder;
|
// configs) and leave Jwt:Secret as the appsettings.json placeholder;
|
||||||
// the mismatched-secret bug shows up only there.
|
// the mismatched-secret bug shows up only there.
|
||||||
private string SecretKey =>
|
private string SecretKey =>
|
||||||
Environment.GetEnvironmentVariable("DORKNET_JWT_SECRET")
|
ConfiguredSecret(config)
|
||||||
?? Environment.GetEnvironmentVariable("RECNET_JWT_SECRET")
|
|
||||||
?? config["Jwt:Secret"]
|
|
||||||
?? throw new InvalidOperationException(
|
?? throw new InvalidOperationException(
|
||||||
"JWT secret not configured. Set DORKNET_JWT_SECRET env var or " +
|
"JWT secret not configured. Set DORKNET_JWT_SECRET env var or " +
|
||||||
"Jwt:Secret in appsettings.Local.json.");
|
"Jwt:Secret in appsettings.Local.json.");
|
||||||
|
|
||||||
|
/// <summary>The server secret, in the one resolution order every user of
|
||||||
|
/// it shares (token signing, token validation, key encryption).</summary>
|
||||||
|
public static string? ConfiguredSecret(IConfiguration config) =>
|
||||||
|
Environment.GetEnvironmentVariable("DORKNET_JWT_SECRET")
|
||||||
|
?? Environment.GetEnvironmentVariable("RECNET_JWT_SECRET")
|
||||||
|
?? config["Jwt:Secret"];
|
||||||
|
|
||||||
public string GenerateToken(long playerId) => CreateJwt(playerId, TimeSpan.FromHours(12), "access");
|
public string GenerateToken(long playerId) => CreateJwt(playerId, TimeSpan.FromHours(12), "access");
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
@@ -0,0 +1,67 @@
|
|||||||
|
using System.Security.Cryptography;
|
||||||
|
using System.Text;
|
||||||
|
using System.Xml.Linq;
|
||||||
|
using Microsoft.AspNetCore.DataProtection.XmlEncryption;
|
||||||
|
using Microsoft.Extensions.DependencyInjection;
|
||||||
|
|
||||||
|
namespace DorkNet.Server.Services;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Encrypts the data-protection key ring (the keys behind the sign-in
|
||||||
|
/// cookies) before it is stored in the <c>DataProtectionKeys</c> table:
|
||||||
|
/// AES-256-GCM under a key derived (HKDF-SHA256) from the server secret.
|
||||||
|
/// The secret comes from the environment, not the database, so a database
|
||||||
|
/// backup alone holds no usable keys. Changing the secret makes the stored
|
||||||
|
/// keys unreadable: the server then creates a new key and everyone signs in
|
||||||
|
/// again.
|
||||||
|
/// </summary>
|
||||||
|
public sealed class DataProtectionKeyEncryption : IXmlEncryptor
|
||||||
|
{
|
||||||
|
private const int NonceSize = 12;
|
||||||
|
private const int TagSize = 16;
|
||||||
|
private static readonly byte[] Info = Encoding.UTF8.GetBytes("DorkNet.DataProtectionKeys.v1");
|
||||||
|
private static readonly XName Root = "dorknetEncryptedKey";
|
||||||
|
private readonly byte[] _key;
|
||||||
|
|
||||||
|
public DataProtectionKeyEncryption(string secret)
|
||||||
|
{
|
||||||
|
if (string.IsNullOrEmpty(secret))
|
||||||
|
throw new InvalidOperationException("The server secret is required to encrypt the data-protection keys.");
|
||||||
|
_key = HKDF.DeriveKey(HashAlgorithmName.SHA256, Encoding.UTF8.GetBytes(secret), 32, salt: null, info: Info);
|
||||||
|
}
|
||||||
|
|
||||||
|
public EncryptedXmlInfo Encrypt(XElement plaintextElement)
|
||||||
|
{
|
||||||
|
var plaintext = Encoding.UTF8.GetBytes(plaintextElement.ToString(SaveOptions.DisableFormatting));
|
||||||
|
var payload = new byte[NonceSize + TagSize + plaintext.Length];
|
||||||
|
var nonce = payload.AsSpan(0, NonceSize);
|
||||||
|
RandomNumberGenerator.Fill(nonce);
|
||||||
|
using (var aes = new AesGcm(_key, TagSize))
|
||||||
|
aes.Encrypt(nonce, plaintext, payload.AsSpan(NonceSize + TagSize), payload.AsSpan(NonceSize, TagSize));
|
||||||
|
return new EncryptedXmlInfo(
|
||||||
|
new XElement(Root, new XAttribute("version", "1"), Convert.ToBase64String(payload)),
|
||||||
|
typeof(Decryptor));
|
||||||
|
}
|
||||||
|
|
||||||
|
public XElement DecryptElement(XElement encrypted)
|
||||||
|
{
|
||||||
|
if (encrypted.Name != Root || (string?)encrypted.Attribute("version") != "1")
|
||||||
|
throw new CryptographicException("Unrecognised encrypted data-protection key.");
|
||||||
|
var payload = Convert.FromBase64String(encrypted.Value);
|
||||||
|
if (payload.Length < NonceSize + TagSize)
|
||||||
|
throw new CryptographicException("Encrypted data-protection key is truncated.");
|
||||||
|
var plaintext = new byte[payload.Length - NonceSize - TagSize];
|
||||||
|
using (var aes = new AesGcm(_key, TagSize))
|
||||||
|
aes.Decrypt(payload.AsSpan(0, NonceSize), payload.AsSpan(NonceSize + TagSize),
|
||||||
|
payload.AsSpan(NonceSize, TagSize), plaintext);
|
||||||
|
return XElement.Parse(Encoding.UTF8.GetString(plaintext));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Created by the data-protection system from the stored type
|
||||||
|
/// name, with the app's service provider.</summary>
|
||||||
|
public sealed class Decryptor(IServiceProvider services) : IXmlDecryptor
|
||||||
|
{
|
||||||
|
public XElement Decrypt(XElement encryptedElement) =>
|
||||||
|
services.GetRequiredService<DataProtectionKeyEncryption>().DecryptElement(encryptedElement);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Authentication.Cookies;
|
|||||||
using Microsoft.AspNetCore.Authentication.JwtBearer;
|
using Microsoft.AspNetCore.Authentication.JwtBearer;
|
||||||
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
|
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
|
||||||
using Microsoft.AspNetCore.DataProtection;
|
using Microsoft.AspNetCore.DataProtection;
|
||||||
|
using Microsoft.AspNetCore.DataProtection.KeyManagement;
|
||||||
using Microsoft.Data.Sqlite;
|
using Microsoft.Data.Sqlite;
|
||||||
using Microsoft.EntityFrameworkCore;
|
using Microsoft.EntityFrameworkCore;
|
||||||
using Microsoft.IdentityModel.Tokens;
|
using Microsoft.IdentityModel.Tokens;
|
||||||
@@ -345,9 +346,7 @@ public static class ServiceCollectionExtensions
|
|||||||
// older Coolify configs.
|
// older Coolify configs.
|
||||||
private static void AddAuthentication(WebApplicationBuilder builder)
|
private static void AddAuthentication(WebApplicationBuilder builder)
|
||||||
{
|
{
|
||||||
var jwtSecret = Environment.GetEnvironmentVariable("DORKNET_JWT_SECRET")
|
var jwtSecret = AuthService.ConfiguredSecret(builder.Configuration)
|
||||||
?? Environment.GetEnvironmentVariable("RECNET_JWT_SECRET")
|
|
||||||
?? builder.Configuration["Jwt:Secret"]
|
|
||||||
?? throw new InvalidOperationException(
|
?? throw new InvalidOperationException(
|
||||||
"JWT secret not configured. Set the DORKNET_JWT_SECRET env var or " +
|
"JWT secret not configured. Set the DORKNET_JWT_SECRET env var or " +
|
||||||
"add `Jwt:Secret` to appsettings.Local.json (the install-plugin.ps1 " +
|
"add `Jwt:Secret` to appsettings.Local.json (the install-plugin.ps1 " +
|
||||||
@@ -422,10 +421,17 @@ public static class ServiceCollectionExtensions
|
|||||||
services.AddSingleton<LinkAttemptLimiter>();
|
services.AddSingleton<LinkAttemptLimiter>();
|
||||||
|
|
||||||
// Cookie encryption keys live in the database, so a redeploy or a
|
// Cookie encryption keys live in the database, so a redeploy or a
|
||||||
// second replica doesn't invalidate everyone's session.
|
// second replica doesn't invalidate everyone's session. They're
|
||||||
|
// stored encrypted with a key derived from the server secret, which
|
||||||
|
// lives outside the database, so a database backup alone can't forge
|
||||||
|
// sessions.
|
||||||
|
services.AddSingleton(new DataProtectionKeyEncryption(
|
||||||
|
AuthService.ConfiguredSecret(builder.Configuration) ?? string.Empty));
|
||||||
services.AddDataProtection()
|
services.AddDataProtection()
|
||||||
.SetApplicationName("DorkNet")
|
.SetApplicationName("DorkNet")
|
||||||
.PersistKeysToDbContext<DorkNetDbContext>();
|
.PersistKeysToDbContext<DorkNetDbContext>();
|
||||||
|
services.AddOptions<KeyManagementOptions>()
|
||||||
|
.Configure<DataProtectionKeyEncryption>((o, enc) => o.XmlEncryptor = enc);
|
||||||
|
|
||||||
var auth = services.AddAuthentication()
|
var auth = services.AddAuthentication()
|
||||||
.AddCookie(ExternalSignIn.WebScheme, o =>
|
.AddCookie(ExternalSignIn.WebScheme, o =>
|
||||||
|
|||||||
@@ -34,9 +34,15 @@ export function Header() {
|
|||||||
getMe().then(setMe).catch(() => { /* site works without sign-in */ });
|
getMe().then(setMe).catch(() => { /* site works without sign-in */ });
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
|
const [signOutError, setSignOutError] = useState<string | null>(null);
|
||||||
const doSignOut = async () => {
|
const doSignOut = async () => {
|
||||||
await signOut().catch(() => {});
|
setSignOutError(null);
|
||||||
window.location.assign('/');
|
try {
|
||||||
|
await signOut();
|
||||||
|
window.location.assign('/');
|
||||||
|
} catch (e) {
|
||||||
|
setSignOutError((e as Error).message);
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
// Sign in / account, shown only when the server has a sign-in provider.
|
// Sign in / account, shown only when the server has a sign-in provider.
|
||||||
@@ -136,6 +142,15 @@ export function Header() {
|
|||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
|
{signOutError && (
|
||||||
|
<div className="border-t border-danger/30 bg-danger/10">
|
||||||
|
<div className="mx-auto max-w-6xl px-4 sm:px-6 py-2 flex items-center justify-between gap-3 text-sm text-danger">
|
||||||
|
<span>Couldn't sign out ({signOutError}). You're still signed in; try again.</span>
|
||||||
|
<button type="button" className="btn-ghost !px-2 !py-1 text-xs" onClick={() => setSignOutError(null)}>Dismiss</button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
|
||||||
{signinError && (
|
{signinError && (
|
||||||
<div className="border-t border-danger/30 bg-danger/10">
|
<div className="border-t border-danger/30 bg-danger/10">
|
||||||
<div className="mx-auto max-w-6xl px-4 sm:px-6 py-2 flex items-center justify-between gap-3 text-sm text-danger">
|
<div className="mx-auto max-w-6xl px-4 sm:px-6 py-2 flex items-center justify-between gap-3 text-sm text-danger">
|
||||||
|
|||||||
@@ -19,6 +19,13 @@ export function getMe(fresh = false): Promise<AccountMe> {
|
|||||||
return cached;
|
return cached;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** `url` if it is a path on this site, else `/` (the server applies the same
|
||||||
|
* rule): never navigate to an address taken from the query string otherwise. */
|
||||||
|
export function localPath(url: string | null | undefined): string {
|
||||||
|
if (!url || !url.startsWith('/') || url.startsWith('//') || url.startsWith('/\\')) return '/';
|
||||||
|
return url;
|
||||||
|
}
|
||||||
|
|
||||||
/** Starts sign-in at the provider; comes back to `returnUrl` on this site. */
|
/** Starts sign-in at the provider; comes back to `returnUrl` on this site. */
|
||||||
export function signInUrl(returnUrl = window.location.pathname + window.location.search): string {
|
export function signInUrl(returnUrl = window.location.pathname + window.location.search): string {
|
||||||
return `/api/auth/external/login?returnUrl=${encodeURIComponent(returnUrl)}`;
|
return `/api/auth/external/login?returnUrl=${encodeURIComponent(returnUrl)}`;
|
||||||
|
|||||||
@@ -1,12 +1,12 @@
|
|||||||
import { useEffect, useState } from 'react';
|
import { useEffect, useState } from 'react';
|
||||||
import { Link as RouterLink, useNavigate, useSearchParams } from 'react-router';
|
import { Link as RouterLink, useNavigate, useSearchParams } from 'react-router';
|
||||||
import { getMe, link, LINK_ERRORS, signInUrl, type AccountMe } from '../lib/account';
|
import { getMe, link, LINK_ERRORS, localPath, signInUrl, type AccountMe } from '../lib/account';
|
||||||
|
|
||||||
// After a first sign-in with the provider: prove which DorkNet account this
|
// After a first sign-in with the provider: prove which DorkNet account this
|
||||||
// identity belongs to, once, with that account's password.
|
// identity belongs to, once, with that account's password.
|
||||||
export function Link() {
|
export function Link() {
|
||||||
const [params] = useSearchParams();
|
const [params] = useSearchParams();
|
||||||
const returnUrl = params.get('returnUrl') || '/';
|
const returnUrl = localPath(params.get('returnUrl'));
|
||||||
const navigate = useNavigate();
|
const navigate = useNavigate();
|
||||||
const [me, setMe] = useState<AccountMe | null>(null);
|
const [me, setMe] = useState<AccountMe | null>(null);
|
||||||
const [username, setUsername] = useState('');
|
const [username, setUsername] = useState('');
|
||||||
@@ -16,7 +16,10 @@ export function Link() {
|
|||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
getMe(true).then((m) => {
|
getMe(true).then((m) => {
|
||||||
if (m.signedIn) navigate(returnUrl, { replace: true });
|
// Signed in with nothing waiting to be linked: nothing to do here. (A
|
||||||
|
// signed-in visitor can still link a new identity, e.g. a second
|
||||||
|
// account's.)
|
||||||
|
if (m.signedIn && !m.pendingLink) navigate(returnUrl, { replace: true });
|
||||||
else setMe(m);
|
else setMe(m);
|
||||||
}).catch((e) => setErr((e as Error).message));
|
}).catch((e) => setErr((e as Error).message));
|
||||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
|
|||||||
+4
-1
@@ -497,7 +497,10 @@ above.
|
|||||||
|
|
||||||
Sessions are HttpOnly cookies per host (`dorknet_session`, 30 days, sliding).
|
Sessions are HttpOnly cookies per host (`dorknet_session`, 30 days, sliding).
|
||||||
The keys that encrypt them are stored in the database (`DataProtectionKeys`),
|
The keys that encrypt them are stored in the database (`DataProtectionKeys`),
|
||||||
so redeploys and extra replicas keep everyone signed in. Unlinking the
|
so redeploys and extra replicas keep everyone signed in. They're stored
|
||||||
|
encrypted with a key derived from the server secret (`DORKNET_JWT_SECRET`),
|
||||||
|
so a database backup alone can't forge sessions; changing that secret signs
|
||||||
|
everyone out. Unlinking the
|
||||||
identity, deleting the account or banning the player ends the session on the
|
identity, deleting the account or banning the player ends the session on the
|
||||||
next request. Wrong passwords on the link form are limited to 5 per 15
|
next request. Wrong passwords on the link form are limited to 5 per 15
|
||||||
minutes per provider identity and per username.
|
minutes per provider identity and per username.
|
||||||
|
|||||||
Reference in New Issue
Block a user