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.
This bug was referenced in samba master: d277700710dc118f61065ed9e16e08e76820b66a b41691d0e546795bda994d94091b8e0a03ab96d6 e38b75a50f79c1d1ea2d7d4489896ca5aa16d9d9 49ac07e786df58b914ee85e2db773c0ba8d4e171 0d888f0c902ebd98cfb82d50ab8b8b3928341ee2 b95431ab2303eb258e37e88d8841f2fb79fc4af5 c2ab1f4696fa3f52918a126d0b37993a07f68bcb f9831259b9f6a49b9e1a7be75198d60374cdef2f d07641fc5a7d2fa323e6d6fe3223da3a6d682405 02c2a8c7b01d6412393423813b710c88b20fb97f f545142380151a626848dbae9ee746167f3299fa 7638abd38a13f9d2b5c769eb12c70eacf49b3806 e1c52ac05a9ff505d2e5eac2f1ece4e95844ee71 6b76bc7339addb14884c2d6ddb20c559c7fbe07d 15c86028a861139cee4560fe093c965ffc30eb13
This bug was referenced in samba v4-17-test: d277700710dc118f61065ed9e16e08e76820b66a b41691d0e546795bda994d94091b8e0a03ab96d6 e38b75a50f79c1d1ea2d7d4489896ca5aa16d9d9 49ac07e786df58b914ee85e2db773c0ba8d4e171 0d888f0c902ebd98cfb82d50ab8b8b3928341ee2 b95431ab2303eb258e37e88d8841f2fb79fc4af5 c2ab1f4696fa3f52918a126d0b37993a07f68bcb f9831259b9f6a49b9e1a7be75198d60374cdef2f d07641fc5a7d2fa323e6d6fe3223da3a6d682405 02c2a8c7b01d6412393423813b710c88b20fb97f f545142380151a626848dbae9ee746167f3299fa 7638abd38a13f9d2b5c769eb12c70eacf49b3806 e1c52ac05a9ff505d2e5eac2f1ece4e95844ee71 6b76bc7339addb14884c2d6ddb20c559c7fbe07d 15c86028a861139cee4560fe093c965ffc30eb13
This bug was referenced in samba v4-17-stable (Release samba-4.17.0rc1): d277700710dc118f61065ed9e16e08e76820b66a b41691d0e546795bda994d94091b8e0a03ab96d6 e38b75a50f79c1d1ea2d7d4489896ca5aa16d9d9 49ac07e786df58b914ee85e2db773c0ba8d4e171 0d888f0c902ebd98cfb82d50ab8b8b3928341ee2 b95431ab2303eb258e37e88d8841f2fb79fc4af5 c2ab1f4696fa3f52918a126d0b37993a07f68bcb f9831259b9f6a49b9e1a7be75198d60374cdef2f d07641fc5a7d2fa323e6d6fe3223da3a6d682405 02c2a8c7b01d6412393423813b710c88b20fb97f f545142380151a626848dbae9ee746167f3299fa 7638abd38a13f9d2b5c769eb12c70eacf49b3806 e1c52ac05a9ff505d2e5eac2f1ece4e95844ee71 6b76bc7339addb14884c2d6ddb20c559c7fbe07d 15c86028a861139cee4560fe093c965ffc30eb13