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

Proposal: support multiple signaturePublickeys #229

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

evan-goode
Copy link
Contributor

This PR allows authentication servers to specify an array of profile property public keys instead of just one.

I propose adding a new field, signaturePublickeys, to the authlib-injector metadata route at the root of the API. signaturePublickeys is an array of strings containing public keys in the same format as the signaturePublickey. Any of the keys specified either in signaturePublickeys or as the signaturePublickey can be used to verify the signature of a textures payload. Authentication servers aren't required to use the new field; signaturePublicKeys is simply ignored if it's missing from the metadata response.

With this change, authentication server A could forward a texture payload from authentication server B, and the client could accept a texture signed by either authentication server. Currently, this forwarding only works with Mojang as authentication server B, since Mojang's public key is hardcoded (mojang-publickey.der).

Wiki updates would be needed following this PR.

@evan-goode
Copy link
Contributor Author

But perhaps this new field should be renamed to propertiesPublicKeys and only be used for property signatures, and have an additional field for playersCertificatesPublicKeys as @erickskrauch suggested in #254?

@Silverteal
Copy link

Silverteal commented Nov 8, 2024

If propertiesPublicKeys and signaturePublickey are all given, shall signaturePublicKey be ignored or used? Or just leave it undefined (that they shouldn't both appear)?

(For the previous two options) give them two at the same time for compatibility won't make much sense since profiles will be signed with multiple possible keys and verification will fail anyway.

Allow authentication servers to specify an array of profile property
public keys and an array of player certificate public keys, similar to
Mojang's https://api.minecraftservices.com/publickeys route.

Adds two new fields, `playerCertificateKeys` and `profilePropertyKeys`,
to the authlib-injector metadata route at the root of the API. Both are
arrays of strings containing public keys in the same format as the
`signaturePublickey`. If `signaturePublickey` is specified, it will be
counted as both a player certificate key and a profile property key,
which is the status quo.

With this change, authentication server A can forward a texture payload
from authentication server B, and the client could accept a texture
signed by either authentication server.

This change also allows more flexibility for authentication server
operators.

Resolves yushijinhun#254.
As of 1.20, the game now loads profile property public keys and player
certificate public keys dynamically from
https://api.minecraftservices.com/publickeys. That route is currently
replying with more keys than just Mojang's original `ylB4B6m5` key, so
we should embed all of these into authlib-injector rather than just the
one.

This may not be a good solution long-term if the keys change, ideally
the keys could be fetched dynamically like the vanilla game does.
@evan-goode evan-goode force-pushed the unmojang/signaturepublickeys branch from 3c11492 to 0f75d90 Compare December 4, 2024 02:43
@evan-goode
Copy link
Contributor Author

I reworked this to add playerCertificateKeys and profilePropertyKeys as separate fields. But as I was working on it, I came up with another possible path that I want to discuss. What if we didn't add the keys to the authlib-injector metadata response, but instead allowed authentication servers to implement /minecraftservices/publickeys and added a feature flag like feature.dynamic_public_keys to tell authlib-injector to fetch keys from there? This approach would be similar to how feature.enable_profile_key was implemented. This way we can keep more compatible with the Mojang API.

One caveat is that authlib-injector would need to fetch /minecraftservices/publickeys itself since old versions of Minecraft won't dynamically fetch public keys on their own. But we may want to implement this logic anyway considering Mojang's public keys could change: 0f75d90.

If propertiesPublicKeys and signaturePublickey are all given, shall signaturePublicKey be ignored or used? Or just leave it undefined (that they shouldn't both appear)?

(For the previous two options) give them two at the same time for compatibility won't make much sense since profiles will be signed with multiple possible keys and verification will fail anyway.

I think you're right, it doesn't matter much. In this PR's current state, the old signaturePublicKey will be trusted as both a profilePropertyKey and a playerCertificateKey if it's specified.

evan-goode added a commit to evan-goode/drasl that referenced this pull request Dec 6, 2024
@Silverteal
Copy link

What if we didn't add the keys to the authlib-injector metadata response, but instead allowed authentication servers to implement /minecraftservices/publickeys and added a feature flag like feature.dynamic_public_keys to tell authlib-injector to fetch keys from there? This approach would be similar to how feature.enable_profile_key was implemented. This way we can keep more compatible with the Mojang API.

Looks good, if a full document is given.
(see also: feature.enable_profile_key seems never has a detailed implementation doc).

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.

3 participants