Skip to content
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

When joining, always retrieve the service's subject_name from the given cert #6660

Merged
merged 6 commits into from
Nov 19, 2024

Conversation

eddyashton
Copy link
Member

Follow-up to #5993.

When 5.x nodes join from pre-5.x nodes, they end up with a default subject_name (they get it in the join response, where it wasn't populated so had a default value). If they then try to renew the service cert, they'll cause a change in the service name. This is the minimal fix - ignore the serialised subject_name field, and always extract it from the given service identity.

This includes a small extension to the lts_compatibility test to verify this behaviour. When run on the 5.x branch, this caused a failure without the C++ change, and passes with the C++ change.

I'd like to make a more systematic cleanup of identity.h to remove some dead code and simplify the constructor paths, but will do that separately from this minimal fix.

@eddyashton eddyashton added auto-backport Automatically backport this PR to LTS branch 5.x-todo PRs which should be backported to 5.x run-long-test Run Long Test job labels Nov 19, 2024
@eddyashton eddyashton requested a review from a team as a code owner November 19, 2024 12:09
@achamayou
Copy link
Member

@eddyashton suggest a changelog entry

@eddyashton
Copy link
Member Author

@eddyashton suggest a changelog entry

Good idea. I'll add that once the current runs are green, to avoid triggering/waiting for unnecessary runs.

@eddyashton eddyashton enabled auto-merge November 19, 2024 15:23
@eddyashton eddyashton removed the run-long-test Run Long Test job label Nov 19, 2024
@eddyashton eddyashton added this pull request to the merge queue Nov 19, 2024
Merged via the queue into microsoft:main with commit 863681d Nov 19, 2024
13 checks passed
@eddyashton eddyashton deleted the joinee_subject_name_from_cert branch November 19, 2024 16:24
@eddyashton
Copy link
Member Author

💔 All backports failed

Status Branch Result
release/5.x An unhandled error occurred. Please see the logs for details

Manual backport

To create the backport manually run:

backport --pr 6660

Questions ?

Please refer to the Backport tool documentation

@eddyashton eddyashton added the backported This PR was successfully backported to LTS branch label Nov 19, 2024
eddyashton added a commit to eddyashton/CCF that referenced this pull request Nov 19, 2024
…iven cert (microsoft#6660)

(cherry picked from commit 863681d)

# Conflicts:
#	CHANGELOG.md
#	python/pyproject.toml
#	src/node/identity.h
eddyashton added a commit that referenced this pull request Nov 19, 2024
…s `subject_name` from the given cert (#6660) (#6665)

Co-authored-by: Amaury Chamayou <amchamay@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.x-todo PRs which should be backported to 5.x auto-backport Automatically backport this PR to LTS branch backported This PR was successfully backported to LTS branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants