[SERVER-44858] Speculative authentication in isMaster Created: 26/Nov/19 Updated: 29/Oct/23 Resolved: 12/Feb/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Networking, Security |
| Affects Version/s: | None |
| Fix Version/s: | 4.3.4 |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Spencer Jackson | Assignee: | Sara Golemon |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Major Change | ||||||||
| Sprint: | Security 2019-12-02, Security 2019-12-16, Security 2019-01-13, Security 2019-01-27, Security 2020-02-10, Security 2020-02-24 | ||||||||
| Participants: | |||||||||
| Description |
|
We should investigate what work needs to be done to accept and make speculative authentication attempts in isMaster. |
| Comments |
| Comment by Githook User [ 12/Feb/20 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Author: {'name': 'Sara Golemon', 'username': 'sgolemon', 'email': 'sara.golemon@mongodb.com'}Message: create mode 100644 jstests/auth/speculative-auth-replset.js | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sara Golemon [ 06/Feb/20 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
jmikola I'll paste the conversation examples I gave john.stewart last night: For MongoDB-X509
For SCRAM-SHA-256 (or SCRAM-SHA-1, just replace the names)
Essentially, the `speculativeAuthenticate` field takes whole command objects restricted to just `authenticate` and `saslStart` commands. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Jeremy Mikola [ 06/Feb/20 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
spencer.jackson: Can you elaborate on what you meant by that? Was that assuming that we would use speculativeSaslStart as the isMaster verb instead of speculativeAuthenticate as sara.golemon suggested in her last reply? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Spencer Jackson [ 05/Feb/20 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I think this proposal makes sense, with an amendment to the schema in #2 as we discussed in Slack. In order to provide multiple per-mechanism metrics, as well as mechanism agnostic metrics, we decided to put "speculativeAuthenticate" under a per-mech object. The resulting schema looks like:
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sara Golemon [ 05/Feb/20 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
If we're going to do it, then I'd rather update the diff we have in flight. As you said, it actually does simplify a few things for us as well. Just to put it in writing: 1/ Use "speculativeAuthenticate" as the only isMaster verb. As a name, this is more generalized than "speculativeSaslStart". 2/ Combine authCounters to be a single map under "security.authentication.speculativeAuthenticate". The mechanism effectively tells us if it's {authenticate:1, ...} or {saslStart: 1, ...} Then everything involving disallowing them to be used in parallel goes. The logic of "did we try to speculate?" gets a little simpler in some ways, a little harder in other (we still need to know if it was SASL or not, but the presence of a SaslClientSession will tell us that). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Spencer Jackson [ 05/Feb/20 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
jmikola sara.golemon, yeah... this request is reasonable. It even may deduplicate some of the field parsing logic. The only real downside I can see offhand is that MONGODB-X509 effectively becomes a SASL mechanism, but that's fine. sara.golemon, should we do it in the review we have active, or do you feel it would be easier as a followon? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Sara Golemon [ 05/Feb/20 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I would say we could still reconsider the API. There's certainly nothing about the current server implementation which would preclude it (well, there's one thing, but it can be refactored around). spencer.jackson What do you think of jmikola's request here? As for the fix version, yeah that's wrong, I'll fix it. We fully expect this to land in 4.4 :o) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment by Jeremy Mikola [ 05/Feb/20 ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
sara.golemon: shane.harvey raised a good question in mongodb/specifications#728 for SPEC-1552 asking about whether the server team considered consolidating speculativeSaslStart and speculativeAuthenticate into a single isMaster option that took the entire auth command. It seems like such a design would simplify the API for drivers. Is that something that can still be considered before this feature is implemented, or has the ship sailed? Also, I noticed that the fixVersion is currently "Backlog". Is this feature still expected to land in 4.4 or will it likely be pushed out? cc: john.stewart |