The e Validated-DNS-Host-Name right, which should apply only apply to objectclass=computer, per MS-ADTS 3.1.1.5.3.1.1.2 dNSHostName https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-adts/5c578b15-d619-408d-ba17-380714b89fd1 Is not implemented in Samba. It appears that any value might be able to be written over NETLOGON via dcesrv_netr_LogonGetDomainInfo rather than being put via this right. This validated update would restrict values to <samAccountName-without-$>.<dnsdomainname>. This matters as we will be linking a trailing $ to objectclass=computer and this helps avoid creation of SPN values that collide with other, possibly privileged, hosts.
A top level bug 14564 / CVE-2020-25722 will be used for these related issues.
https://research.ifcr.dk/certifried-active-directory-domain-privilege-escalation-cve-2022-26923-9e098fe298f4?gi=79eaa17b1dc8 says that now: Furthermore, the “Validated write to DNS host name” permission now only allows setting a dNSHostName attribute that matches the SAM Account Name of the account. We should catch up and implement this before it bites us.
Created attachment 17315 [details] WIP patch for master A proposed patch with tests.
Comment on attachment 17315 [details] WIP patch for master The next step needed is to handle the modification via NETLOGON, to ensure it operates via this validated write, not SYSTEM.
*** Bug 14885 has been marked as a duplicate of this bug. ***
Because the update in dcesrv_netr_LogonGetDomainInfo() sets an SPN that will conflict with the primary SPN of any legitimate service, I think we dodge a security hole here, just. It then becomes more like a littering issue, and we have other places (DNS records) where garbage data can be written. Perhaps some auditing tool is using dnsHostName, but Samba doesn't use it much, other than to control SPN values. So I think we should, over the next day or so, think about this and potentially remove the embargo. If do do that we should propose this as a public patch and go though the normal process (unless we come up with something worse of course). I seek other views!
Created attachment 17333 [details] proposed patch for master v2
After some discussion around the office, it is hard to know what the impact of this would be on individual deployments that might rely on dnsHostName so we will do the full CVE and security release thing here.
Note that the patchset from https://bugzilla.samba.org/show_bug.cgi?id=15008 must be applied prior to this one to avoid segfaults.
Comment on attachment 17333 [details] proposed patch for master v2 [PATCH 02/14] tests/py_credentials: Add tests for setting dNSHostName with LogonGetDomainInfo() + # Ensure we can't use LogonGetDomainInfo to update dNSHostName to an + # invalid value, even with rights. Can this test be split in two to check these rights (WP and validated write) individually? [PATCH 03/14] dsdb: Implement validated dNSHostName write This in the acl_check_dns_host_name() looks like a fail open, which seems bad to me: + + if (oc_el == NULL) { + /* Not a computer or server, so no need to validate. */ + talloc_free(tmp_ctx); + return LDB_SUCCESS; + } + } I'm worried we might be in the 'add' case and have yet a further subclass of computer (eg the attacker controls search_res) or just that for some reason this right is applied on some other type of object. [PATCH 05/14] dsdb/modules/acl: Handle VALIDATED_DNS_HOSTNAME_SPN_WRITE control Some of us don't really like boolean parameters, and I think in this case the complaint is justfied. I think pass validated_write as a typed paramter called implicit_validated_write_control (and I think we should rename the control to be FORCE_ALLOW_VALIDATED_DNS_HOSTNAME_SPN_WRITE). [PATCH 13/14] s4:rpc_server/common: Add dcesrv_samdb_connect_session_info() Ensure that both session_info values are overwritten, we don't want the 'transport' session info of anonymous in the logs, via audit_session_info + if (session_info != NULL) { + ldb_session_info = session_info; audit_session_info = user_session_info;
(In reply to Andrew Bartlett from comment #8) Given the constraints enforced in the NETLOGON server, thankfully there is less opportunity for abuse than I had feared, but the dnsHostName suffix can be changed freely.
Removing embargo. The constraints in comment 11 mean this isn't worth doing the full security dance for, we should just do a normal public bug and MR process for this. We keep the CVE tag so that those concerned about the low-severity issue can track it, and in case further concerns are found later.