...
Summary Historically we haven't allowed x509 auth when the client certificate O/OU/DC matched the server certificate O/OU/DC. The intention of SERVER-45938 was to provide a parameter to disable the O/OU/DC check and disable it automatically when the clusterAuthMode is keyFile (because in this case we don't care about the cert clash as there would be no x509 cluster auth). The enforceUserClusterSeparation parameter was introduced to override this behaviour. Code was added to the createUser and authentication commands accordingly. The logic for the authentication command has been implemented - I believe - incorrectly. Currently, the O/OU/DC client/server validation only takes place if the clusterAuthMode does not allow X509 (i.e keyFile mode). For X509 clusterAuthMode, no validation is performed, only a log warning. We actually want the validation to be performed on the X509 code path only - with no validation when using keyFile. The validation logic for the createUser command and configuration check on process startup seem correct. Faulty logic: https://github.com/mongodb/mongo/blob/master/src/mongo/db/commands/authentication_commands.cpp#L232-L256 Motivation How does this affect the end user? Possible security implications due to faulty validation logic. Currently, when X509 clusterAuth is enabled, any client certificate that matches the server O/OU/DC will bypass validation and be granted __system privileges. How likely is it that this problem or use case will occur? Fairly rare.
mark.baker-munton commented on Fri, 10 Mar 2023 08:19:40 +0000: spencer.jackson@mongodb.com - gotcha, ok, that's a much bigger change then. I had forgotten about the arbiter problem. Thanks for the explanation, that's really useful to know! spencer.jackson@10gen.com commented on Thu, 9 Mar 2023 19:27:12 +0000: Hmm. An interesting question. That change would be possible, but a little harder than it might seem on its face. I believe isInternalClient() will return true if the remote party set a flag on their connection. However, these flags not particularly trustworthy. Consider that we're inspecting them here, before authentication has concluded. So, an adversary could just enable this flag to gain access here. That being said, it might be a nice guardrail, to keep misconfigured clients from accidentally attaining more privileges than they intended. Their administrators would then have to hash out a new subject name scheme with their CA administrators. Of course, on the other other hand, we have some applications based on MongoDB Drivers which need to connect as, and impersonate, cluster members. Automation, for example, has to pretend to be a cluster member in order to issue commands on Arbiters. Arbiters don't have data stores, so can't store user information, so can't remember the Automation Agent's password. It's a silly edge case, but Automation would need to be able to gain internal authorization rights. That would mean... probably adding a weird quasi secret Driver API for setting the internal client flag? That would be a project's worth of work to coordinate the upgrade across server, the Go Driver, and the Automation Agent. Also, that uassert should go in the keyFile auth path as well. We know the client is authenticating using MONGODB-X509, and their cert would be considered a cluster cert if the configuration was changed. We want to pre-emptively break, rather than allow the administrator to use certificates which could grant more permission in the future if the configuration flag was toggled. mark.baker-munton commented on Thu, 9 Mar 2023 08:29:39 +0000: Hi spencer.jackson@mongodb.com! Thanks for the detail, that makes a bit more sense now. I have one question though. You say there is no way to distinguish between cluster and user certificates but it looks like we have a way of determining whether the client connection itself is internal (the isInternalClient function). Would it be feasible to use the isInternalClient() function here at all? Something like this: // keyfile auth path if (!clusterAuthMode.allowsX509()) { ... authorizeExternalUser(); } else { // x509 auth path if (!isInternalClient()) { uassert(ErrorCodes::AuthenticationFailed, "The provided certificate can only be used for cluster authentication, not " "client authentication. The current configuration does not allow x.509 " "cluster authentication, check the --clusterAuthMode flag", !gEnforceUserClusterSeparation); } session->setAsClusterMember(); authorizationSession->grantInternalAuthorization(client); } spencer.jackson@10gen.com commented on Wed, 8 Mar 2023 22:20:47 +0000: Hi mark.baker-munton@mongodb.com! I don't think I'm seeing the issue you're describing. In the codeblock you've referenced, if (sslConfiguration.isClusterMember(clientName)) at the very top of the block is checking whether or not the presented client certificate's O/OU/DC components match the server's certificate. All the remaining code in the block is run under the assumption that the presented certificate matches the rules for being a cluster certificate. Thus, in the codepath where we are accepting keyFiles, we're checking whether enforceUserClusterSeparation was enabled. If the flag has not been enabled, then the administrator has deliberately relaxed the validation rules, so that they could use X.509 authentication with certificates that look like cluster certificates. Otherwise, they're trying to authenticate as a user which, if X.509 intracluster authentication was enabled, could be interpreted as a cluster user and so should be rejected so the administrator doesn't get comfortable with minting certificates like this for applications. In the alternative path, where we are accepting intracluster X.509 certificates, no further validation can be performed can be performed on the certificate. Given that our configuration tells us to accept a set of X.509 certificates as cluster certificates, and we've been presented with a member of that set, we must accept the certificate as a cluster member. We don't have a mechanism, other than the subject name, for distinguishing between cluster and user certificates.