[CDRIVER-3491] mongoc_client_reset should reset key vault client Created: 17/Jan/20 Updated: 10/Feb/20 Resolved: 10/Feb/20 |
|
| Status: | Closed |
| Project: | C Driver |
| Component/s: | libmongoc |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Kevin Albertson | Assignee: | Kevin Albertson |
| Resolution: | Won't Do | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Description |
|
A mongoc_client_t with automatic encryption (set via mongoc_client_enable_auto_encryption()) may use a separate inner mongoc_client_t passed via mongoc_auto_encryption_opts_set_keyvault_client to communicate with the key vault. Calling mongoc_client_reset should also reset the inner mongoc_client_t used to communicate with the key vault. Otherwise, the inner key vault client could end up reusing sessions from its own session pool. |
| Comments |
| Comment by Jeremy Mikola [ 31/Jan/20 ] |
|
andreas.braun: I'm thinking of ways that PHPC could reset its key vault client automatically when the application's main client is reset. Storing a reference to the key vault mongoc_client_t on a Manager wouldn't help, since our Session and Cursor classes have direct references to a mongoc_client_t instead of an indirect reference through the Manager object. That said, if our list of persisted clients also associates a PID with each client, perhaps we can tweak our reset logic to iterate on that list and reset anything that wasn't created by the current PID (would catch an otherwise unused key vault client) and hasn't already been reset by the current PID (to avoid double resets). |
| Comment by Kevin Albertson [ 30/Jan/20 ] |
|
For libmongoc, we already require the user to keep a handle to the key vault client so they can mongoc_client_destroy it later. Perhaps it is reasonable to just document that a user must mongoc_client_reset that client upon forking instead of resetting twice? But I'm less familiar with how this works in regards to PHP. If it would make the PHP implementation easier, that's reason enough to reset the key vault client. I don't think the risk of double resetting is an issue. If a user were to call mongoc_client_reset on the key vault client in direct succession, then no new resources would have been created in between the two resets, so there'd be no orphaned resources.
jmikola let me know if you think this is a change worthwhile to implement, or if it would help with PHP. Otherwise, I'd vote to make this a docs only change. |
| Comment by Jeremy Mikola [ 17/Jan/20 ] |
|
To clarify, we definitely don't want to reset the key vault client if it's identical to the application client (where FLE has been enabled). With respect to a separate key vault client, it is possible that the application would still have a reference to it and might call mongoc_client_reset() on both the application and key vault clients. Before implementing this ticket, I think we should consider possible side effects of resetting a key vault client twice. This is something I was careful not to do in |