From 45d326c257019eead0aa9570a7f68d0c8d8bd8ea Mon Sep 17 00:00:00 2001 From: Gabriel Nagy Date: Mon, 31 Jul 2023 22:45:30 +0300 Subject: [PATCH 1/8] Infer needed directories from given state directory Instead of having defaults for all needed directories in the certificate Python script, only make the state directory overridable since the others are descendants of it. This will ease integration testing as we will only need to expose the state directory and the global trust directory. --- internal/policies/certificate/cert-autoenroll | 18 ++++++------------ .../certificate/cert-autoenroll_test.go | 16 +++++++--------- internal/policies/certificate/certificate.go | 10 +++++----- .../enroll_with_empty_advanced_configuration | 8 ++++---- .../golden/enroll_with_simple_configuration | 8 ++++---- ...with_simple_configuration_and_debug_enabled | 8 ++++---- .../enroll_with_valid_advanced_configuration | 8 ++++---- .../TestCertAutoenrollScript/golden/unenroll | 2 +- .../golden/computer,_configured_to_enroll | 2 +- ...onfigured_to_enroll,_advanced_configuration | 2 +- .../golden/computer,_configured_to_unenroll | 2 +- .../computer,_no_entries,_samba_cache_present | 2 +- 12 files changed, 39 insertions(+), 47 deletions(-) diff --git a/internal/policies/certificate/cert-autoenroll b/internal/policies/certificate/cert-autoenroll index 4c2429a6d..f2b4582df 100755 --- a/internal/policies/certificate/cert-autoenroll +++ b/internal/policies/certificate/cert-autoenroll @@ -40,15 +40,9 @@ def main(): parser.add_argument('--policy_servers_json', type=str, help='GPO entries for advanced configuration of the policy servers. \ Must be in JSON format.') - parser.add_argument('--samba_cache_dir', type=str, - default='/var/lib/adsys/samba', - help='Directory to store GPO Samba cache in.') - parser.add_argument('--private_dir', type=str, - default='/var/lib/adsys/private/certs', - help='Directory to store private keys in.') - parser.add_argument('--trust_dir', type=str, - default='/var/lib/adsys/certs', - help='Directory to store trusted certificates in.') + parser.add_argument('--state_dir', type=str, + default='/var/lib/adsys', + help='Directory to store all certificate-related files in.') parser.add_argument('--global_trust_dir', type=str, default='/usr/local/share/ca-certificates', help='Directory to symlink root CA certificates to.') @@ -57,9 +51,9 @@ def main(): args = parser.parse_args() - samba_cache_dir = args.samba_cache_dir - trust_dir = args.trust_dir - private_dir = args.private_dir + samba_cache_dir = os.path.join(args.state_dir, 'samba') + trust_dir = os.path.join(args.state_dir, 'certs') + private_dir = os.path.join(args.state_dir, 'private', 'certs') global_trust_dir = args.global_trust_dir # Create needed directories if they don't exist diff --git a/internal/policies/certificate/cert-autoenroll_test.go b/internal/policies/certificate/cert-autoenroll_test.go index 8f7bbc94a..1bd2f03d8 100644 --- a/internal/policies/certificate/cert-autoenroll_test.go +++ b/internal/policies/certificate/cert-autoenroll_test.go @@ -106,17 +106,15 @@ func TestCertAutoenrollScript(t *testing.T) { tc := tc name := name t.Run(name, func(t *testing.T) { - tmpdir := t.TempDir() - stateDir := filepath.Join(tmpdir, "state") - privateDir := filepath.Join(tmpdir, "private") - trustDir := filepath.Join(tmpdir, "trust") - globalTrustDir := filepath.Join(tmpdir, "ca-certificates") + stateDir := t.TempDir() + sambaCacheDir := filepath.Join(stateDir, "samba") + globalTrustDir := filepath.Join(stateDir, "ca-certificates") if tc.readOnlyPath { - testutils.MakeReadOnly(t, tmpdir) + testutils.MakeReadOnly(t, stateDir) } - args := append(tc.args, "--samba_cache_dir", stateDir, "--private_dir", privateDir, "--trust_dir", trustDir, "--global_trust_dir", globalTrustDir) + args := append(tc.args, "--state_dir", stateDir, "--global_trust_dir", globalTrustDir) // #nosec G204: we control the command line name and only change it for tests cmd := exec.Command(certAutoenrollCmd, args...) @@ -131,12 +129,12 @@ func TestCertAutoenrollScript(t *testing.T) { } require.NoErrorf(t, err, "cert-autoenroll should have exited successfully: %s", string(out)) - got := strings.ReplaceAll(string(out), tmpdir, "#TMPDIR#") + got := strings.ReplaceAll(string(out), stateDir, "#STATEDIR#") want := testutils.LoadWithUpdateFromGolden(t, got) require.Equal(t, want, got, "Unexpected output from cert-autoenroll script") if slices.Contains(tc.args, "unenroll") { - require.NoDirExists(t, filepath.Join(stateDir, "samba"), "Samba cache directory should have been removed on unenroll") + require.NoDirExists(t, sambaCacheDir, "Samba cache directory should have been removed on unenroll") } }) } diff --git a/internal/policies/certificate/certificate.go b/internal/policies/certificate/certificate.go index 27241509e..41518bc50 100644 --- a/internal/policies/certificate/certificate.go +++ b/internal/policies/certificate/certificate.go @@ -42,7 +42,7 @@ import ( // the policy in ApplyPolicy. type Manager struct { domain string - sambaCacheDir string + stateDir string krb5CacheDir string vendorPythonDir string certEnrollCmd []string @@ -131,7 +131,7 @@ func New(domain string, opts ...Option) *Manager { return &Manager{ domain: domain, - sambaCacheDir: filepath.Join(args.stateDir, "samba"), + stateDir: args.stateDir, krb5CacheDir: filepath.Join(args.runDir, "krb5cc"), vendorPythonDir: filepath.Join(args.shareDir, "python"), certEnrollCmd: args.certAutoenrollCmd, @@ -158,12 +158,12 @@ func (m *Manager) ApplyPolicy(ctx context.Context, objectName string, isComputer idx := slices.IndexFunc(entries, func(e entry.Entry) bool { return e.Key == "autoenroll" }) if idx == -1 { // If the Samba cache directory doesn't exist, we don't have anything to unenroll - if _, err := os.Stat(m.sambaCacheDir); err != nil && os.IsNotExist(err) { + if _, err := os.Stat(filepath.Join(m.stateDir, "samba")); err != nil && os.IsNotExist(err) { return nil } log.Debug(ctx, "Certificate autoenrollment is not configured, unenrolling machine") - if err := m.runScript(ctx, "unenroll", objectName, "--samba_cache_dir", m.sambaCacheDir); err != nil { + if err := m.runScript(ctx, "unenroll", objectName, "--state_dir", m.stateDir); err != nil { return err } @@ -217,7 +217,7 @@ func (m *Manager) ApplyPolicy(ctx context.Context, objectName string, isComputer if err := m.runScript(ctx, action, objectName, "--policy_servers_json", string(jsonGPOData), - "--samba_cache_dir", m.sambaCacheDir, + "--state_dir", m.stateDir, ); err != nil { return err } diff --git a/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_empty_advanced_configuration b/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_empty_advanced_configuration index d64213e44..a8b5474f6 100644 --- a/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_empty_advanced_configuration +++ b/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_empty_advanced_configuration @@ -2,10 +2,10 @@ Loading smb.conf [global] realm = example.com -Loading state file: #TMPDIR#/state/cert_gpo_state_keypress.tdb +Loading state file: #STATEDIR#/samba/cert_gpo_state_keypress.tdb Enroll called guid: adsys-cert-autoenroll-keypress -trust_dir: #TMPDIR#/trust; mode: 0o40755 -private_dir: #TMPDIR#/private; mode: 0o40700 -global_trust_dir: #TMPDIR#/ca-certificates; mode: 0o40755 +trust_dir: #STATEDIR#/certs; mode: 0o40755 +private_dir: #STATEDIR#/private/certs; mode: 0o40700 +global_trust_dir: #STATEDIR#/ca-certificates; mode: 0o40755 diff --git a/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_simple_configuration b/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_simple_configuration index d64213e44..a8b5474f6 100644 --- a/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_simple_configuration +++ b/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_simple_configuration @@ -2,10 +2,10 @@ Loading smb.conf [global] realm = example.com -Loading state file: #TMPDIR#/state/cert_gpo_state_keypress.tdb +Loading state file: #STATEDIR#/samba/cert_gpo_state_keypress.tdb Enroll called guid: adsys-cert-autoenroll-keypress -trust_dir: #TMPDIR#/trust; mode: 0o40755 -private_dir: #TMPDIR#/private; mode: 0o40700 -global_trust_dir: #TMPDIR#/ca-certificates; mode: 0o40755 +trust_dir: #STATEDIR#/certs; mode: 0o40755 +private_dir: #STATEDIR#/private/certs; mode: 0o40700 +global_trust_dir: #STATEDIR#/ca-certificates; mode: 0o40755 diff --git a/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_simple_configuration_and_debug_enabled b/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_simple_configuration_and_debug_enabled index 3876ae0a7..d0ed58399 100644 --- a/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_simple_configuration_and_debug_enabled +++ b/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_simple_configuration_and_debug_enabled @@ -3,10 +3,10 @@ Loading smb.conf realm = example.com log level = 10 -Loading state file: #TMPDIR#/state/cert_gpo_state_keypress.tdb +Loading state file: #STATEDIR#/samba/cert_gpo_state_keypress.tdb Enroll called guid: adsys-cert-autoenroll-keypress -trust_dir: #TMPDIR#/trust; mode: 0o40755 -private_dir: #TMPDIR#/private; mode: 0o40700 -global_trust_dir: #TMPDIR#/ca-certificates; mode: 0o40755 +trust_dir: #STATEDIR#/certs; mode: 0o40755 +private_dir: #STATEDIR#/private/certs; mode: 0o40700 +global_trust_dir: #STATEDIR#/ca-certificates; mode: 0o40755 diff --git a/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_valid_advanced_configuration b/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_valid_advanced_configuration index f2d1af1de..daa653c6f 100644 --- a/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_valid_advanced_configuration +++ b/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/enroll_with_valid_advanced_configuration @@ -2,13 +2,13 @@ Loading smb.conf [global] realm = example.com -Loading state file: #TMPDIR#/state/cert_gpo_state_keypress.tdb +Loading state file: #STATEDIR#/samba/cert_gpo_state_keypress.tdb Enroll called guid: adsys-cert-autoenroll-keypress -trust_dir: #TMPDIR#/trust; mode: 0o40755 -private_dir: #TMPDIR#/private; mode: 0o40700 -global_trust_dir: #TMPDIR#/ca-certificates; mode: 0o40755 +trust_dir: #STATEDIR#/certs; mode: 0o40755 +private_dir: #STATEDIR#/private/certs; mode: 0o40700 +global_trust_dir: #STATEDIR#/ca-certificates; mode: 0o40755 entries: keyname: Software\Policies\Microsoft\Cryptography\PolicyServers\37c9dc30f207f27f61a2f7c3aed598a6e2920b54 diff --git a/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/unenroll b/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/unenroll index 8b44d2e33..ff5415b5f 100644 --- a/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/unenroll +++ b/internal/policies/certificate/testdata/TestCertAutoenrollScript/golden/unenroll @@ -2,7 +2,7 @@ Loading smb.conf [global] realm = example.com -Loading state file: #TMPDIR#/state/cert_gpo_state_keypress.tdb +Loading state file: #STATEDIR#/samba/cert_gpo_state_keypress.tdb Unenroll called guid: adsys-cert-autoenroll-keypress remove: ['ZXhhbXBsZS1DQQ=='] diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll index 1dbbbe06a..b53aaaf88 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll @@ -1,3 +1,3 @@ -enroll keypress example.com --policy_servers_json null --samba_cache_dir #TMPDIR#/statedir/samba +enroll keypress example.com --policy_servers_json null --state_dir #TMPDIR#/statedir KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress PYTHONPATH=#TMPDIR#/sharedir/python diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration index 7af6802c2..ebedab266 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration @@ -1,3 +1,3 @@ -enroll keypress example.com --policy_servers_json [{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"AuthFlags","data":2,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"Cost","data":2147483645,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"Flags","data":20,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"FriendlyName","data":"ActiveDirectoryEnrollmentPolicy","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"PolicyID","data":"{A5E9BF57-71C6-443A-B7FC-79EFA6F73EBD}","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"URL","data":"LDAP:","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers","valuename":"Flags","data":0,"type":4}] --samba_cache_dir #TMPDIR#/statedir/samba +enroll keypress example.com --policy_servers_json [{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"AuthFlags","data":2,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"Cost","data":2147483645,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"Flags","data":20,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"FriendlyName","data":"ActiveDirectoryEnrollmentPolicy","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"PolicyID","data":"{A5E9BF57-71C6-443A-B7FC-79EFA6F73EBD}","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"URL","data":"LDAP:","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers","valuename":"Flags","data":0,"type":4}] --state_dir #TMPDIR#/statedir KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress PYTHONPATH=#TMPDIR#/sharedir/python diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll index 5d4686e66..87bdd0251 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll @@ -1,3 +1,3 @@ -unenroll keypress example.com --policy_servers_json null --samba_cache_dir #TMPDIR#/statedir/samba +unenroll keypress example.com --policy_servers_json null --state_dir #TMPDIR#/statedir KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress PYTHONPATH=#TMPDIR#/sharedir/python diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present index e877e31f7..c9291a660 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present @@ -1,3 +1,3 @@ -unenroll keypress example.com --samba_cache_dir #TMPDIR#/statedir/samba +unenroll keypress example.com --state_dir #TMPDIR#/statedir KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress PYTHONPATH=#TMPDIR#/sharedir/python From ee1209d980f063d419655882e57f74ee9a944e94 Mon Sep 17 00:00:00 2001 From: Gabriel Nagy Date: Mon, 31 Jul 2023 22:57:12 +0300 Subject: [PATCH 2/8] Make global trust store path overridable --- internal/consts/consts.go | 2 + internal/policies/certificate/certificate.go | 20 ++++--- .../golden/computer,_configured_to_enroll | 2 +- ...nfigured_to_enroll,_advanced_configuration | 2 +- .../golden/computer,_configured_to_unenroll | 2 +- .../computer,_no_entries,_samba_cache_present | 2 +- internal/policies/manager.go | 54 +++++++++++-------- 7 files changed, 53 insertions(+), 31 deletions(-) diff --git a/internal/consts/consts.go b/internal/consts/consts.go index e53cb0017..c5b73097c 100644 --- a/internal/consts/consts.go +++ b/internal/consts/consts.go @@ -58,6 +58,8 @@ const ( DefaultApparmorDir = "/etc/apparmor.d/adsys" // DefaultSystemUnitDir is the default directory for systemd unit files. DefaultSystemUnitDir = "/etc/systemd/system" + // DefaultGlobalTrustDir is the default directory for the global trust store. + DefaultGlobalTrustDir = "/usr/local/share/ca-certificates" ) // SSSD related properties. diff --git a/internal/policies/certificate/certificate.go b/internal/policies/certificate/certificate.go index 41518bc50..5c6d06284 100644 --- a/internal/policies/certificate/certificate.go +++ b/internal/policies/certificate/certificate.go @@ -45,6 +45,7 @@ type Manager struct { stateDir string krb5CacheDir string vendorPythonDir string + globalTrustDir string certEnrollCmd []string mu sync.Mutex // Prevents multiple instances of the certificate manager from running in parallel @@ -81,6 +82,7 @@ type options struct { stateDir string runDir string shareDir string + globalTrustDir string certAutoenrollCmd []string } @@ -108,6 +110,13 @@ func WithShareDir(p string) func(*options) { } } +// WithGlobalTrustDir overrides the default global trust store directory. +func WithGlobalTrustDir(p string) func(*options) { + return func(a *options) { + a.globalTrustDir = p + } +} + // WithCertAutoenrollCmd overrides the default certificate autoenroll command. func WithCertAutoenrollCmd(cmd []string) func(*options) { return func(a *options) { @@ -122,6 +131,7 @@ func New(domain string, opts ...Option) *Manager { stateDir: consts.DefaultStateDir, runDir: consts.DefaultRunDir, shareDir: consts.DefaultShareDir, + globalTrustDir: consts.DefaultGlobalTrustDir, certAutoenrollCmd: []string{"python3", "-c", CertEnrollCode}, } // applied options @@ -134,6 +144,7 @@ func New(domain string, opts ...Option) *Manager { stateDir: args.stateDir, krb5CacheDir: filepath.Join(args.runDir, "krb5cc"), vendorPythonDir: filepath.Join(args.shareDir, "python"), + globalTrustDir: args.globalTrustDir, certEnrollCmd: args.certAutoenrollCmd, } } @@ -163,7 +174,7 @@ func (m *Manager) ApplyPolicy(ctx context.Context, objectName string, isComputer } log.Debug(ctx, "Certificate autoenrollment is not configured, unenrolling machine") - if err := m.runScript(ctx, "unenroll", objectName, "--state_dir", m.stateDir); err != nil { + if err := m.runScript(ctx, "unenroll", objectName); err != nil { return err } @@ -215,10 +226,7 @@ func (m *Manager) ApplyPolicy(ctx context.Context, objectName string, isComputer return fmt.Errorf(i18n.G("failed to marshal policy server registry entries: %v"), err) } - if err := m.runScript(ctx, action, objectName, - "--policy_servers_json", string(jsonGPOData), - "--state_dir", m.stateDir, - ); err != nil { + if err := m.runScript(ctx, action, objectName, "--policy_servers_json", string(jsonGPOData)); err != nil { return err } @@ -227,7 +235,7 @@ func (m *Manager) ApplyPolicy(ctx context.Context, objectName string, isComputer // runScript runs the certificate autoenrollment script with the given arguments. func (m *Manager) runScript(ctx context.Context, action, objectName string, extraArgs ...string) error { - scriptArgs := []string{action, objectName, m.domain} + scriptArgs := []string{action, objectName, m.domain, "--state_dir", m.stateDir, "--global_trust_dir", m.globalTrustDir} scriptArgs = append(scriptArgs, extraArgs...) cmdArgs := append(m.certEnrollCmd, scriptArgs...) cmdCtx, cancel := context.WithTimeout(ctx, time.Second*10) diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll index b53aaaf88..ac55108c3 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll @@ -1,3 +1,3 @@ -enroll keypress example.com --policy_servers_json null --state_dir #TMPDIR#/statedir +enroll keypress example.com --state_dir #TMPDIR#/statedir --global_trust_dir /usr/local/share/ca-certificates --policy_servers_json null KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress PYTHONPATH=#TMPDIR#/sharedir/python diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration index ebedab266..9729ce367 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration @@ -1,3 +1,3 @@ -enroll keypress example.com --policy_servers_json [{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"AuthFlags","data":2,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"Cost","data":2147483645,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"Flags","data":20,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"FriendlyName","data":"ActiveDirectoryEnrollmentPolicy","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"PolicyID","data":"{A5E9BF57-71C6-443A-B7FC-79EFA6F73EBD}","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"URL","data":"LDAP:","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers","valuename":"Flags","data":0,"type":4}] --state_dir #TMPDIR#/statedir +enroll keypress example.com --state_dir #TMPDIR#/statedir --global_trust_dir /usr/local/share/ca-certificates --policy_servers_json [{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"AuthFlags","data":2,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"Cost","data":2147483645,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"Flags","data":20,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"FriendlyName","data":"ActiveDirectoryEnrollmentPolicy","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"PolicyID","data":"{A5E9BF57-71C6-443A-B7FC-79EFA6F73EBD}","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"URL","data":"LDAP:","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers","valuename":"Flags","data":0,"type":4}] KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress PYTHONPATH=#TMPDIR#/sharedir/python diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll index 87bdd0251..95eedd39c 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll @@ -1,3 +1,3 @@ -unenroll keypress example.com --policy_servers_json null --state_dir #TMPDIR#/statedir +unenroll keypress example.com --state_dir #TMPDIR#/statedir --global_trust_dir /usr/local/share/ca-certificates --policy_servers_json null KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress PYTHONPATH=#TMPDIR#/sharedir/python diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present index c9291a660..35eef7348 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present @@ -1,3 +1,3 @@ -unenroll keypress example.com --state_dir #TMPDIR#/statedir +unenroll keypress example.com --state_dir #TMPDIR#/statedir --global_trust_dir /usr/local/share/ca-certificates KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress PYTHONPATH=#TMPDIR#/sharedir/python diff --git a/internal/policies/manager.go b/internal/policies/manager.go index 953ef5f75..ba8c64337 100644 --- a/internal/policies/manager.go +++ b/internal/policies/manager.go @@ -92,19 +92,20 @@ type systemdCaller interface { } type options struct { - cacheDir string - stateDir string - dconfDir string - sudoersDir string - policyKitDir string - runDir string - shareDir string - apparmorDir string - apparmorFsDir string - systemUnitDir string - proxyApplier proxy.Caller - systemdCaller systemdCaller - gdm *gdm.Manager + cacheDir string + stateDir string + dconfDir string + sudoersDir string + policyKitDir string + runDir string + shareDir string + apparmorDir string + apparmorFsDir string + systemUnitDir string + globalTrustDir string + proxyApplier proxy.Caller + systemdCaller systemdCaller + gdm *gdm.Manager apparmorParserCmd []string certAutoenrollCmd []string @@ -202,6 +203,15 @@ func WithSystemUnitDir(p string) Option { } } +// WithGlobalTrustDir specifies a personalized global trust directory for use +// with the certificate manager. +func WithGlobalTrustDir(p string) Option { + return func(o *options) error { + o.globalTrustDir = p + return nil + } +} + // WithProxyApplier specifies a personalized proxy applier for the proxy policy manager. func WithProxyApplier(p proxy.Caller) Option { return func(o *options) error { @@ -237,14 +247,15 @@ func NewManager(bus *dbus.Conn, hostname string, backend backends.Backend, opts // defaults args := options{ - cacheDir: consts.DefaultCacheDir, - stateDir: consts.DefaultStateDir, - runDir: consts.DefaultRunDir, - shareDir: consts.DefaultShareDir, - apparmorDir: consts.DefaultApparmorDir, - systemUnitDir: consts.DefaultSystemUnitDir, - systemdCaller: defaultSystemdCaller, - gdm: nil, + cacheDir: consts.DefaultCacheDir, + stateDir: consts.DefaultStateDir, + runDir: consts.DefaultRunDir, + shareDir: consts.DefaultShareDir, + apparmorDir: consts.DefaultApparmorDir, + systemUnitDir: consts.DefaultSystemUnitDir, + globalTrustDir: consts.DefaultGlobalTrustDir, + systemdCaller: defaultSystemdCaller, + gdm: nil, } // applied options (including dconf manager used by gdm) for _, o := range opts { @@ -295,6 +306,7 @@ func NewManager(bus *dbus.Conn, hostname string, backend backends.Backend, opts certificate.WithStateDir(args.stateDir), certificate.WithRunDir(args.runDir), certificate.WithShareDir(args.shareDir), + certificate.WithGlobalTrustDir(args.globalTrustDir), } if args.certAutoenrollCmd != nil { certificateOpts = append(certificateOpts, certificate.WithCertAutoenrollCmd(args.certAutoenrollCmd)) From f826c9aab2257a368ca2f44858f0744b65bf5ef2 Mon Sep 17 00:00:00 2001 From: Gabriel Nagy Date: Mon, 31 Jul 2023 23:07:24 +0300 Subject: [PATCH 3/8] Make global trust and state dirs configurable To aid in integration tests, these must be editable via the configuration file. --- cmd/adsysd/daemon/daemon.go | 16 +++-- conf.example/adsys.yaml | 2 + internal/adsysservice/adsysservice.go | 80 +++++++++++++++------- internal/adsysservice/adsysservice_test.go | 4 ++ 4 files changed, 70 insertions(+), 32 deletions(-) diff --git a/cmd/adsysd/daemon/daemon.go b/cmd/adsysd/daemon/daemon.go index b47a3cc93..2e8588b94 100644 --- a/cmd/adsysd/daemon/daemon.go +++ b/cmd/adsysd/daemon/daemon.go @@ -40,14 +40,16 @@ type daemonConfig struct { Socket string CacheDir string `mapstructure:"cache_dir"` + StateDir string `mapstructure:"state_dir"` RunDir string `mapstructure:"run_dir"` - DconfDir string `mapstructure:"dconf_dir"` - SudoersDir string `mapstructure:"sudoers_dir"` - PolicyKitDir string `mapstructure:"policykit_dir"` - ApparmorDir string `mapstructure:"apparmor_dir"` - ApparmorFsDir string `mapstructure:"apparmorfs_dir"` - SystemUnitDir string `mapstructure:"systemunit_dir"` + DconfDir string `mapstructure:"dconf_dir"` + SudoersDir string `mapstructure:"sudoers_dir"` + PolicyKitDir string `mapstructure:"policykit_dir"` + ApparmorDir string `mapstructure:"apparmor_dir"` + ApparmorFsDir string `mapstructure:"apparmorfs_dir"` + SystemUnitDir string `mapstructure:"systemunit_dir"` + GlobalTrustDir string `mapstructure:"global_trust_dir"` AdBackend string `mapstructure:"ad_backend"` SSSdConfig sss.Config `mapstructure:"sssd"` @@ -111,6 +113,7 @@ func New() *App { RunE: func(cmd *cobra.Command, args []string) error { adsys, err := adsysservice.New(context.Background(), adsysservice.WithCacheDir(a.config.CacheDir), + adsysservice.WithStateDir(a.config.StateDir), adsysservice.WithRunDir(a.config.RunDir), adsysservice.WithDconfDir(a.config.DconfDir), adsysservice.WithSudoersDir(a.config.SudoersDir), @@ -118,6 +121,7 @@ func New() *App { adsysservice.WithApparmorDir(a.config.ApparmorDir), adsysservice.WithApparmorFsDir(a.config.ApparmorFsDir), adsysservice.WithSystemUnitDir(a.config.SystemUnitDir), + adsysservice.WithGlobalTrustDir(a.config.GlobalTrustDir), adsysservice.WithADBackend(a.config.AdBackend), adsysservice.WithSSSConfig(a.config.SSSdConfig), adsysservice.WithWinbindConfig(a.config.WinbindConfig), diff --git a/conf.example/adsys.yaml b/conf.example/adsys.yaml index 2364de8e8..58d5bee20 100644 --- a/conf.example/adsys.yaml +++ b/conf.example/adsys.yaml @@ -5,12 +5,14 @@ socket: /tmp/adsysd/socket # Service only configuration service_timeout: 3600 cache_dir: /tmp/adsysd/cache +state_dir: /tmp/adsysd/lib run_dir: /tmp/adsysd/run dconf_dir: /etc/dconf sudoers_dir: /etc/sudoers.d policykit_dir: /etc/polkit-1 apparmor_dir: /etc/apparmor.d/adsys apparmorfs_dir: /sys/kernel/security/apparmor +global_trust_dir: /usr/local/share/ca-certificates # Backend selection: sssd (default) or winbind #ad_backend: sssd diff --git a/internal/adsysservice/adsysservice.go b/internal/adsysservice/adsysservice.go index 76926ac4c..8c37725e4 100644 --- a/internal/adsysservice/adsysservice.go +++ b/internal/adsysservice/adsysservice.go @@ -46,28 +46,32 @@ type Service struct { } type state struct { - cacheDir string - runDir string - dconfDir string - sudoersDir string - policyKitDir string - apparmorDir string - systemUnitDir string + cacheDir string + stateDir string + runDir string + dconfDir string + sudoersDir string + policyKitDir string + apparmorDir string + systemUnitDir string + globalTrustDir string } type options struct { - cacheDir string - runDir string - dconfDir string - sudoersDir string - policyKitDir string - apparmorDir string - apparmorFsDir string - systemUnitDir string - adBackend string - sssConfig sss.Config - winbindConfig winbind.Config - authorizer authorizerer + cacheDir string + stateDir string + runDir string + dconfDir string + sudoersDir string + policyKitDir string + apparmorDir string + apparmorFsDir string + systemUnitDir string + globalTrustDir string + adBackend string + sssConfig sss.Config + winbindConfig winbind.Config + authorizer authorizerer } type option func(*options) error @@ -83,6 +87,14 @@ func WithCacheDir(p string) func(o *options) error { } } +// WithStateDir specifies a personalized daemon state directory. +func WithStateDir(p string) func(o *options) error { + return func(o *options) error { + o.stateDir = p + return nil + } +} + // WithRunDir specifies a personalized /run. func WithRunDir(p string) func(o *options) error { return func(o *options) error { @@ -141,6 +153,14 @@ func WithSystemUnitDir(p string) func(o *options) error { } } +// WithGlobalTrustDir specifies a personalized directory for global trust store. +func WithGlobalTrustDir(p string) func(o *options) error { + return func(o *options) error { + o.globalTrustDir = p + return nil + } +} + // WithADBackend specifies our specific backend to select. func WithADBackend(backend string) func(o *options) error { return func(o *options) error { @@ -261,6 +281,9 @@ func New(ctx context.Context, opts ...option) (s *Service, err error) { if args.cacheDir != "" { policyOptions = append(policyOptions, policies.WithCacheDir(args.cacheDir)) } + if args.stateDir != "" { + policyOptions = append(policyOptions, policies.WithStateDir(args.stateDir)) + } if args.dconfDir != "" { policyOptions = append(policyOptions, policies.WithDconfDir(args.dconfDir)) } @@ -282,6 +305,9 @@ func New(ctx context.Context, opts ...option) (s *Service, err error) { if args.systemUnitDir != "" { policyOptions = append(policyOptions, policies.WithSystemUnitDir(args.systemUnitDir)) } + if args.globalTrustDir != "" { + policyOptions = append(policyOptions, policies.WithGlobalTrustDir(args.globalTrustDir)) + } m, err := policies.NewManager(bus, hostname, adBackend, policyOptions...) if err != nil { return nil, err @@ -295,13 +321,15 @@ func New(ctx context.Context, opts ...option) (s *Service, err error) { policyManager: m, authorizer: args.authorizer, state: state{ - cacheDir: args.cacheDir, - dconfDir: args.dconfDir, - sudoersDir: args.sudoersDir, - policyKitDir: args.policyKitDir, - runDir: args.runDir, - apparmorDir: args.apparmorDir, - systemUnitDir: args.systemUnitDir, + cacheDir: args.cacheDir, + stateDir: args.stateDir, + dconfDir: args.dconfDir, + sudoersDir: args.sudoersDir, + policyKitDir: args.policyKitDir, + runDir: args.runDir, + apparmorDir: args.apparmorDir, + systemUnitDir: args.systemUnitDir, + globalTrustDir: args.globalTrustDir, }, initSystemTime: initSysTime, bus: bus, diff --git a/internal/adsysservice/adsysservice_test.go b/internal/adsysservice/adsysservice_test.go index 9dcc4c8a5..67c8a9516 100644 --- a/internal/adsysservice/adsysservice_test.go +++ b/internal/adsysservice/adsysservice_test.go @@ -57,12 +57,14 @@ func TestNew(t *testing.T) { temp := t.TempDir() adsysCacheDir := filepath.Join(temp, "parentcache", "cache") + adsysStateDir := filepath.Join(temp, "var", "lib") adsysRunDir := filepath.Join(temp, "parentrun", "run") dconfDir := filepath.Join(temp, "dconf") sudoersDir := filepath.Join(temp, "sudoers.d") policyKitDir := filepath.Join(temp, "polkit-1") apparmorDir := filepath.Join(temp, "apparmor.d", "adsys") apparmorFsDir := filepath.Join(temp, "apparmorfs") + globalTrustDir := filepath.Join(temp, "ca-certificates") if tc.existingAdsysDirs { require.NoError(t, os.MkdirAll(adsysCacheDir, 0700), "Setup: could not create adsys cache directory") require.NoError(t, os.MkdirAll(adsysRunDir, 0700), "Setup: could not create adsys run directory") @@ -86,12 +88,14 @@ func TestNew(t *testing.T) { options := []adsysservice.Option{ adsysservice.WithCacheDir(adsysCacheDir), + adsysservice.WithStateDir(adsysStateDir), adsysservice.WithRunDir(adsysRunDir), adsysservice.WithDconfDir(dconfDir), adsysservice.WithSudoersDir(sudoersDir), adsysservice.WithPolicyKitDir(policyKitDir), adsysservice.WithApparmorDir(apparmorDir), adsysservice.WithApparmorFsDir(apparmorFsDir), + adsysservice.WithGlobalTrustDir(globalTrustDir), adsysservice.WithSSSConfig(sssdConfig), adsysservice.WithWinbindConfig(winbindConfig), } From f84a0fa89e780708ea8dfadcd51913b381e55c38 Mon Sep 17 00:00:00 2001 From: Gabriel Nagy Date: Tue, 1 Aug 2023 12:33:34 +0300 Subject: [PATCH 4/8] Append to PYTHONPATH instead of overriding To be able to use the samba mock in the integration tests we need to append to the PYTHONPATH variable instead of overriding it. This way previously set paths will take precedence. --- internal/policies/certificate/certificate.go | 2 +- .../TestPolicyApply/golden/computer,_configured_to_enroll | 2 +- .../computer,_configured_to_enroll,_advanced_configuration | 2 +- .../TestPolicyApply/golden/computer,_configured_to_unenroll | 2 +- .../golden/computer,_no_entries,_samba_cache_present | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/policies/certificate/certificate.go b/internal/policies/certificate/certificate.go index 5c6d06284..a1467143d 100644 --- a/internal/policies/certificate/certificate.go +++ b/internal/policies/certificate/certificate.go @@ -245,7 +245,7 @@ func (m *Manager) runScript(ctx context.Context, action, objectName string, extr cmd := exec.CommandContext(cmdCtx, cmdArgs[0], cmdArgs[1:]...) cmd.Env = append(os.Environ(), fmt.Sprintf("KRB5CCNAME=%s", filepath.Join(m.krb5CacheDir, objectName)), - fmt.Sprintf("PYTHONPATH=%s", m.vendorPythonDir), + fmt.Sprintf("PYTHONPATH=%s:%s", os.Getenv("PYTHONPATH"), m.vendorPythonDir), ) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll index ac55108c3..d26d7ddb6 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll @@ -1,3 +1,3 @@ enroll keypress example.com --state_dir #TMPDIR#/statedir --global_trust_dir /usr/local/share/ca-certificates --policy_servers_json null KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress -PYTHONPATH=#TMPDIR#/sharedir/python +PYTHONPATH=:#TMPDIR#/sharedir/python diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration index 9729ce367..d24501b34 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration @@ -1,3 +1,3 @@ enroll keypress example.com --state_dir #TMPDIR#/statedir --global_trust_dir /usr/local/share/ca-certificates --policy_servers_json [{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"AuthFlags","data":2,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"Cost","data":2147483645,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"Flags","data":20,"type":4},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"FriendlyName","data":"ActiveDirectoryEnrollmentPolicy","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"PolicyID","data":"{A5E9BF57-71C6-443A-B7FC-79EFA6F73EBD}","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers\\37c9dc30f207f27f61a2f7c3aed598a6e2920b54","valuename":"URL","data":"LDAP:","type":1},{"keyname":"Software\\Policies\\Microsoft\\Cryptography\\PolicyServers","valuename":"Flags","data":0,"type":4}] KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress -PYTHONPATH=#TMPDIR#/sharedir/python +PYTHONPATH=:#TMPDIR#/sharedir/python diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll index 95eedd39c..a367e8c81 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll @@ -1,3 +1,3 @@ unenroll keypress example.com --state_dir #TMPDIR#/statedir --global_trust_dir /usr/local/share/ca-certificates --policy_servers_json null KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress -PYTHONPATH=#TMPDIR#/sharedir/python +PYTHONPATH=:#TMPDIR#/sharedir/python diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present index 35eef7348..1464f1af0 100644 --- a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present +++ b/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present @@ -1,3 +1,3 @@ unenroll keypress example.com --state_dir #TMPDIR#/statedir --global_trust_dir /usr/local/share/ca-certificates KRB5CCNAME=#TMPDIR#/rundir/krb5cc/keypress -PYTHONPATH=#TMPDIR#/sharedir/python +PYTHONPATH=:#TMPDIR#/sharedir/python From d10233b5eaaa1a63cbffa08357557bd041dc55f2 Mon Sep 17 00:00:00 2001 From: Gabriel Nagy Date: Tue, 1 Aug 2023 12:35:58 +0300 Subject: [PATCH 5/8] Use shutil to remove non-empty directory Removing a directory with `os.removedirs` will fail if it's non-empty. We can circumvent this behavior by using `shutil.rmtree` instead. --- internal/policies/certificate/cert-autoenroll | 4 +++- internal/policies/certificate/cert-autoenroll_test.go | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/policies/certificate/cert-autoenroll b/internal/policies/certificate/cert-autoenroll index f2b4582df..f6b939efd 100755 --- a/internal/policies/certificate/cert-autoenroll +++ b/internal/policies/certificate/cert-autoenroll @@ -5,6 +5,7 @@ import json import os import sys import tempfile +import shutil from samba import param from samba.credentials import MUST_USE_KERBEROS, Credentials @@ -80,7 +81,8 @@ def main(): ext.enroll(guid, entries, trust_dir, private_dir, global_trust_dir) else: ext.unenroll(guid) - os.removedirs(samba_cache_dir) + if os.path.exists(samba_cache_dir): + shutil.rmtree(samba_cache_dir) def gpo_entries(entries_json): """ diff --git a/internal/policies/certificate/cert-autoenroll_test.go b/internal/policies/certificate/cert-autoenroll_test.go index 1bd2f03d8..39375cb7f 100644 --- a/internal/policies/certificate/cert-autoenroll_test.go +++ b/internal/policies/certificate/cert-autoenroll_test.go @@ -110,6 +110,9 @@ func TestCertAutoenrollScript(t *testing.T) { sambaCacheDir := filepath.Join(stateDir, "samba") globalTrustDir := filepath.Join(stateDir, "ca-certificates") + // Create a dummy cache file to ensure we don't fail when removing a non-empty directory + testutils.CreatePath(t, filepath.Join(sambaCacheDir, "cert_gpo_state_HOST.tdb")) + if tc.readOnlyPath { testutils.MakeReadOnly(t, stateDir) } From 6a2ae832e0fae12e96f01c093f4ced124c3e091c Mon Sep 17 00:00:00 2001 From: Gabriel Nagy Date: Tue, 1 Aug 2023 12:37:34 +0300 Subject: [PATCH 6/8] Set state and global trust dir in integration tests --- cmd/adsysd/integration_tests/adsys_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/adsysd/integration_tests/adsys_test.go b/cmd/adsysd/integration_tests/adsys_test.go index dd848102b..b6cc04142 100644 --- a/cmd/adsysd/integration_tests/adsys_test.go +++ b/cmd/adsysd/integration_tests/adsys_test.go @@ -236,6 +236,7 @@ socket: %s/socket # Service only configuration cache_dir: %s/cache +state_dir: %s/lib run_dir: %s/run service_timeout: 30 @@ -254,7 +255,8 @@ policykit_dir: %s/polkit-1 apparmor_dir: %s/apparmor.d/adsys apparmorfs_dir: %s/apparmorfs systemunit_dir: %s/systemd/system -`, args.adsysDir, args.adsysDir, args.adsysDir, args.backend, args.adsysDir, args.adsysDir, args.adsysDir, args.adsysDir, args.adsysDir, args.adsysDir, args.adsysDir)) +global_trust_dir: %s/share/ca-certificates +`, args.adsysDir, args.adsysDir, args.adsysDir, args.adsysDir, args.backend, args.adsysDir, args.adsysDir, args.adsysDir, args.adsysDir, args.adsysDir, args.adsysDir, args.adsysDir, args.adsysDir)) testutils.WriteFile(t, confFile, confData, os.ModePerm) require.NoError(t, os.MkdirAll(filepath.Join(args.adsysDir, "dconf"), 0750), "Setup: should create dconf dir") From b6275a74c77d4d6162db2513b234dac20aee2399 Mon Sep 17 00:00:00 2001 From: Gabriel Nagy Date: Tue, 1 Aug 2023 12:41:01 +0300 Subject: [PATCH 7/8] Integration tests for certificate policy manager Old data - policy configured & disabled Up to date - policy configured & enabled Some things to notice in the tests behavior: - no changes to user golden files with no initial state - purging machine policies removes the samba directory if it exists Fixes UDENG-1059 --- .../integration_tests/adsysctl_policy_test.go | 17 +++++++++++++++++ .../Machine/Registry.pol | Bin 9234 -> 9388 bytes .../already_up_to_date/lib/certs/.empty | 0 .../lib/private/certs/.empty | 0 .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../lib/certs/.empty | 0 .../lib/private/certs/.empty | 0 .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../machine,_first_time/lib/certs/.empty | 0 .../lib/private/certs/.empty | 0 .../machine,_first_time/lib/samba/.empty | 0 .../lib/certs/.empty | 0 .../lib/private/certs/.empty | 0 .../lib/samba/.empty | 0 .../machine,_update_old_data/lib/certs/.empty | 0 .../lib/private/certs/.empty | 0 .../machine,_update_old_data/lib/samba/.empty | 0 .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../purge_machine_policies/lib/certs/.empty | 0 .../lib/private/certs/.empty | 0 .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../refresh_all_connected/lib/certs/.empty | 0 .../lib/private/certs/.empty | 0 .../refresh_all_connected/lib/samba/.empty | 0 .../refresh_some_connected/lib/certs/.empty | 0 .../lib/private/certs/.empty | 0 .../refresh_some_connected/lib/samba/.empty | 0 .../lib/certs/.empty | 0 .../lib/private/certs/.empty | 0 .../lib/samba/.empty | 0 .../lib/certs/.empty | 0 .../lib/private/certs/.empty | 0 .../lib/samba/.empty | 0 .../cache/policies/HOST/policies | 3 +++ .../Machine/Registry.pol | Bin 7414 -> 7568 bytes .../lib/samba/cert_gpo_state_HOST.tdb | 1 + .../old-data/cache/policies/HOST/policies | 3 +++ 46 files changed, 37 insertions(+) create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/already_up_to_date/lib/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/already_up_to_date/lib/private/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/already_up_to_date/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/current_user,_first_time/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/current_user,_first_time_with_winbind_backend/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/current_user,_static_ad_server/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/current_user_with_default_domain_completion/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/current_user_with_domain_username/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/does_not_error_when_d-bus_proxy_object_is_not_available/lib/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/does_not_error_when_d-bus_proxy_object_is_not_available/lib/private/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/does_not_error_when_d-bus_proxy_object_is_not_available/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/invalid_krb5ccname_format_is_supported/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/krb5ccname_is_ignored_when_requesting_ticket_on_other_user/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/krb5ccname_is_ignored_with_existing_ticket_for_user/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/machine,_first_time/lib/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/machine,_first_time/lib/private/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/machine,_first_time/lib/samba/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/machine,_first_time_with_winbind_backend/lib/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/machine,_first_time_with_winbind_backend/lib/private/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/machine,_first_time_with_winbind_backend/lib/samba/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/machine,_update_old_data/lib/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/machine,_update_old_data/lib/private/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/machine,_update_old_data/lib/samba/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/other_user,_first_time/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/purge_current_user_policies/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/purge_machine_policies/lib/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/purge_machine_policies/lib/private/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/purge_other_user_policies/lib/samba/cert_gpo_state_HOST.tdb create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_all_connected/lib/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_all_connected/lib/private/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_all_connected/lib/samba/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_some_connected/lib/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_some_connected/lib/private/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_some_connected/lib/samba/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_with_no_user_connected_updates_machines/lib/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_with_no_user_connected_updates_machines/lib/private/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_with_no_user_connected_updates_machines/lib/samba/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_with_one_dangling_symlink_ignores_the_respective_user/lib/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_with_one_dangling_symlink_ignores_the_respective_user/lib/private/certs/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/golden/refresh_with_one_dangling_symlink_ignores_the_respective_user/lib/samba/.empty create mode 100644 cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/states/localhost-uptodate/lib/samba/cert_gpo_state_HOST.tdb diff --git a/cmd/adsysd/integration_tests/adsysctl_policy_test.go b/cmd/adsysd/integration_tests/adsysctl_policy_test.go index aa5908006..6530c50b3 100644 --- a/cmd/adsysd/integration_tests/adsysctl_policy_test.go +++ b/cmd/adsysd/integration_tests/adsysctl_policy_test.go @@ -711,6 +711,22 @@ func TestPolicyUpdate(t *testing.T) { systemAnswer: "apply_proxy_fail", wantErr: true, }, + "Error on system certificate autoenroll failing": { + args: []string{"-m"}, + krb5ccname: "-", + krb5ccNamesState: []krb5ccNamesWithState{ + { + src: "ccache_EXAMPLE.COM", + machine: true, + }, + }, + initState: "localhost-uptodate", + // this generates an error when parent directories are not writable + readOnlyDirs: []string{ + "lib", // state directory + }, + wantErr: true, + }, "Error on host is offline, without policies": { sssdConf: "sssd.conf-offline", initState: "old-data", @@ -1073,6 +1089,7 @@ func TestPolicyUpdate(t *testing.T) { testutils.CompareTreesWithFiltering(t, filepath.Join(adsysDir, "polkit-1"), filepath.Join(goldenPath, "polkit-1"), update) testutils.CompareTreesWithFiltering(t, filepath.Join(adsysDir, "apparmor.d", "adsys"), filepath.Join(goldenPath, "apparmor.d", "adsys"), update) testutils.CompareTreesWithFiltering(t, filepath.Join(adsysDir, "systemd", "system"), filepath.Join(goldenPath, "systemd", "system"), update) + testutils.CompareTreesWithFiltering(t, filepath.Join(adsysDir, "lib"), filepath.Join(goldenPath, "lib"), update) // Current user can have different UID depending on where it’s running. We can’t mock it as we rely on current uid // in the process for authorization check. Just make it generic. diff --git a/cmd/adsysd/integration_tests/testdata/AD/SYSVOL/example.com/Policies/{C4F393CA-AD9A-4595-AEBC-3FA6EE484285}/Machine/Registry.pol b/cmd/adsysd/integration_tests/testdata/AD/SYSVOL/example.com/Policies/{C4F393CA-AD9A-4595-AEBC-3FA6EE484285}/Machine/Registry.pol index 7156cbbd9d926ba675d7d7fc7eab646a6ae446b9..7586f8f8fef64b20c2abfa7415a1ed71b3b08f7e 100644 GIT binary patch delta 128 zcmbQ_vBqZ y!{EqJ3Kn%`$OG%iVaQ>~Wk?0`N*EXztby{b3;{r0Ibc&O!D1{(BqNX=%K!k-+Zg-+ delta 7 OcmbPW{mpX2HyHpABLlqv diff --git a/cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/states/localhost-uptodate/lib/samba/cert_gpo_state_HOST.tdb b/cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/states/localhost-uptodate/lib/samba/cert_gpo_state_HOST.tdb new file mode 100644 index 000000000..9a69ddcee --- /dev/null +++ b/cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/states/localhost-uptodate/lib/samba/cert_gpo_state_HOST.tdb @@ -0,0 +1 @@ +TDB file diff --git a/cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/states/old-data/cache/policies/HOST/policies b/cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/states/old-data/cache/policies/HOST/policies index a1a4aa6f4..368d9b029 100644 --- a/cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/states/old-data/cache/policies/HOST/policies +++ b/cmd/adsysd/integration_tests/testdata/TestPolicyUpdate/states/old-data/cache/policies/HOST/policies @@ -54,6 +54,9 @@ gpos: disabled: true - key: proxy/no-proxy value: old-localhost,127.0.0.1,::1 + certificate: + - key: autoenroll + value: "32768" - id: '{31B2F340-016D-11D2-945F-00C04FB984F9}' name: Default Domain Policy From 4de2c2ab5d4860b9a8ceecf18a80ba6f95dafb98 Mon Sep 17 00:00:00 2001 From: Gabriel Nagy Date: Tue, 1 Aug 2023 17:41:08 +0300 Subject: [PATCH 8/8] Fix certificate test name Update the certificate test name to match the tested method name (and our convention). --- internal/policies/certificate/certificate_test.go | 2 +- .../golden/computer,_configured_to_enroll | 0 .../computer,_configured_to_enroll,_advanced_configuration | 0 .../golden/computer,_configured_to_unenroll | 0 .../golden/computer,_no_entries,_samba_cache_present | 0 5 files changed, 1 insertion(+), 1 deletion(-) rename internal/policies/certificate/testdata/{TestPolicyApply => TestApplyPolicy}/golden/computer,_configured_to_enroll (100%) rename internal/policies/certificate/testdata/{TestPolicyApply => TestApplyPolicy}/golden/computer,_configured_to_enroll,_advanced_configuration (100%) rename internal/policies/certificate/testdata/{TestPolicyApply => TestApplyPolicy}/golden/computer,_configured_to_unenroll (100%) rename internal/policies/certificate/testdata/{TestPolicyApply => TestApplyPolicy}/golden/computer,_no_entries,_samba_cache_present (100%) diff --git a/internal/policies/certificate/certificate_test.go b/internal/policies/certificate/certificate_test.go index 872a3efd2..262c17457 100644 --- a/internal/policies/certificate/certificate_test.go +++ b/internal/policies/certificate/certificate_test.go @@ -32,7 +32,7 @@ var advancedConfigurationEntries = []entry.Entry{ {Key: "Software/Policies/Microsoft/Cryptography/PolicyServers/Flags", Value: "0"}, } -func TestPolicyApply(t *testing.T) { +func TestApplyPolicy(t *testing.T) { t.Parallel() tests := map[string]struct { diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll b/internal/policies/certificate/testdata/TestApplyPolicy/golden/computer,_configured_to_enroll similarity index 100% rename from internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll rename to internal/policies/certificate/testdata/TestApplyPolicy/golden/computer,_configured_to_enroll diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration b/internal/policies/certificate/testdata/TestApplyPolicy/golden/computer,_configured_to_enroll,_advanced_configuration similarity index 100% rename from internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_enroll,_advanced_configuration rename to internal/policies/certificate/testdata/TestApplyPolicy/golden/computer,_configured_to_enroll,_advanced_configuration diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll b/internal/policies/certificate/testdata/TestApplyPolicy/golden/computer,_configured_to_unenroll similarity index 100% rename from internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_configured_to_unenroll rename to internal/policies/certificate/testdata/TestApplyPolicy/golden/computer,_configured_to_unenroll diff --git a/internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present b/internal/policies/certificate/testdata/TestApplyPolicy/golden/computer,_no_entries,_samba_cache_present similarity index 100% rename from internal/policies/certificate/testdata/TestPolicyApply/golden/computer,_no_entries,_samba_cache_present rename to internal/policies/certificate/testdata/TestApplyPolicy/golden/computer,_no_entries,_samba_cache_present