[SERVER-53064] Connections created using transient SSL params should not be stored in the global DBConnectionPool - security risk Created: 24/Nov/20 Updated: 29/Oct/23 Resolved: 21/Feb/21 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 5.0.0 |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Andrew Shuvalov (Inactive) | Assignee: | Andrew Shuvalov (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | pm-1791_milestone-P, pm-1791_non-cloud-blocking | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Backwards Compatibility: | Fully Compatible |
| Sprint: | Sharding 2021-01-25, Sharding 2021-02-22 |
| Participants: |
| Description |
|
The global DBConnectionPool keeps reusable connections. If a connection was created using transient SSL params, it should not be recycled into this pool, because it can be picked from some other code logic and that will be a security risk. |
| Comments |
| Comment by Githook User [ 20/Feb/21 ] |
|
Author: {'name': 'Andrew Shuvalov', 'email': 'andrew.shuvalov@mongodb.com', 'username': 'shuvalov-mdb'}Message: |
| Comment by Andrew Shuvalov (Inactive) [ 20/Jan/21 ] |
|
Hi spencer.jackson, just want to get your opinion for above. Please don't worry about the code, just the summary that there is a potential future danger as the code evolves, where putting some protective fences right now will require some more significant refactoring. I can either proceed with agreed priority or close this with "won't fix". |
| Comment by Andrew Shuvalov (Inactive) [ 20/Jan/21 ] |
The reasoning why I still think a fix is necessary: we are only one refactoring away from the security condition being triggered. Indeed we use DBClientConnection::connect, which is not using global pool. However, I don't see why a refactoring that would also make using the DBClientReplicaSet::connect is impossible. Nobody will ever remember this little detail and the security risk may be introduced by unrelated code changes years down the road. So I'm posting the fix. |
| Comment by Andrew Shuvalov (Inactive) [ 10/Dec/20 ] |
|
cheahuychou.mao said: "I don’t think this issue exists on the donor side since we use separate NetworkInterfaceTL/ConnectionPool for each migration. I also don't this issue exists on the recipient side. Here is the reasoning:
I will confirm it later when pending SSL related code is submitted, I still want to do an experiment but it requires changes that are not in the repo yet. |