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

Fix retrieving multiple certificate fingerprints on InspIRCd v4. #379

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

SadieCat
Copy link
Contributor

InspIRCd v4 supports using multiple TLS fingerprint algorithms to allow upgrading from insecure algorithms over time. We expose this to clients by sending multiple RPL_WHOISCERTFP lines.

Unfortunately, irc-framework only caches the last of these to users which breaks WHOIS output on The Lounge as the last one on multiple-fingerprint networks is always a fallback. This has resulted in confused users who are unsure why their client is showing a different fingerprint to services.

This PR fixes that by caching the multiple fingerprint algorithms sent by the IRC server.

@ItsOnlyBinary
Copy link
Contributor

Thanks for pointing this out.

Would you be happy if the first RPL_WHOISCERTFP is set as certfp then any extras are added to certfp_fallbacks array? This way would ensure backwards compatibility, while offering the option to update and display the extra certs.

I can make the changes if you think this is acceptable

@SadieCat
Copy link
Contributor Author

SadieCat commented Aug 21, 2024

I'm fine with that. :)

You could also just add a certfps field and deprecate the old API. I'm not too bothered either way.

@SadieCat
Copy link
Contributor Author

I updated this to match what I suggested. Is this acceptable?

@ItsOnlyBinary ItsOnlyBinary merged commit 4ed9d28 into kiwiirc:master Aug 27, 2024
4 checks passed
@SadieCat SadieCat deleted the fix-certfp branch August 27, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants