[CDRIVER-3728] GSSAPI auth commands must not use implicit sessions Created: 26/Jun/20 Updated: 28/Oct/23 Resolved: 05/Aug/20 |
|
| Status: | Closed |
| Project: | C Driver |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 1.18.0, 1.17.3, 1.18.0-alpha |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Kevin Albertson | Assignee: | Unassigned |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | new-eng | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Description |
|
Authentication commands must not append a session ID per the driver session spec.
However, _mongoc_cluster_auth_node_cyrus and _mongoc_cluster_auth_node_sspi do not set prohibit_lsid. I believe they may still be appending a session ID unintentionally. |
| Comments |
| Comment by Pierre Mickael GONZALO [ 06/Aug/20 ] |
|
Alright thank you I'll know for the next time |
| Comment by Kevin Albertson [ 05/Aug/20 ] |
|
Thank you pierremickael.gonzalo@gmail.com! I authorized the patch build and merged the PR. Evergreen, MongoDB's continuous integration test platform. Evergreen tests on all platforms we support. Alas, it does require employee intervention to run user submitted pull requests. But, Travis tasks should still run on all PRs. And you can also run all tests locally. See https://github.com/mongodb/mongo-c-driver/blob/master/CONTRIBUTING.md#testing. |
| Comment by Kevin Albertson [ 05/Aug/20 ] |
|
PR: https://github.com/mongodb/mongo-c-driver/pull/675 |
| Comment by Pierre Mickael GONZALO [ 03/Aug/20 ] |
|
Hi,
But I do not know what it means if the error comes from my side or not. Maybe it is related to this cluster's test that we talked previously but you said that it is manually tested so could you give me some hint on what's going wrong ? |
| Comment by Kevin Albertson [ 31/Jul/20 ] |
No need. We can run manual tests on our end when reviewing the change since we already have Kerberos clusters set up to test against. |
| Comment by Pierre Mickael GONZALO [ 31/Jul/20 ] |
Alright never mind I've just misunderstood. So the manually test is intended to be tested by myself but how does it works ? Should I send you a screenshot of the server logs ?
Yes I am interrested in attempting the fix, I've actually already done but for now I can't create a remote branch so I can ask for a pull request. |
| Comment by Kevin Albertson [ 30/Jul/20 ] |
Yes. That is what I meant.
If you are referring to the field has_temp_session in mongoc_cmd_parts_t, that is used internally to determine whether an implicit session is applied. It's not clear to me how you'd access that from within a test.
Yes, if it is not reasonable to add automated tests, then we'd have to rely on manually looking at the sent command to check that the behavior is correct. That was the decision in — If you are interested in attempting the fix, I'd suggest proceeding with the change you proposed. Manually testing this will require authenticating against a Kerberos cluster. We have existing Kerberos clusters so we can validate the change. |
| Comment by Pierre Mickael GONZALO [ 30/Jul/20 ] |
|
Hi, so both functions calls "mongoc_cluster_run_command_private" and according to his description no APM callbacks are executed. I think is that you meant by: "And authentication commands are not capture in command monitoring". So you would like to know if is there an alternative to APM ? I would say to add an additionnal argument to the function that enables us to test it. I've seen the parts variable set his attribute "has_temp_server" to true in case the prohibit_lsid is set, maybe we can use it to complete our argument. Honestly I think it's not a good idea to do that way. |
| Comment by Kevin Albertson [ 29/Jul/20 ] |
|
Hello pierremickael.gonzalo@gmail.com, I agree, that seems like the most straightforward solution. If you would like to submit a PR with the change, we'd happily review it. Ideally, we would like to additionally add a test to validate the change. The C driver does not have much infrastructure for testing authentication conversations. The existing integration tests check that authentication works against MongoDB servers when expected, and fails when we do not. Since the session ID in the authentication command does not appear to affect whether authentication succeeds. And authentication commands are not capture in command monitoring. I would like to explore whether it is reasonable to add automated testing. If it is not feasible, we may need to rely on manual inspection of the commands. |
| Comment by Pierre Mickael GONZALO [ 29/Jul/20 ] |
|
Hi, I would set the prohibit_lsid attribute to true to the parts variable in both functions right after the call to mongoc_cmd_parts_init, did I miss something ? |