Context
Surfaced during security review of spec 047 (Process-Wide State Cleanup).
Spec 047 changes pool ownership from singleton to per-catalog `unique_ptr`. As a side effect, when two ATTACHes use the same DSN with different aliases, the credentials (`Password`) are now held in two separate `MSSQLConnectionInfo` structs instead of one shared pool entry. This doubles the in-memory credential footprint for the same-DSN-multi-alias case, increasing attack surface for memory-disclosure vulnerabilities (heap inspection, core dumps).
The 047 review proposed credential zeroization as mitigation. On closer analysis, a better redesign is possible: eliminate the in-memory password copy entirely for the secret-backed auth path.
Problem analysis
Two credential paths exist today, treated identically:
(a) Connection-string path — `ATTACH 'Server=...;Password=xxx;...' AS db ...`
- Password lives in DuckDB attach-options string (catalog metadata, persists for lifetime of the catalog entry).
- Our `MSSQLConnectionInfo.password` is a copy of what DuckDB already holds.
- Zeroization here is symbolic: DuckDB still has the original, and we need it for transparent reconnect (pool replaces dead connections by reopening with the same credentials).
- No clean fix without DuckDB-level attach-API redesign. Out of scope for this work.
(b) Secret-backed path — `ATTACH '' AS db (TYPE mssql, SECRET s)`
- DuckDB Secret store holds the password (encrypted at rest in some Secret backends; in-RAM for `LocalSecretManager`).
- Today: at ATTACH time we fetch the secret once, copy password into `MSSQLConnectionInfo.password`, and from there it's identical to path (a).
- This copy is unnecessary. We could keep only `secret_name` and dereference the secret at LOGIN7 time. Password lives on stack for the duration of one login, then vanishes.
- Bonus: secret rotation works transparently — pool reconnects pick up the updated secret automatically without re-ATTACH.
Proposed design
Introduce a `CredentialProvider` abstraction in `tds::` namespace:
```cpp
class CredentialProvider {
public:
virtual ~CredentialProvider() = default;
// Fills password / token into the LOGIN7 packet. Implementations may
// materialize credentials transiently and zeroize after use.
virtual void FillLogin7(Login7Packet& packet, ClientContext& context) const = 0;
};
// Path (a): credentials supplied inline at ATTACH time.
class StaticCredentialProvider : public CredentialProvider {
std::string password_; // long-lived copy, matches DuckDB's catalog metadata
};
// Path (b): credentials looked up from DuckDB Secret store on demand.
class SecretBackedCredentialProvider : public CredentialProvider {
std::string secret_name_;
void FillLogin7(Login7Packet& packet, ClientContext& context) const override {
auto creds = SecretReader::Get(context, secret_name_); // transient
packet.SetCredentials(creds);
// creds destructor zeroizes
}
};
```
`MSSQLConnectionInfo` holds `std::unique_ptr` instead of a bare `password` string. Pool constructs the provider once at ATTACH time. `pool.Acquire()` for a fresh socket calls `provider->FillLogin7(packet, context)`.
Scope estimate
Non-trivial — touches multiple subsystems:
- Three auth strategies (SQL auth, FEDAUTH, integrated/Kerberos) — all must adopt the provider interface
- `Login7Packet` builder API changes
- Pool reconnect path: must hold a `ClientContext` reference (or weak reference) for secret lookups — interacts with DETACH lifecycle (race: secret read during shutdown)
- New tests: secret rotation propagates to next reconnect, secret deletion during pool lifetime, mixed-mode (different auths in same process)
- Documentation updates (Azure.md, Kerberos.md credential-handling sections)
Estimated 300–500 LOC implementation + interface redesign + ~10 tests. Justifies a dedicated spec.
Suggested spec id
Spec 049 — "Lazy credential materialization (secret-deref auth path)" — kicks off after 047 implementation lands.
What 047 will do regarding this
- Add an explicit non-goal in `spec.md` referencing this issue
- Add a note to FR-001 acknowledging the 2× credential footprint tradeoff for same-DSN-multi-alias case
References
Context
Surfaced during security review of spec 047 (Process-Wide State Cleanup).
Spec 047 changes pool ownership from singleton to per-catalog `unique_ptr`. As a side effect, when two ATTACHes use the same DSN with different aliases, the credentials (`Password`) are now held in two separate `MSSQLConnectionInfo` structs instead of one shared pool entry. This doubles the in-memory credential footprint for the same-DSN-multi-alias case, increasing attack surface for memory-disclosure vulnerabilities (heap inspection, core dumps).
The 047 review proposed credential zeroization as mitigation. On closer analysis, a better redesign is possible: eliminate the in-memory password copy entirely for the secret-backed auth path.
Problem analysis
Two credential paths exist today, treated identically:
(a) Connection-string path — `ATTACH 'Server=...;Password=xxx;...' AS db ...`
(b) Secret-backed path — `ATTACH '' AS db (TYPE mssql, SECRET s)`
Proposed design
Introduce a `CredentialProvider` abstraction in `tds::` namespace:
```cpp
class CredentialProvider {
public:
virtual ~CredentialProvider() = default;
// Fills password / token into the LOGIN7 packet. Implementations may
// materialize credentials transiently and zeroize after use.
virtual void FillLogin7(Login7Packet& packet, ClientContext& context) const = 0;
};
// Path (a): credentials supplied inline at ATTACH time.
class StaticCredentialProvider : public CredentialProvider {
std::string password_; // long-lived copy, matches DuckDB's catalog metadata
};
// Path (b): credentials looked up from DuckDB Secret store on demand.
class SecretBackedCredentialProvider : public CredentialProvider {
std::string secret_name_;
void FillLogin7(Login7Packet& packet, ClientContext& context) const override {
auto creds = SecretReader::Get(context, secret_name_); // transient
packet.SetCredentials(creds);
// creds destructor zeroizes
}
};
```
`MSSQLConnectionInfo` holds `std::unique_ptr` instead of a bare `password` string. Pool constructs the provider once at ATTACH time. `pool.Acquire()` for a fresh socket calls `provider->FillLogin7(packet, context)`.
Scope estimate
Non-trivial — touches multiple subsystems:
Estimated 300–500 LOC implementation + interface redesign + ~10 tests. Justifies a dedicated spec.
Suggested spec id
Spec 049 — "Lazy credential materialization (secret-deref auth path)" — kicks off after 047 implementation lands.
What 047 will do regarding this
References