Certificate enrollment: Fixes for LDAP searches and for some issues with multiple domain environments #1185
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
In the log output, there were several line breaks that were being escaped (i.e. '\\n'), which was preventing them from being properly printed. Fixing this allows for a better debugging experience and clearer output.
denisonbarbosa
changed the title
The previous implementation tried to autodiscover the CEPCES server to which it should connect to and fetch the templates for a specific CA. However, it was setup in a way which would always override what was configured in the cepces config file. This was problematic as it was always assuming that the CEPCES server was the same as the CA source, which isn't always the case. These changes now allow us to override this behavior through the cepces.conf file or keep it as is, provided that the configuration file is the default one. This also improves the output and error handling of the process, as it didn't return any kind of message when failing to parse the templates properly.
Some of the LDAP queries, although working on simple single-domain environments, were causing problems on more complex ones (multiple domains). This refines some of the queries to properly handle both cases using the values that they should be using.
denisonbarbosa
force-pushed
the
fix-enrollment
branch
from
a8a09cb to
81549ef
Compare
didrocks
requested changes
Jan 31, 2025
| ext.enroll(guid, entries, trust_dir, private_dir) | ||
| enrolled_cas = ext.enroll(guid, entries, trust_dir, private_dir) | ||
| if enrolled_cas is None: | ||
| log.warning('Could not enroll to any certificates') |
There was a problem hiding this comment.
"any certificate servers" (or service), rather?
| @@ -191,14 +194,31 @@ def get_supported_templates(server): | |||
| log.error('Failed to find cepces-submit') | |||
| return [] | |||
|
|
|||
| cepces_config = configparser.ConfigParser() | |||
There was a problem hiding this comment.
that brings me back some nostalgia :)
| p = Popen(cepces_args, env=env, stdout=PIPE, stderr=PIPE) | ||
| out, err = bytes(), bytes() | ||
| try: | ||
| out, err = p.communicate(timeout=3) |
There was a problem hiding this comment.
maybe add a comment on the need for timeout. (could be stuck forever)
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.
This PR handles some issues encountered during certificate auto-enrollment on environments with multiple domains. Specific details for the fixes are listed on their respective commit messages, so I highly recommend looking at those.
Closes #1108
UDENG-4835