-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement certificate policy manager for machines #745
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GabrielNagy
force-pushed
the
certificate-policy-manager
branch
from
July 26, 2023 07:16
d0414fc
to
6bce3d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Well done!
GabrielNagy
force-pushed
the
certificate-policy-manager
branch
2 times, most recently
from
July 28, 2023 14:42
e82e905
to
9b17e5e
Compare
Codecov Report
@@ Coverage Diff @@
## main #745 +/- ##
==========================================
+ Coverage 86.20% 86.23% +0.03%
==========================================
Files 75 77 +2
Lines 8196 8517 +321
==========================================
+ Hits 7065 7345 +280
- Misses 814 850 +36
- Partials 317 322 +5
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
The certificate policy manager is configured via the Windows GPO entries instead of our custom ADMX/ADML counterpart. We were already able to parse these entries but we were excluding them as they didn't start with our Ubuntu-specific prefix. The easiest way to make this work is to add the Ubuntu prefix to the keys that we want to use in our certificate policy manager.
We will need to know both the domain and online status of the backend for the certificate policy manager. Since the backend object contains both of these, pass it along when creating the policy manager object.
This manager leverages the Samba implementation of certificate autoenrollment. It contains a few patches and fixes on top of the samba#master version of `gp_cert_auto_enroll_ext.py` file, that will ultimately be upstreamed. Autoenrollment is performed via a separate policy manager that runs a helper Python script (`cert-autoenroll`) which communicates with the Windows CEP/CES services through Samba. For better control and to avoid unexpected behavior we vendor the required Samba files, which are confirmed to work on all Ubuntu versions starting with (and including) Jammy (22.04). Samba has its own cache mechanism which stores information concerning the applied GPOs which we are using in order to ensure idempotency. By default, Samba would parse the .reg file itself (see the process_group_policy method from the vendored code). However, it is better to have this functionality entirely within adsys so we can provide samba the pre-parsed list of GPO entries and override the entry point of the extension. This ensures we don't operate on disk files which can change at anytime (even during adsys policy application). Doing this we also have better knowledge on the enabled/disabled state of the GPO entry used to configure the policy. The advanced configuration entries are passed via JSON to the external script which then takes care to create the proper PReg entries that can be used by Samba to apply additional logic when determining the policy servers to use. Fixes UDENG-1056
Similar to the adsys-gpolist, provide a way for users to dump the certificate autoenrollment script for debugging purposes.
The Samba implementation creates the private directory with 0700 permissions and uses 0755 for the rest. We should honor that in our implementation as well. Additionally, create the global trust dir on the off chance it doesn't already exist.
And change the naming from stateDir to sambaCacheDir to better reflect what the path is used for.
Similar to our other policy manager behaviors, if the certificate policy is either disabled or not configured, unenroll the machine if applicable. To avoid running the helper Python script on every policy apply, determine if we actually need to unenroll by checking the existence of the Samba cache directory. Additionally, update the Python script to remove the directory after unenrollment is successful.
Because Python coverage can't be parallelized, avoid running these tests in parallel.
GabrielNagy
force-pushed
the
certificate-policy-manager
branch
from
July 31, 2023 09:35
9b17e5e
to
1a7be22
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details are in the individual commits
Fixes UDENG-1056