Add user agent to headers for all RetroAchievements server calls

[LillyJadeKatrin]

Requested agent syntax is "Dolphin/5.0-23456"

[AdmiralCurtiss]

This is not used outside the code file and does not change depending on the instance, so you can just put it as a const static file-scope variable in the code file.

Also I haven't tested this but I suspect std::string("Dolphin/" SCM_DESC_STR) would also work and actually generate the string at compiletime.

[AdmiralCurtiss]

Also, if the point of this is easy detection of the Dolphin build used, you should probably also include the branch name and possibly commit hash.

[JosJuice]

Having access to this string would also be useful for other parts of Dolphin that speak HTTP. For instance, I made our code that contacts redump.org set a user agent, but I didn't know about the conventions for user agents and therefore skipped the slash. Exposing your newly added user agent string as a getter in Version.h would let me update that code to use the proper version with a slash.

Also, I would very much prefer the hardcoded "Dolphin" to be centralized in Version.cpp, so that people forking Dolphin only have to change it in one place. We wouldn't want forks to confusingly show up as if they are Dolphin with version numbers that don't make sense in the context of upstream Dolphin.

[LillyJadeKatrin]

Not entirely certain I have a complete grasp on what the desired changes are but I made an attempt, anything else specifically you'd like me to modify?

[JosJuice]

Right now, "Dolphin" occurs twice in Version.cpp. Could you make it so it's defined only once, and then used in both GetScmRevStr and GetUserAgentStr?

[AdmiralCurtiss]

Not entirely certain I have a complete grasp on what the desired changes are but I made an attempt, anything else specifically you'd like me to modify?

I mean something like this: 56ad44b

The USER_AGENT_HEADER variable doesn't need to be in the header since no one outside of AchievementManager.cpp needs it. And C string literals can be combined by just writing them next to eachother, which avoids heap allocation for the temporary std::strings.

[AdmiralCurtiss]

Also I have to point out again -- this is technically your problem and you seem to have ignored my last comment about it -- SCM_DESC_STR is not enough information to actually pinpoint a build. You're not including information about whether this is a PR build or anything. For example the current PR build here has a user_agent_str of "Dolphin/5.0-21090", which happens to match https://dolphin-emu.org/download/dev/a50ab40a5fbf4acb179a4c17da5f68a6e7cbf09e/

[LillyJadeKatrin]

Sorry Curtiss, your concern there has been relayed, I just don't have an answer from RC admin on it yet so I haven't changed it.

[sepalani]

I have mixed feelings about this feature. IANAL, but it might raise privacy concerns and issues regarding GDPR. While an exact version in a User-Agent string on modern web browsers most likely won't allow user de-anonymisation, an exact Dolphin build (especially custom ones) and an IP address likely will.

Some questions regarding the implementation:

• Is there an opt-in/opt-out mechanism (like Dolphin analytics) regarding RetroAchievements or is an API key only provided when an user agreed that such pieces of information might be collected?
• Moreover, is the Dolphin version number necessary?
• Can't an API version be provided instead?

This stackoverflow post summarises the User-Agent format nicely: https://stackoverflow.com/a/2601492

For example,

• opt'd out users could use :
  • User-Agent: Dolphin (RetroAchievements/1.0)
• opt'd in users could have more details such as:
  • User-Agent: Dolphin/5.0-23456 (RetroAchievements/1.0; Windows; 5.0-23456-dirty; pr-retroachievements-bugfix)

Additionally, allowing this level of granularity might be used to identify Dolphin and dissociate RetroAchievements (e.g. Dolphin (Analytics) or Dolphin (WiiConnect24)) involvement if needed in other places where HTTP is used. I'm unsure, how useful the User-Agent would be. However, I do believe that adding one (while, afaik, we currently don't have one) shouldn't be done lightly to avoid issues in the future due to its format.

[LillyJadeKatrin]

I'm reaching out to RetroAchievements administration to get some solid answers but for now the only question here I feel reasonably confident answering is this:

• Is there an opt-in/opt-out mechanism (like Dolphin analytics) regarding RetroAchievements or is an API key only provided when an user agreed that such pieces of information might be collected?

RetroAchievements will not function without a user account and will not make API requests unless the user is currently logged in, actively logging in, or actively logging out, and it is in logging in that an API key is created. In order to get an API key, the user must have created a RetroAchievements account with all agreements that that entails. I do not recall what messaging is given to the user before logging in so I don't know if there is still a concern about a user clicking the login button and failing to log in without having a RetroAchievements account.

[wescopeland]

Hello, Dolphin team! I'm an administrator at RetroAchievements, and I contribute heavily to the webdev team, including helping maintain our 3rd party API.

While I'm not privy to the implementation details here, I asked @LillyJadeKatrin to implement this change.

Our Connect API (the endpoints which emulators integrate with) is frequently the target of pentesting and abuse. Relatively soon as a sane security measure we will block all Connect API invocations which do not have a user agent header attached to the HTTP call. We only managed to catch this because of a user poking around with Gamecube achievements currently in development, which triggered some notifications on an internal alerting tool.

We're currently asking that clients identify themselves by attaching a user agent header on each API call. At a bare minimum, we'll require:

IntegrationName/VersionNumber

Any additional metadata beyond that is an added bonus and purely up to the integration team's discretion.

The version number is specifically there so if something goes wrong with an integration's hardcore mode implementation, we can block unlocks from that version of the emulator. @sepalani raises interesting points, and using an internal version number decoupled from Dolphin's actual release would also be fine.

[JosJuice]

I don't think providing an exact version number would make it possible for RetroAchievements to do any tracking that they can't already do using the mandatory login system. With that said, @sepalani, do you feel there is a privacy problem with including the exact version number in the user agent for other integrations like the redump.org one I mentioned?

[sepalani]

@wescopeland

Our Connect API (the endpoints which emulators integrate with) is frequently the target of pentesting and abuse. Relatively soon as a sane security measure we will block all Connect API invocations which do not have a user agent header attached to the HTTP call. We only managed to catch this because of a user poking around with Gamecube achievements currently in development, which triggered some notifications on an internal alerting tool.

I doubt that enforcing a User-Agent will prevent malicious use of the API. A User-Agent can be easily spoofed automatically or programmatically. An attacker can use a proxy such as Burp, ZAP or many well-known tools to change the User-Agent. Many emulators are open source so an attacker can also weaponise the emulator itself.

You might want to have a look at security guides (e.g. OWASP API Security) to improve the API security and adapting it according to the project's constraints. I don't have the full picture of the project but you should rather enforce security around the API token such as a user is responsible for its API token. Such token, then might be used to authenticate the user or ban it if it abuses the service you're providing (i.e. like many SaaS companies providing free or paid API keys to avoid bot/attackers abusing their services).

@JosJuice

do you feel there is a privacy problem with including the exact version number in the user agent for other integrations like the redump.org one I mentioned

Fair point, I do but to a less extent since that only happens when verifying a disc and checking the redump checkbox. However, I do believe (though I am not a lawyer) that it might impact privacy depending on how the third-party (redump, retroachievements, wiilink, etc.) processes these pieces of information. For instance, sending requests to a third-party with Dolphin major version (5.0) won't deanonymise the user. However, sending the full version and the current branch might deanonymise most Dolphin GitHub contributors when compared to their IP address and GitHub activity. While I think that it might not bother most users, it might bother some of them if they weren't aware nonetheless.

[JosJuice]

However, sending the full version and the current branch might deanonymise most Dolphin GitHub contributors when compared to their IP address and GitHub activity.

Considering that the services are optional to use, I think this is an acceptable risk to take given that the services publish a privacy policy describing how they use personal data and they stick to that privacy policy. (Redump.org in particular does not have a privacy policy, and the administrator of the site is nearly impossible to reach. We may want to limit the amount of data sent to Redump.org because of this.)

[wescopeland]

@sepalani

I doubt that enforcing a User-Agent will prevent malicious use of the API. A User-Agent can be easily spoofed automatically or programmatically. An attacker can use a proxy such as Burp, ZAP or many well-known tools to change the User-Agent. Many emulators are open source so an attacker can also weaponise the emulator itself.

I appreciate your insights on the limitations of relying solely on a user agent header for security. It seems there's a slight misunderstanding regarding our security strategy. Our decision to require a user agent header for Connect API calls is indeed part of a broader approach to enhance the platform's security posture. At no point did I suggest it was the sole solution.

We're actively exploring enhancing the security of the Connect API with additional measures. Asking integration partners to include a user agent header shouldn't be a tall order and is quite reasonable IMHO.

[JosJuice]

I think something went wrong with the latest push here.

[LillyJadeKatrin]

I don't understand what's going on here, I've had to fix the same conflicts on this like four times and it's still bugging out, sorry about that. I'll look tonight.

[BhaaLseN]

Try an interactive rebase instead (fetch first!) where you use the origin state, and remove any commits that you don't recognize. It looked a bit like you went forward (picking up the new changes,) then back again (copying the new changes onto an old state, but since it was a rebase it re-created all commits one-by-one.)

Thats also why it now has conflicts, because it goes backwards.