#16 (audit L-1..L-8): fix the low-severity findings
L-1 RequireRole: guard std::stoi on the bundle id — a non-numeric/out-of-range
value now yields a clean 401 instead of an uncaught exception → 500.
AuthPrincipal::id documented as numeric-only (carry UUIDs in username).
L-2 SmtpTransport: require TLS (CURLUSESSL_ALL) for non-loopback relays so a
stripped STARTTLS can't downgrade credentials/body to cleartext; localhost
relay stays opportunistic.
L-3 AuditLog: escapeJson now escapes all control chars (RFC 8259) so a newline
in a field can't forge/corrupt the audit JSON; SKIP_FIELDS gains credential
names (password/passwordHash/tlsCertDn/apiKey/token/secret) so secrets never
land in changed_fields.
L-4 ws/Hub: consume the thread_local auth handoff once, up front, and clear it
unconditionally — a stale value can't attach to a later connection on a
reused worker thread.
L-5 TemporalRepository: default id generator draws 128 bits from the platform
CSPRNG (std::random_device) per call instead of a once-seeded mt19937_64,
so entity_ids aren't predictable from observed output.
L-6 AuthInterceptor: expired-session sweep is now a lock-free atomic timer and
exception-isolated; documented that resolveBySessionHash() must enforce
expiry at query time (the sweep is GC only).
L-7 new util/ConstantTime.hpp (constantTimeEquals) + TokenHasher doc requiring a
>=256-bit cryptographic hash.
L-8 IQueryable: likeEscape + Field::likeContains/likePrefix emit
`LIKE ? ESCAPE '\'` with %/_/\ escaped for untrusted terms; documented the
compile-time identifier-source invariant.
Tests: new test_constant_time; likeEscape/likeContains/likePrefix cases added to
test_queryable. All 20 ctest targets pass. README + header docs updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
fafee1278f
commit
9976efe1de
14 changed files with 285 additions and 28 deletions
|
|
@ -14,11 +14,13 @@ hardened auth / security stack. Header-only, oatpp 1.3+, C++17.
|
||||||
| `util/TokenExtract.hpp` | `extractToken` (Cookie/Bearer) + `cookieValue(header,name)` exact-name cookie parse (authkit#16 M-1 — no substring matching, so a sibling `xsession=` can't shadow `session=`), `isValidIp` (IPv4/IPv6 via `inet_pton`), `clientIpTrusted` (loopback-gated XFF; returns the `"unknown"`/`"invalid"` sentinels off-proxy — treat as one shared rate-limit bucket, M-8). |
|
| `util/TokenExtract.hpp` | `extractToken` (Cookie/Bearer) + `cookieValue(header,name)` exact-name cookie parse (authkit#16 M-1 — no substring matching, so a sibling `xsession=` can't shadow `session=`), `isValidIp` (IPv4/IPv6 via `inet_pton`), `clientIpTrusted` (loopback-gated XFF; returns the `"unknown"`/`"invalid"` sentinels off-proxy — treat as one shared rate-limit bucket, M-8). |
|
||||||
| `util/OriginCheck.hpp` | `originHostname`, `sameOrigin(originOrReferer, host)`, `originAllowed(origin, allowlist)` — pure CSRF/CSWSH origin helpers (authkit#16 M-4/M-10). Used by `AuthInterceptor` for session mutations; call `sameOrigin`/`originAllowed` in your WSController to block Cross-Site WebSocket Hijacking at the handshake. |
|
| `util/OriginCheck.hpp` | `originHostname`, `sameOrigin(originOrReferer, host)`, `originAllowed(origin, allowlist)` — pure CSRF/CSWSH origin helpers (authkit#16 M-4/M-10). Used by `AuthInterceptor` for session mutations; call `sameOrigin`/`originAllowed` in your WSController to block Cross-Site WebSocket Hijacking at the handshake. |
|
||||||
| `util/SessionCookie.hpp` | `buildSetSessionCookie(token, opts)` / `buildClearSessionCookie(opts)` — safe-by-default `Set-Cookie` builder (HttpOnly + Secure + SameSite=Strict + Path=/ by default; opt out explicitly). Rejects control chars / `;` in fields (authkit#16 M-9). Returns the header value only; framework-agnostic. |
|
| `util/SessionCookie.hpp` | `buildSetSessionCookie(token, opts)` / `buildClearSessionCookie(opts)` — safe-by-default `Set-Cookie` builder (HttpOnly + Secure + SameSite=Strict + Path=/ by default; opt out explicitly). Rejects control chars / `;` in fields (authkit#16 M-9). Returns the header value only; framework-agnostic. |
|
||||||
|
| `util/ConstantTime.hpp` | `constantTimeEquals(a, b)` — branch-free secret comparison for consumers that compare a token/HMAC/hash in memory rather than via an indexed store lookup (authkit#16 L-7). |
|
||||||
|
| `mail/SmtpTransport.hpp` | libcurl SMTP+MIME sender. Requires TLS (`CURLUSESSL_ALL`) for non-loopback relays so credentials/body can't be sent cleartext if STARTTLS is stripped (authkit#16 L-2); a `localhost`/`127.0.0.1` relay stays opportunistic. Rejects CR/LF/NUL in `to`/`fromAddress` (header-injection guard, authkit#16 H-5). |
|
||||||
| `startup/RequireEncryptionKey.hpp` | `requireEncryptionKey(envVarName, encryptionEnabled, allowPlaintext)` — refuse startup without a symmetric key unless a dev flag overrides. |
|
| `startup/RequireEncryptionKey.hpp` | `requireEncryptionKey(envVarName, encryptionEnabled, allowPlaintext)` — refuse startup without a symmetric key unless a dev flag overrides. |
|
||||||
| `repo/Repository.hpp` + `IHistoryRepository.hpp` + `TemporalFieldTraits.hpp` + `TemporalAt.hpp` + `ActorContext.hpp` | Pure-abstract `Repository<TDto>` interface set distilled from fewo-webapp's per-entity `*Db` clients. Mixed UUID allocation on `save`, separate `IHistoryRepository<T>` for temporal versions, `TemporalFieldTraits<T>` to map canonical (entity_id, valid_from, valid_until) onto whatever a DTO actually calls them, `ActorContext` placeholder for the scope-guard decorator. |
|
| `repo/Repository.hpp` + `IHistoryRepository.hpp` + `TemporalFieldTraits.hpp` + `TemporalAt.hpp` + `ActorContext.hpp` | Pure-abstract `Repository<TDto>` interface set distilled from fewo-webapp's per-entity `*Db` clients. Mixed UUID allocation on `save`, separate `IHistoryRepository<T>` for temporal versions, `TemporalFieldTraits<T>` to map canonical (entity_id, valid_from, valid_until) onto whatever a DTO actually calls them, `ActorContext` placeholder for the scope-guard decorator. |
|
||||||
| `repo/TemporalRepository.hpp` | Decorator that wraps any `Repository<TDto>` and turns it into a temporally-versioned one. **Stable-live + historical-copy semantics (authkit#13):** the live row's `id` PK is preserved across updates; each prior version is captured as a fresh row with a new `id`. `softDelete` closes the live row in place; with `ON UPDATE CASCADE` on consumer-side composite child FKs, child rows follow automatically. `findByEntityIdAt(id, at)` returns the version live at a point in time; implements `IHistoryRepository<T>`. Inner adapter is expected to expose all rows (live + historical) and treat `save` as upsert keyed by **`id`** (per-row PK). DTOs register their four temporal columns via `OATPP_AUTHKIT_REGISTER_TEMPORAL(Dto, id, entity_id, valid_from, valid_until)`. |
|
| `repo/TemporalRepository.hpp` | Decorator that wraps any `Repository<TDto>` and turns it into a temporally-versioned one. **Stable-live + historical-copy semantics (authkit#13):** the live row's `id` PK is preserved across updates; each prior version is captured as a fresh row with a new `id`. `softDelete` closes the live row in place; with `ON UPDATE CASCADE` on consumer-side composite child FKs, child rows follow automatically. `findByEntityIdAt(id, at)` returns the version live at a point in time; implements `IHistoryRepository<T>`. Inner adapter is expected to expose all rows (live + historical) and treat `save` as upsert keyed by **`id`** (per-row PK). DTOs register their four temporal columns via `OATPP_AUTHKIT_REGISTER_TEMPORAL(Dto, id, entity_id, valid_from, valid_until)`. |
|
||||||
| `repo/ScopeGuardRepository.hpp` | Generic resource-scope decorator. Takes a `bool(ActorContext, TDto)` predicate, an actor accessor, and an `entity_id` accessor at construction; gates every method on the predicate. On `save` the predicate must pass on the incoming DTO **and**, for an update, on the row as it currently stands — so an actor can't reparent an out-of-scope row into its own scope by relabelling it in the request body. Throws `ScopeDeniedException` on deny (catchers translate to 403). Knows nothing about consumer-specific concepts like "property" or "tenant" — the predicate decides. **`ScopeGuardQueryable<T>`** (same header) is the variant for `IQueryable` inners: it filters `query()` results through the predicate too, so the queryable surface can't bypass the guard. |
|
| `repo/ScopeGuardRepository.hpp` | Generic resource-scope decorator. Takes a `bool(ActorContext, TDto)` predicate, an actor accessor, and an `entity_id` accessor at construction; gates every method on the predicate. On `save` the predicate must pass on the incoming DTO **and**, for an update, on the row as it currently stands — so an actor can't reparent an out-of-scope row into its own scope by relabelling it in the request body. Throws `ScopeDeniedException` on deny (catchers translate to 403). Knows nothing about consumer-specific concepts like "property" or "tenant" — the predicate decides. **`ScopeGuardQueryable<T>`** (same header) is the variant for `IQueryable` inners: it filters `query()` results through the predicate too, so the queryable surface can't bypass the guard. |
|
||||||
| `repo/IQueryable.hpp` | Optional capability for repos that resolve a typed query AST. `field<&Dto::col>().eq(...)` style DSL composes via `&&` / `||` / `!`; `Query<TDto>::toSql()` emits parameterised SQL plus a bind bag. Bounded surface — equality, range, IN, LIKE, NULL, ORDER BY, LIMIT/OFFSET. No joins, subqueries, or aggregates. Concrete repos opt in by deriving `IQueryable<TDto>`. Wrap a scope-guarded `IQueryable` with `ScopeGuardQueryable<T>` (not the plain `ScopeGuardRepository`) so `query()` is scope-filtered. |
|
| `repo/IQueryable.hpp` | Optional capability for repos that resolve a typed query AST. `field<&Dto::col>().eq(...)` style DSL composes via `&&` / `||` / `!`; `Query<TDto>::toSql()` emits parameterised SQL plus a bind bag. Bounded surface — equality, range, IN, LIKE, NULL, ORDER BY, LIMIT/OFFSET. No joins, subqueries, or aggregates. For user-supplied search terms use `likeContains`/`likePrefix` (or `likeEscape`), which escape `%`/`_`/`\` and emit `LIKE ? ESCAPE '\'` (authkit#16 L-8); raw `like()` binds the pattern verbatim (trusted patterns only). Column/table identifiers come only from compile-time registration — never from request data. Concrete repos opt in by deriving `IQueryable<TDto>`. Wrap a scope-guarded `IQueryable` with `ScopeGuardQueryable<T>` (not the plain `ScopeGuardRepository`) so `query()` is scope-filtered. |
|
||||||
| `repo/IAuditSink.hpp` + `repo/AuditLogRepository.hpp` | Cross-cutting audit-trail decorator. Emits an `AuditEvent` (actor, entity type/id, op, timestamp) per mutation through a consumer-supplied `IAuditSink`. Ops are `Create` / `Update` / `Delete` / `Read`; pre-write `findByEntityId` lookup distinguishes Create from Update. Configurable enabled-op set (default `{Create,Update,Delete}` — `Read` is opt-in, `list()` never audited). Sink failures are caught and swallowed unless a `bool(const std::exception&)` handler asks to rethrow. Stacks with `TemporalRepository` and `ScopeGuardRepository`. |
|
| `repo/IAuditSink.hpp` + `repo/AuditLogRepository.hpp` | Cross-cutting audit-trail decorator. Emits an `AuditEvent` (actor, entity type/id, op, timestamp) per mutation through a consumer-supplied `IAuditSink`. Ops are `Create` / `Update` / `Delete` / `Read`; pre-write `findByEntityId` lookup distinguishes Create from Update. Configurable enabled-op set (default `{Create,Update,Delete}` — `Read` is opt-in, `list()` never audited). Sink failures are caught and swallowed unless a `bool(const std::exception&)` handler asks to rethrow. Stacks with `TemporalRepository` and `ScopeGuardRepository`. |
|
||||||
| `repo/SchemaContract.hpp` | Declarative schema model for the decorator stack (authkit#14). Each decorator exposes a `static constexpr DecoratorSchema kSchema` listing the columns/indexes it contributes to the entity table plus any sidecar tables it owns. `SchemaBuilder<Decorators…>::create(table, exec)` composes contributions into a single `CREATE TABLE` per entity table; sidecars emit separately. `SchemaContract<Decorators…>::verify(table, probe)` is a runtime introspect-and-assert that throws `SchemaContractViolation` if any required column or sidecar is missing. Decorator code never runs ALTER at runtime — Atlas (atlasgo.io) owns evolution between deploys; the C++ side only declares desired state and checks it. |
|
| `repo/SchemaContract.hpp` | Declarative schema model for the decorator stack (authkit#14). Each decorator exposes a `static constexpr DecoratorSchema kSchema` listing the columns/indexes it contributes to the entity table plus any sidecar tables it owns. `SchemaBuilder<Decorators…>::create(table, exec)` composes contributions into a single `CREATE TABLE` per entity table; sidecars emit separately. `SchemaContract<Decorators…>::verify(table, probe)` is a runtime introspect-and-assert that throws `SchemaContractViolation` if any required column or sidecar is missing. Decorator code never runs ALTER at runtime — Atlas (atlasgo.io) owns evolution between deploys; the C++ side only declares desired state and checks it. |
|
||||||
| `repo/RedactedFieldRepository.hpp` | Decorator that nulls out named fields on **historical** rows only (authkit#15). Sits below `TemporalRepository` and inspects each `save`: if `valid_until != SENTINEL`, the row is being closed as a historical version, so the configured fields (e.g. `passwordHash`, `tlsCertDn`) are set to null before persisting. The live row keeps its values intact. Built for the case where a credential rides a temporal row — every change creates a historical version with the prior secret preserved, and the redaction prevents a DB breach from yielding every credential a user has ever had. The constructor throws `std::invalid_argument` if a configured field name isn't a DTO member (authkit#16 M-6) — a typo would otherwise silently redact nothing. |
|
| `repo/RedactedFieldRepository.hpp` | Decorator that nulls out named fields on **historical** rows only (authkit#15). Sits below `TemporalRepository` and inspects each `save`: if `valid_until != SENTINEL`, the row is being closed as a historical version, so the configured fields (e.g. `passwordHash`, `tlsCertDn`) are set to null before persisting. The live row keeps its values intact. Built for the case where a credential rides a temporal row — every change creates a historical version with the prior secret preserved, and the redaction prevents a DB breach from yielding every credential a user has ever had. The constructor throws `std::invalid_argument` if a configured field name isn't a DTO member (authkit#16 M-6) — a typo would otherwise silently redact nothing. |
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,9 @@
|
||||||
#ifndef OATPP_AUTHKIT_AUTH_INTERCEPTOR_HPP
|
#ifndef OATPP_AUTHKIT_AUTH_INTERCEPTOR_HPP
|
||||||
#define OATPP_AUTHKIT_AUTH_INTERCEPTOR_HPP
|
#define OATPP_AUTHKIT_AUTH_INTERCEPTOR_HPP
|
||||||
|
|
||||||
|
#include <atomic>
|
||||||
#include <chrono>
|
#include <chrono>
|
||||||
|
#include <cstdint>
|
||||||
#include <memory>
|
#include <memory>
|
||||||
#include <string>
|
#include <string>
|
||||||
#include <functional>
|
#include <functional>
|
||||||
|
|
@ -22,7 +24,13 @@
|
||||||
|
|
||||||
namespace oatpp_authkit {
|
namespace oatpp_authkit {
|
||||||
|
|
||||||
/** @brief Caller-supplied hash function — SHA-256 on the raw token typically. */
|
/** @brief Caller-supplied hash function — SHA-256 on the raw token typically.
|
||||||
|
*
|
||||||
|
* authkit#16 L-7: MUST be a fixed-length cryptographic hash (≥256-bit, e.g.
|
||||||
|
* SHA-256) over a high-entropy token. The store looks the session/API key up
|
||||||
|
* by this hash, so a weak/short/truncating hash weakens matching. Consumers
|
||||||
|
* that compare a secret in memory (rather than via an indexed lookup) should
|
||||||
|
* use `oatpp_authkit::constantTimeEquals` (`util/ConstantTime.hpp`). */
|
||||||
using TokenHasher = std::function<std::string(const std::string&)>;
|
using TokenHasher = std::function<std::string(const std::string&)>;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -225,14 +233,26 @@ public:
|
||||||
std::shared_ptr<OutgoingResponse> intercept(
|
std::shared_ptr<OutgoingResponse> intercept(
|
||||||
const std::shared_ptr<IncomingRequest>& request) override
|
const std::shared_ptr<IncomingRequest>& request) override
|
||||||
{
|
{
|
||||||
// Periodic expired-session sweep — at most once per hour.
|
// Periodic expired-session GC — at most once per hour, process-wide.
|
||||||
|
// authkit#16 L-6: this is best-effort cleanup, NOT the expiry gate —
|
||||||
|
// resolveBySessionHash() must itself reject expired sessions (see
|
||||||
|
// IAuthBackend). The timer is a lock-free atomic so concurrent requests
|
||||||
|
// don't race the read-modify-write, and the sweep is exception-isolated
|
||||||
|
// so a transient DB error during GC can't 500 an otherwise-valid request.
|
||||||
{
|
{
|
||||||
using Clock = std::chrono::steady_clock;
|
using Clock = std::chrono::steady_clock;
|
||||||
static Clock::time_point lastCleanup = Clock::now();
|
static std::atomic<std::int64_t> lastCleanupMs{-1};
|
||||||
auto now = Clock::now();
|
const std::int64_t nowMs = std::chrono::duration_cast<std::chrono::milliseconds>(
|
||||||
if (std::chrono::duration_cast<std::chrono::hours>(now - lastCleanup).count() >= 1) {
|
Clock::now().time_since_epoch()).count();
|
||||||
lastCleanup = now;
|
std::int64_t prev = lastCleanupMs.load(std::memory_order_relaxed);
|
||||||
m_backend->deleteExpiredSessions();
|
if (prev < 0) {
|
||||||
|
// First request: arm the timer, don't sweep yet.
|
||||||
|
lastCleanupMs.compare_exchange_strong(prev, nowMs);
|
||||||
|
} else if (nowMs - prev >= 3600000) {
|
||||||
|
// Only the thread that wins the CAS performs the sweep.
|
||||||
|
if (lastCleanupMs.compare_exchange_strong(prev, nowMs)) {
|
||||||
|
try { m_backend->deleteExpiredSessions(); } catch (...) {}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,12 @@ namespace oatpp_authkit {
|
||||||
* into this struct inside their IAuthBackend implementation.
|
* into this struct inside their IAuthBackend implementation.
|
||||||
*/
|
*/
|
||||||
struct AuthPrincipal {
|
struct AuthPrincipal {
|
||||||
int id{0}; ///< Stable numeric id from the user store.
|
/// Stable numeric id from the user store. NOTE (authkit#16 L-1): this is an
|
||||||
|
/// `int`, so it only round-trips numeric ids. A store keyed on UUIDs / other
|
||||||
|
/// non-numeric ids must not stuff them here — `requireUser` rejects a
|
||||||
|
/// non-numeric bundle id with 401. Carry such identities in `username` (or
|
||||||
|
/// extend this struct) instead.
|
||||||
|
int id{0};
|
||||||
std::string username;
|
std::string username;
|
||||||
std::string role; ///< Arbitrary string; policy decides what "admin"/"readonly" mean.
|
std::string role; ///< Arbitrary string; policy decides what "admin"/"readonly" mean.
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -23,7 +23,14 @@ class IAuthBackend {
|
||||||
public:
|
public:
|
||||||
virtual ~IAuthBackend() = default;
|
virtual ~IAuthBackend() = default;
|
||||||
|
|
||||||
/** @brief Look up an active session by its hashed token. */
|
/** @brief Look up an *active, non-expired* session by its hashed token.
|
||||||
|
*
|
||||||
|
* @warning Enforce expiry HERE (authkit#16 L-6): filter on the session's
|
||||||
|
* `expires_at` in this query and return `std::nullopt` for an
|
||||||
|
* expired row. The interceptor's periodic `deleteExpiredSessions`
|
||||||
|
* is best-effort garbage collection that only runs on request
|
||||||
|
* traffic — relying on it for expiry would leave a stale token
|
||||||
|
* valid until the next sweep (or indefinitely on an idle server). */
|
||||||
virtual std::optional<AuthPrincipal> resolveBySessionHash(const std::string& hash) = 0;
|
virtual std::optional<AuthPrincipal> resolveBySessionHash(const std::string& hash) = 0;
|
||||||
|
|
||||||
/** @brief Look up an API key by its hashed token; also touch `last_used_at`. */
|
/** @brief Look up an API key by its hashed token; also touch `last_used_at`. */
|
||||||
|
|
|
||||||
|
|
@ -31,7 +31,22 @@ inline AuthPrincipal requireUser(const std::shared_ptr<IncomingRequest>& request
|
||||||
OATPP_ASSERT_HTTP(id && role, Status::CODE_401, "Authentication required");
|
OATPP_ASSERT_HTTP(id && role, Status::CODE_401, "Authentication required");
|
||||||
|
|
||||||
AuthPrincipal p;
|
AuthPrincipal p;
|
||||||
p.id = std::stoi(std::string(*id));
|
// authkit#16 L-1: parse defensively. The bundle id is normally a decimal
|
||||||
|
// written by AuthInterceptor, but a non-numeric / out-of-range value (or a
|
||||||
|
// future principal id that isn't an int) must surface as a clean 401, not
|
||||||
|
// an uncaught std::invalid_argument/out_of_range escaping the endpoint as a
|
||||||
|
// 500. The OATPP_ASSERT_HTTP is kept OUTSIDE the try so its HttpError isn't
|
||||||
|
// swallowed by the catch.
|
||||||
|
bool idOk = false;
|
||||||
|
{
|
||||||
|
const std::string idStr(*id);
|
||||||
|
try {
|
||||||
|
std::size_t consumed = 0;
|
||||||
|
int parsed = std::stoi(idStr, &consumed);
|
||||||
|
if (consumed == idStr.size()) { p.id = parsed; idOk = true; }
|
||||||
|
} catch (...) { idOk = false; }
|
||||||
|
}
|
||||||
|
OATPP_ASSERT_HTTP(idOk, Status::CODE_401, "Authentication required");
|
||||||
p.role = std::string(*role);
|
p.role = std::string(*role);
|
||||||
p.username = username ? std::string(*username) : "";
|
p.username = username ? std::string(*username) : "";
|
||||||
return p;
|
return p;
|
||||||
|
|
|
||||||
|
|
@ -81,18 +81,44 @@ CREATE INDEX IF NOT EXISTS idx_audit_log_table_entity ON audit_log(table_name
|
||||||
private:
|
private:
|
||||||
std::shared_ptr<AuditLogDb> m_db;
|
std::shared_ptr<AuditLogDb> m_db;
|
||||||
|
|
||||||
/** @brief Fields to skip when computing UPDATE diffs — internal/metadata. */
|
/** @brief Fields to skip when computing UPDATE diffs — internal/metadata
|
||||||
|
* plus credentials. authkit#16 L-3: never copy a secret into the long-lived
|
||||||
|
* `audit_log.changed_fields` column (covers both snake_case and camelCase
|
||||||
|
* identifiers since the diff matches on the DTO's C++ field name). */
|
||||||
static inline const std::set<std::string> SKIP_FIELDS = {
|
static inline const std::set<std::string> SKIP_FIELDS = {
|
||||||
"id", "entity_id", "created_at", "updated_at", "valid_from"
|
"id", "entity_id", "created_at", "updated_at", "valid_from",
|
||||||
|
"password", "passwordHash", "password_hash",
|
||||||
|
"tlsCertDn", "tls_cert_dn",
|
||||||
|
"apiKey", "api_key", "token", "secret"
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/** @brief RFC 8259-compliant JSON string escaping. authkit#16 L-3: the
|
||||||
|
* previous version escaped only `\` and `"`, so a control character (e.g.
|
||||||
|
* a newline in a user-supplied name) produced invalid JSON in the audit
|
||||||
|
* trail and allowed newline/log injection into anything re-emitting the
|
||||||
|
* column. */
|
||||||
static std::string escapeJson(const std::string& s) {
|
static std::string escapeJson(const std::string& s) {
|
||||||
|
static const char* hex = "0123456789abcdef";
|
||||||
std::string out;
|
std::string out;
|
||||||
out.reserve(s.size());
|
out.reserve(s.size() + 8);
|
||||||
for (char c : s) {
|
for (unsigned char c : s) {
|
||||||
if (c == '\\') out += "\\\\";
|
switch (c) {
|
||||||
else if (c == '"') out += "\\\"";
|
case '\\': out += "\\\\"; break;
|
||||||
else out += c;
|
case '"': out += "\\\""; break;
|
||||||
|
case '\b': out += "\\b"; break;
|
||||||
|
case '\f': out += "\\f"; break;
|
||||||
|
case '\n': out += "\\n"; break;
|
||||||
|
case '\r': out += "\\r"; break;
|
||||||
|
case '\t': out += "\\t"; break;
|
||||||
|
default:
|
||||||
|
if (c < 0x20) {
|
||||||
|
out += "\\u00";
|
||||||
|
out += hex[(c >> 4) & 0xF];
|
||||||
|
out += hex[c & 0xF];
|
||||||
|
} else {
|
||||||
|
out += static_cast<char>(c);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return out;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -107,9 +107,14 @@ inline std::string send(
|
||||||
curl_easy_setopt(curl, CURLOPT_USERNAME, cfg.username.c_str());
|
curl_easy_setopt(curl, CURLOPT_USERNAME, cfg.username.c_str());
|
||||||
curl_easy_setopt(curl, CURLOPT_PASSWORD, cfg.password.c_str());
|
curl_easy_setopt(curl, CURLOPT_PASSWORD, cfg.password.c_str());
|
||||||
}
|
}
|
||||||
curl_easy_setopt(curl, CURLOPT_USE_SSL, CURLUSESSL_TRY);
|
// authkit#16 L-2: require TLS for non-loopback relays. CURLUSESSL_TRY would
|
||||||
// Allow self-signed certs on localhost relay — a common dev / pipe-transport setup.
|
// silently fall back to cleartext if STARTTLS is unavailable or stripped by
|
||||||
if (cfg.host == "localhost" || cfg.host == "127.0.0.1") {
|
// a MITM, leaking the SMTP AUTH credentials and message body. A local relay
|
||||||
|
// (localhost / pipe transport) stays on TRY with verification relaxed since
|
||||||
|
// there's no network hop to protect.
|
||||||
|
const bool loopbackRelay = (cfg.host == "localhost" || cfg.host == "127.0.0.1");
|
||||||
|
curl_easy_setopt(curl, CURLOPT_USE_SSL, loopbackRelay ? CURLUSESSL_TRY : CURLUSESSL_ALL);
|
||||||
|
if (loopbackRelay) {
|
||||||
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0L);
|
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0L);
|
||||||
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0L);
|
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0L);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -30,6 +30,14 @@ namespace oatpp_authkit::repo {
|
||||||
// two function templates. The primary templates are intentionally declared
|
// two function templates. The primary templates are intentionally declared
|
||||||
// without a definition: forgetting to register a field is a hard compile or
|
// without a definition: forgetting to register a field is a hard compile or
|
||||||
// link error rather than a runtime surprise.
|
// link error rather than a runtime surprise.
|
||||||
|
//
|
||||||
|
// SECURITY INVARIANT (authkit#16 L-8): column and table *identifiers* are
|
||||||
|
// emitted into SQL unparameterised (SQL placeholders can't bind identifiers).
|
||||||
|
// They come ONLY from these compile-time registrations / `Field<&Dto::mem>`,
|
||||||
|
// never from request data — so there is no injection vector. Never construct
|
||||||
|
// an `OrderBySpec`/`Field` column name from a runtime/user string; map a
|
||||||
|
// client sort field to a registered `Field` via an allowlist first. All
|
||||||
|
// *values* (eq/ne/in/like/...) are always bound as `?` parameters.
|
||||||
|
|
||||||
template <auto MemPtr>
|
template <auto MemPtr>
|
||||||
const char* columnName();
|
const char* columnName();
|
||||||
|
|
@ -69,6 +77,22 @@ inline BindValue toBindValue(const oatpp::String& v) {
|
||||||
return v ? BindValue{std::string(*v)} : BindValue{};
|
return v ? BindValue{std::string(*v)} : BindValue{};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Escape LIKE wildcards (`%`, `_`) and the escape char (`\`) in a
|
||||||
|
* user-supplied search term so they're matched literally (authkit#16
|
||||||
|
* L-8). Pair with the `LIKE ? ESCAPE '\'` clause emitted by
|
||||||
|
* `Field::likeContains` / `Field::likePrefix`.
|
||||||
|
*/
|
||||||
|
inline std::string likeEscape(const std::string& term) {
|
||||||
|
std::string out;
|
||||||
|
out.reserve(term.size() + 4);
|
||||||
|
for (char c : term) {
|
||||||
|
if (c == '\\' || c == '%' || c == '_') out.push_back('\\');
|
||||||
|
out.push_back(c);
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
// ─── AST nodes ──────────────────────────────────────────────────────────────
|
// ─── AST nodes ──────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
class AstNode {
|
class AstNode {
|
||||||
|
|
@ -122,6 +146,20 @@ public:
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/** @brief `col LIKE ? ESCAPE '\'` — the explicit ESCAPE makes a `\`-escaped
|
||||||
|
* pattern (see `likeEscape`) treat `%`/`_` literally. */
|
||||||
|
class LikeNode : public AstNode {
|
||||||
|
std::string col_;
|
||||||
|
BindValue val_;
|
||||||
|
public:
|
||||||
|
LikeNode(std::string c, BindValue v) : col_(std::move(c)), val_(std::move(v)) {}
|
||||||
|
void emit(std::ostringstream& sql,
|
||||||
|
std::vector<BindValue>& binds) const override {
|
||||||
|
sql << col_ << " LIKE ? ESCAPE '\\'";
|
||||||
|
binds.push_back(val_);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
class CombineNode : public AstNode {
|
class CombineNode : public AstNode {
|
||||||
const char* sep_;
|
const char* sep_;
|
||||||
std::vector<std::shared_ptr<AstNode>> children_;
|
std::vector<std::shared_ptr<AstNode>> children_;
|
||||||
|
|
@ -218,10 +256,28 @@ public:
|
||||||
return Predicate{std::make_shared<InNode>(column(), std::move(bs))};
|
return Predicate{std::make_shared<InNode>(column(), std::move(bs))};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** @brief Raw `col LIKE ?` with the pattern bound verbatim. The caller owns
|
||||||
|
* the `%`/`_` wildcards — only pass a TRUSTED pattern here. For a
|
||||||
|
* user-supplied search term use `likeContains` / `likePrefix` (which
|
||||||
|
* escape the metacharacters), or wrap it with `likeEscape`. */
|
||||||
Predicate like(const std::string& pat) const {
|
Predicate like(const std::string& pat) const {
|
||||||
return Predicate{std::make_shared<CompareNode>(
|
return Predicate{std::make_shared<CompareNode>(
|
||||||
column(), "LIKE", BindValue{pat})};
|
column(), "LIKE", BindValue{pat})};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** @brief Substring match of a user-supplied `term` with LIKE wildcards
|
||||||
|
* escaped — emits `col LIKE '%<escaped>%' ESCAPE '\'` (authkit#16 L-8). */
|
||||||
|
Predicate likeContains(const std::string& term) const {
|
||||||
|
return Predicate{std::make_shared<LikeNode>(
|
||||||
|
column(), BindValue{"%" + likeEscape(term) + "%"})};
|
||||||
|
}
|
||||||
|
|
||||||
|
/** @brief Prefix match of a user-supplied `term` with LIKE wildcards
|
||||||
|
* escaped — emits `col LIKE '<escaped>%' ESCAPE '\'`. */
|
||||||
|
Predicate likePrefix(const std::string& term) const {
|
||||||
|
return Predicate{std::make_shared<LikeNode>(
|
||||||
|
column(), BindValue{likeEscape(term) + "%"})};
|
||||||
|
}
|
||||||
Predicate isNull() const { return Predicate{std::make_shared<IsNullNode>(column(), true)}; }
|
Predicate isNull() const { return Predicate{std::make_shared<IsNullNode>(column(), true)}; }
|
||||||
Predicate isNotNull() const { return Predicate{std::make_shared<IsNullNode>(column(), false)}; }
|
Predicate isNotNull() const { return Predicate{std::make_shared<IsNullNode>(column(), false)}; }
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -293,10 +293,17 @@ private:
|
||||||
|
|
||||||
static IdGen defaultIdGen() {
|
static IdGen defaultIdGen() {
|
||||||
return [] {
|
return [] {
|
||||||
static thread_local std::mt19937_64 rng{std::random_device{}()};
|
// authkit#16 L-5: draw 128 bits straight from the platform CSPRNG
|
||||||
|
// (std::random_device → getrandom()/urandom on Linux) on every
|
||||||
|
// call. The old code seeded a mt19937_64 once from a single
|
||||||
|
// random_device sample, making the whole id stream predictable from
|
||||||
|
// observed outputs — a problem if a consumer ever treats entity_id
|
||||||
|
// as an unguessable handle. Consumers needing a hard guarantee can
|
||||||
|
// still inject their own IdGen.
|
||||||
|
static thread_local std::random_device rd;
|
||||||
char buf[33];
|
char buf[33];
|
||||||
std::snprintf(buf, sizeof(buf), "%016llx%016llx",
|
std::snprintf(buf, sizeof(buf), "%08x%08x%08x%08x",
|
||||||
(unsigned long long)rng(), (unsigned long long)rng());
|
(unsigned)rd(), (unsigned)rd(), (unsigned)rd(), (unsigned)rd());
|
||||||
return oatpp::String(buf);
|
return oatpp::String(buf);
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
|
||||||
36
include/oatpp-authkit/util/ConstantTime.hpp
Normal file
36
include/oatpp-authkit/util/ConstantTime.hpp
Normal file
|
|
@ -0,0 +1,36 @@
|
||||||
|
#ifndef OATPP_AUTHKIT_UTIL_CONSTANT_TIME_HPP
|
||||||
|
#define OATPP_AUTHKIT_UTIL_CONSTANT_TIME_HPP
|
||||||
|
|
||||||
|
// Constant-time comparison (authkit#16 L-7).
|
||||||
|
//
|
||||||
|
// The interceptor looks tokens up by hash in the store (effectively
|
||||||
|
// constant-time via an indexed equality), so it doesn't need this. But a
|
||||||
|
// consumer that ever compares a secret (token, HMAC, hash) in memory must not
|
||||||
|
// use std::string::operator== / memcmp — those short-circuit on the first
|
||||||
|
// mismatching byte and leak, via timing, how much of the secret was guessed.
|
||||||
|
|
||||||
|
#include <cstddef>
|
||||||
|
#include <string>
|
||||||
|
|
||||||
|
namespace oatpp_authkit {
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @brief Compare two byte strings without an early-exit on the first
|
||||||
|
* differing byte. The length difference is folded into the result, so
|
||||||
|
* unequal-length inputs still take time proportional to the longer one
|
||||||
|
* (the length of a fixed-size hash/token is not itself secret).
|
||||||
|
*/
|
||||||
|
inline bool constantTimeEquals(const std::string& a, const std::string& b) {
|
||||||
|
const std::size_t n = a.size() > b.size() ? a.size() : b.size();
|
||||||
|
volatile unsigned char diff = static_cast<unsigned char>(a.size() ^ b.size());
|
||||||
|
for (std::size_t i = 0; i < n; ++i) {
|
||||||
|
const unsigned char ca = (i < a.size()) ? static_cast<unsigned char>(a[i]) : 0;
|
||||||
|
const unsigned char cb = (i < b.size()) ? static_cast<unsigned char>(b[i]) : 0;
|
||||||
|
diff = static_cast<unsigned char>(diff | (ca ^ cb));
|
||||||
|
}
|
||||||
|
return diff == 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
} // namespace oatpp_authkit
|
||||||
|
|
||||||
|
#endif
|
||||||
|
|
@ -191,7 +191,15 @@ public:
|
||||||
{
|
{
|
||||||
socket.setListener(std::make_shared<Listener>());
|
socket.setListener(std::make_shared<Listener>());
|
||||||
|
|
||||||
if (!t_pendingAuth.has_value()) {
|
// authkit#16 L-4: consume the thread-local handoff exactly once, up
|
||||||
|
// front, and clear it unconditionally. If a prior connection's
|
||||||
|
// onAfterCreate ever failed to clear it (or oatpp reuses this worker
|
||||||
|
// thread), a leftover value must NOT attach to this socket — and our
|
||||||
|
// own value must not leak to the next connection on this thread.
|
||||||
|
std::optional<SocketInfo> pending = std::move(t_pendingAuth);
|
||||||
|
t_pendingAuth.reset();
|
||||||
|
|
||||||
|
if (!pending.has_value()) {
|
||||||
// Should not happen — WSController validates before handshake.
|
// Should not happen — WSController validates before handshake.
|
||||||
OATPP_LOGW("Hub", "WebSocket connected without auth context — closing");
|
OATPP_LOGW("Hub", "WebSocket connected without auth context — closing");
|
||||||
try { socket.sendClose(4001, "Unauthorized"); } catch (...) {}
|
try { socket.sendClose(4001, "Unauthorized"); } catch (...) {}
|
||||||
|
|
@ -204,14 +212,12 @@ public:
|
||||||
// #439: refuse extra connections beyond the cap rather than
|
// #439: refuse extra connections beyond the cap rather than
|
||||||
// allowing unbounded growth of s_sockets / presence maps.
|
// allowing unbounded growth of s_sockets / presence maps.
|
||||||
OATPP_LOGW("Hub", "socket cap %zu hit — rejecting", kMaxSockets);
|
OATPP_LOGW("Hub", "socket cap %zu hit — rejecting", kMaxSockets);
|
||||||
t_pendingAuth.reset();
|
|
||||||
try { socket.sendClose(1013, "Server Busy"); } catch (...) {}
|
try { socket.sendClose(1013, "Server Busy"); } catch (...) {}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
s_sockets[&socket] = std::move(*t_pendingAuth);
|
s_sockets[&socket] = std::move(*pending);
|
||||||
s_lastSeen[&socket] = std::chrono::steady_clock::now();
|
s_lastSeen[&socket] = std::chrono::steady_clock::now();
|
||||||
}
|
}
|
||||||
t_pendingAuth.reset();
|
|
||||||
|
|
||||||
OATPP_LOGD("Hub", "client connected: %s (total=%zu)",
|
OATPP_LOGD("Hub", "client connected: %s (total=%zu)",
|
||||||
s_sockets[&socket].username.c_str(), s_sockets.size());
|
s_sockets[&socket].username.c_str(), s_sockets.size());
|
||||||
|
|
|
||||||
|
|
@ -22,6 +22,10 @@ add_executable(test_origin_check test_origin_check.cpp)
|
||||||
target_link_libraries(test_origin_check PRIVATE oatpp::authkit oatpp::oatpp)
|
target_link_libraries(test_origin_check PRIVATE oatpp::authkit oatpp::oatpp)
|
||||||
add_test(NAME origin_check COMMAND test_origin_check)
|
add_test(NAME origin_check COMMAND test_origin_check)
|
||||||
|
|
||||||
|
add_executable(test_constant_time test_constant_time.cpp)
|
||||||
|
target_link_libraries(test_constant_time PRIVATE oatpp::authkit oatpp::oatpp)
|
||||||
|
add_test(NAME constant_time COMMAND test_constant_time)
|
||||||
|
|
||||||
add_executable(test_session_cookie test_session_cookie.cpp)
|
add_executable(test_session_cookie test_session_cookie.cpp)
|
||||||
target_link_libraries(test_session_cookie PRIVATE oatpp::authkit oatpp::oatpp)
|
target_link_libraries(test_session_cookie PRIVATE oatpp::authkit oatpp::oatpp)
|
||||||
add_test(NAME session_cookie COMMAND test_session_cookie)
|
add_test(NAME session_cookie COMMAND test_session_cookie)
|
||||||
|
|
|
||||||
44
test/test_constant_time.cpp
Normal file
44
test/test_constant_time.cpp
Normal file
|
|
@ -0,0 +1,44 @@
|
||||||
|
// Tests for oatpp-authkit/util/ConstantTime.hpp (authkit#16 L-7).
|
||||||
|
// Verifies functional correctness; timing-invariance is a property of the
|
||||||
|
// branch-free implementation, not asserted here.
|
||||||
|
|
||||||
|
#include "oatpp-authkit/util/ConstantTime.hpp"
|
||||||
|
|
||||||
|
#include <cstdio>
|
||||||
|
#include <string>
|
||||||
|
|
||||||
|
namespace {
|
||||||
|
|
||||||
|
int g_failures = 0;
|
||||||
|
#define REQUIRE(expr) do { \
|
||||||
|
if (!(expr)) { \
|
||||||
|
std::fprintf(stderr, "FAIL %s:%d %s\n", __FILE__, __LINE__, #expr); \
|
||||||
|
++g_failures; \
|
||||||
|
} \
|
||||||
|
} while (0)
|
||||||
|
|
||||||
|
using namespace oatpp_authkit;
|
||||||
|
|
||||||
|
void test_constant_time_equals() {
|
||||||
|
REQUIRE(constantTimeEquals("", ""));
|
||||||
|
REQUIRE(constantTimeEquals("abc", "abc"));
|
||||||
|
REQUIRE(constantTimeEquals(std::string(64, 'a'), std::string(64, 'a')));
|
||||||
|
|
||||||
|
REQUIRE(!constantTimeEquals("abc", "abd")); // differ at last byte
|
||||||
|
REQUIRE(!constantTimeEquals("abc", "xbc")); // differ at first byte
|
||||||
|
REQUIRE(!constantTimeEquals("abc", "ab")); // length mismatch (prefix)
|
||||||
|
REQUIRE(!constantTimeEquals("ab", "abc"));
|
||||||
|
REQUIRE(!constantTimeEquals("", "a"));
|
||||||
|
|
||||||
|
// Embedded NUL handled (string-length aware, not C-string).
|
||||||
|
REQUIRE(constantTimeEquals(std::string("a\0b", 3), std::string("a\0b", 3)));
|
||||||
|
REQUIRE(!constantTimeEquals(std::string("a\0b", 3), std::string("a\0c", 3)));
|
||||||
|
}
|
||||||
|
|
||||||
|
} // namespace
|
||||||
|
|
||||||
|
int main() {
|
||||||
|
test_constant_time_equals();
|
||||||
|
std::printf("%s (%d failures)\n", g_failures ? "FAIL" : "OK", g_failures);
|
||||||
|
return g_failures ? 1 : 0;
|
||||||
|
}
|
||||||
|
|
@ -148,6 +148,29 @@ void test_like_pattern_is_bound_not_interpolated() {
|
||||||
REQUIRE_EQ(std::get<std::string>(sql.binds[0]), std::string("Al%"));
|
REQUIRE_EQ(std::get<std::string>(sql.binds[0]), std::string("Al%"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void test_like_contains_escapes_wildcards() {
|
||||||
|
// authkit#16 L-8: a user term with %/_/\ must be matched literally via an
|
||||||
|
// explicit ESCAPE clause, not treated as wildcards.
|
||||||
|
auto sql = Query<MockQueryDto>()
|
||||||
|
.where(field<&MockQueryDto::name>().likeContains("50%_off\\x"))
|
||||||
|
.toSql();
|
||||||
|
REQUIRE_EQ(sql.text, std::string(
|
||||||
|
"SELECT * FROM mock_query WHERE name LIKE ? ESCAPE '\\'"));
|
||||||
|
REQUIRE_EQ(std::get<std::string>(sql.binds[0]),
|
||||||
|
std::string("%50\\%\\_off\\\\x%"));
|
||||||
|
|
||||||
|
auto pfx = Query<MockQueryDto>()
|
||||||
|
.where(field<&MockQueryDto::name>().likePrefix("a_b"))
|
||||||
|
.toSql();
|
||||||
|
REQUIRE_EQ(pfx.text, std::string(
|
||||||
|
"SELECT * FROM mock_query WHERE name LIKE ? ESCAPE '\\'"));
|
||||||
|
REQUIRE_EQ(std::get<std::string>(pfx.binds[0]), std::string("a\\_b%"));
|
||||||
|
|
||||||
|
// The bare likeEscape helper.
|
||||||
|
REQUIRE_EQ(likeEscape("100%_\\"), std::string("100\\%\\_\\\\"));
|
||||||
|
REQUIRE_EQ(likeEscape("plain"), std::string("plain"));
|
||||||
|
}
|
||||||
|
|
||||||
void test_is_null_and_is_not_null() {
|
void test_is_null_and_is_not_null() {
|
||||||
auto a = Query<MockQueryDto>()
|
auto a = Query<MockQueryDto>()
|
||||||
.where(field<&MockQueryDto::email>().isNull())
|
.where(field<&MockQueryDto::email>().isNull())
|
||||||
|
|
@ -208,6 +231,7 @@ int main() {
|
||||||
test_in_with_multiple_values();
|
test_in_with_multiple_values();
|
||||||
test_in_with_empty_list_is_always_false();
|
test_in_with_empty_list_is_always_false();
|
||||||
test_like_pattern_is_bound_not_interpolated();
|
test_like_pattern_is_bound_not_interpolated();
|
||||||
|
test_like_contains_escapes_wildcards();
|
||||||
test_is_null_and_is_not_null();
|
test_is_null_and_is_not_null();
|
||||||
test_not_negates_predicate();
|
test_not_negates_predicate();
|
||||||
test_order_by_and_limit_offset();
|
test_order_by_and_limit_offset();
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue