Skip to content

Commit

Permalink
Ensure GPO URLs contain the FQDN of the controller
Browse files Browse the repository at this point in the history
This is the main driver for the changes in the previous commits. The way
this behavior worked in the past is that we would use the URL returned
in the gPCFileSysPath field without any changes.

This introduced an inconsistency that went unobserved until recently.
Namely, we would get the list of GPOs using the FQDN of the domain
controller (e.g. adc.example.com), whereas the list of GPO URLs only
included the domain name (e.g. example.com). This meant that when
downloading the actual GPO data, libsmbclient would try to autodiscover
a domain controller from which to perform the download, given only the
domain name.

In some cases, especially complicated AD deployments with lots of DCs,
libsmbclient could autoresolve to an unhealthy DC (we take unhealthy to
mean any DC from which GPO files cannot be downloaded, regardless of
reason). This would fail the GPO download with a cryptic "invalid
argument" error.

Besides the chance of the above happening, autodiscovery also takes
longer as opposed to passing a valid DC FQDN to libsmbclient from the
start.

To fix this, we rewrite the GPO URL in the gPCFileSysPath field to
include the FQDN of the domain controller which essentially ensures the
DC we get the GPO list from, and the DC we download the GPO data from
are the same, minimizing the chance of mismatches like this occurring.

This has some drawbacks in the integration tests where we set up a real
SMB share and download from it, so we need to ensure the mocked server
URL is the actual SMB server.

Fixes #733 / UDENG-843
  • Loading branch information
GabrielNagy committed Oct 24, 2023
1 parent 5bed0ec commit 91a6cdd
Show file tree
Hide file tree
Showing 40 changed files with 76 additions and 64 deletions.
5 changes: 4 additions & 1 deletion cmd/adsysd/integration_tests/systemdaemons/system_daemons.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ def sssd_on_bus(bus: dbus.Bus):
False)
main_object.AddMethods("", [
("IsOnline", "", "b", "ret = True"),
("ActiveServer", "s", "s", 'ret = "adc.example.com"'),
# In real environments this is the FQDN of a domain controller
# For testing purposes we need to match the value to the underlying SMB
# server running on localhost
("ActiveServer", "s", "s", 'ret = "localhost:1446"'),
])

main_object.AddObject(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Active Directory:
Configuration: testdata/sssd-configs/sssd.conf-example.com
Cache: /tmp/sss_cache
Domain: example.com
Server FQDN: adc.example.com
Server FQDN: localhost:1446

Daemon:
Timeout after 30s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Active Directory:
Configuration: testdata/sssd-configs/sssd.conf-example.com
Cache: /tmp/sss_cache
Domain: example.com
Server FQDN: adc.example.com
Server FQDN: localhost:1446

Daemon:
Timeout after 30s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Active Directory:
Configuration: testdata/sssd-configs/sssd.conf-example.com
Cache: /tmp/sss_cache
Domain: example.com
Server FQDN: adc.example.com
Server FQDN: localhost:1446

Daemon:
Timeout after 30s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Active Directory:
Configuration: testdata/sssd-configs/sssd.conf-example.com
Cache: /tmp/sss_cache
Domain: example.com
Server FQDN: adc.example.com
Server FQDN: localhost:1446

Daemon:
Timeout after 30s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Active Directory:
Configuration: testdata/sssd-configs/sssd.conf-example.com
Cache: /tmp/sss_cache
Domain: example.com
Server FQDN: adc.example.com
Server FQDN: localhost:1446

Daemon:
Timeout after 30s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Active Directory:
Configuration: testdata/sssd-configs/sssd.conf-example.com
Cache: /tmp/sss_cache
Domain: example.com
Server FQDN: adc.example.com
Server FQDN: localhost:1446

Daemon:
Timeout after 30s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Active Directory:
Configuration: testdata/sssd-configs/sssd.conf-example.com
Cache: /tmp/sss_cache
Domain: example.com
Server FQDN: adc.example.com
Server FQDN: localhost:1446

Daemon:
Timeout after 30s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Active Directory:
Configuration: testdata/sssd-configs/sssd.conf-example.com_static-server
Cache: /tmp/sss_cache
Domain: example.com
Server FQDN: staticserver.example.com
Server FQDN: localhost:1446

Daemon:
Timeout after 30s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Active Directory:
Configuration: testdata/sssd-configs/sssd.conf-example.com
Cache: /tmp/sss_cache
Domain: example.com
Server FQDN: adc.example.com
Server FQDN: localhost:1446

Daemon:
Timeout after 30s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Active Directory:
Configuration: testdata/sssd-configs/sssd.conf-example.com
Cache: /tmp/sss_cache
Domain: example.com
Server FQDN: adc.example.com
Server FQDN: localhost:1446

Daemon:
Timeout after 30s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ domains = example.com

[domain/example.com]
ad_domain = example.com
ad_server = staticserver.example.com
ad_server = localhost:1446
11 changes: 10 additions & 1 deletion internal/ad/adsys-gpolist
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,17 @@ def main():
return ReturnCode.GPO_FAILED

for g in gpos:
print("%s\tsmb:%s" % (g[0], str(g[1]).replace("\\", "/")))
gpo_name = g[0]
gpo_path = parse_gpo_path(g[1], fqdn)
print("%s\t%s" % (gpo_name, gpo_path))

def parse_gpo_path(gpo_path, dc_fqdn):
''' Parse a GPO path to a SMB path with the appropriate DC FQDN '''
path = str(gpo_path).replace("\\", "/")
parts = path[2:].split("/")
parts[0] = dc_fqdn

return "smb://" +"/".join(parts)

if __name__ == "__main__":
exit(main())
2 changes: 1 addition & 1 deletion internal/ad/backends/winbind/mock/libwbclient_mock.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ wbcErr wbcLookupDomainController(const char *domain, uint32_t flags, struct wbcD

struct wbcDomainControllerInfo *dc = malloc(sizeof(struct wbcDomainControllerInfo));
// This is the only field used at the moment
dc->dc_name = "\\\\adcontroller.example.com";
dc->dc_name = "\\\\localhost:1446";
*dc_info = dc;
return WBC_ERR_SUCCESS;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
* Domain(): example.com
* ServerFQDN(): adcontroller.example.com
* ServerFQDN(): localhost:1446
* IsOnline(): false
* HostKrb5CCName(): /tmp/krb5cc_0
* DefaultDomainSuffix(): example.com
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
* Domain(): example.com
* ServerFQDN(): adcontroller.example.com
* ServerFQDN(): localhost:1446
* IsOnline ERROR(): could not get online status for domain "example.com": status code 2
* HostKrb5CCName(): /tmp/krb5cc_0
* DefaultDomainSuffix(): example.com
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
* Domain(): example.com
* ServerFQDN(): adcontroller.example.com
* ServerFQDN(): localhost:1446
* IsOnline(): true
* HostKrb5CCName ERROR(): could not get krb5 cached ticket for "UBUNTU$@EXAMPLE.COM": exit status 1:
EXIT 1 requested in mock
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
* Domain(): example.com
* ServerFQDN(): adcontroller.example.com
* ServerFQDN(): localhost:1446
* IsOnline(): true
* HostKrb5CCName(): /tmp/krb5cc_0
* DefaultDomainSuffix(): example.com
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
* Domain(): example.com
* ServerFQDN(): adcontroller.example.com
* ServerFQDN(): localhost:1446
* IsOnline(): true
* HostKrb5CCName(): /tmp/krb5cc_0
* DefaultDomainSuffix(): example.com
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
* Domain(): overridden.com
* ServerFQDN(): adcontroller.example.com
* ServerFQDN(): localhost:1446
* IsOnline(): true
* HostKrb5CCName(): /tmp/krb5cc_0
* DefaultDomainSuffix(): overridden.com
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
RnDDepBlockInheritance GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnDDepBlockInheritance_GPO
RnDDepBlockInheritance GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnDDepBlockInheritance_GPO
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Searching for account failed with: Failed to find account hostnameWithTruncatedLongName
ITDep1 GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/ITDep1_GPO
IT GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/IT_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
ITDep1 GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/ITDep1_GPO
IT GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/IT_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
6 changes: 3 additions & 3 deletions internal/ad/testdata/TestAdsysGPOList/golden/disabled_gpos
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
RnDDep3 GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnDDep3_GPO
RnD GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
RnDDep3 GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnDDep3_GPO
RnD GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
RnD GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
RnD GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
IT GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/IT_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
IT GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/IT_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
RnDDep2 Forced GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnDDep2_Forced_GPO
SubBlocked GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/SubBlocked_GPO
SubDep2BlockInheritance GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/SubDep2BlockInheritance_GPO
RnDDep2 Forced GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnDDep2_Forced_GPO
SubBlocked GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/SubBlocked_GPO
SubDep2BlockInheritance GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/SubDep2BlockInheritance_GPO
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
RnDDep2 Forced GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnDDep2_Forced_GPO
SubDep2ForcedPolicy Forced GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/SubDep2ForcedPolicy_Forced_GPO
RnDDep2 GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnDDep2_GPO
RnD GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
RnDDep2 Forced GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnDDep2_Forced_GPO
SubDep2ForcedPolicy Forced GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/SubDep2ForcedPolicy_Forced_GPO
RnDDep2 GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnDDep2_GPO
RnD GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
ITDep1 GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/ITDep1_GPO
IT GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/IT_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
ITDep1 GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/ITDep1_GPO
IT GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/IT_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
6 changes: 3 additions & 3 deletions internal/ad/testdata/TestAdsysGPOList/golden/machine_gpos
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
ITDep1 GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/ITDep1_GPO
IT GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/IT_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
ITDep1 GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/ITDep1_GPO
IT GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/IT_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
RnDDep1 GPO1 smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnDDep1_GPO1
RnDDep1 GPO2 smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnDDep1_GPO2
RnD GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
RnDDep1 GPO1 smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnDDep1_GPO1
RnDDep1 GPO2 smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnDDep1_GPO2
RnD GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
NogPOptions GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/NogPOptions_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
NogPOptions GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/NogPOptions_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
4 changes: 2 additions & 2 deletions internal/ad/testdata/TestAdsysGPOList/golden/return_hierarchy
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
RnD GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
RnD GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
RnD GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
RnD GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
RnD GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
RnD GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Failed to fetch gpo object with nTSecurityDescriptor RnDDep4_Security_descriptor_missing_GPO

RnD GPO smb://localhost:1445/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://localhost:1445/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}
RnD GPO smb://ldap_url/SYSVOL/gpoonly.com/Policies/RnD_GPO
Default Domain Policy smb://ldap_url/SYSVOL/gpoonly.com/Policies/{31B2F340-016D-11D2-945F-00C04FB984F9}

0 comments on commit 91a6cdd

Please sign in to comment.