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

Enforce the use of no custom signatures in first-party plugins #22

Closed
philpax opened this issue May 6, 2022 · 14 comments
Closed

Enforce the use of no custom signatures in first-party plugins #22

philpax opened this issue May 6, 2022 · 14 comments

Comments

@philpax
Copy link
Contributor

philpax commented May 6, 2022

This one is quite controversial, but technically not impossible. We can disable all sigscanning/etc functionality in first-party plugins, so that developers are forced to use safe(r) APIs that can be updated independently of the plugin.

This would be quite difficult and annoy quite a lot of people, but it would make the case for 1pp respecting the rules even stronger, and allow for a stronger overall Dalamud and ClientStructs (as 2pp and 3pp would need to upstream their changes to be able to go 1pp.)

@NadyaNayme
Copy link

I think meeting somewhere in the middle would be more productive. As discussed on the Discord there's a few downsides to adding additional overhead and slowing devs down for what would mostly just be bureaucracy with some benefits of having total compliance.

I think advocating for the use of ClientStructs and encouraging devs to add sigs they use, even if they continue using the sigs outside of CS, has a lot of merit. Making it easier to contribute to ClientStructs by lowering any barriers to contributing sigs would also make devs more likely to to do so. If the process is as painless as possible many devs even stated they'd prefer to move all their sigs into client structs even without being forced to do so.

I'm not sure I can agree with mandating it - but it being encouraged and a few people advocating/pushing for contributions would go a long way and carry most, but not all, of the benefits of mandating it without incurring any of the major downsides of such a requirement.

I wouldn't let perfect be the enemy of good. Mandating CS might be a pipedream for a perfect world but encouraging contributions and making it easier to contribute is still a much better world even if it's only 75% of the way to perfection.

@philpax
Copy link
Contributor Author

philpax commented May 6, 2022

Understandable! I agree with that, but before committing to that I'd like to do a quick survey of the 1pp ecosystem to know how much breakage would ensue from this. There's no particular rush on this, so we can kick the can down the road until/if something bad happens :>

@Helios747
Copy link

Definitely do not agree with mandating. Even from a stability standpoint requiring that everybody shove their sigs into clientstructs isn't much of a win.

I think the suggestion can be made at plugin submission time and should be, but holding up submission because you're waiting for a PR to be merged to clientstructs doesn't feel worth it. I'm not sure I agree with the argument of lightening maint burden on plugin devs, because it just shifts that burden to clientstructs maintainers. And don't a lot of sigs just stay broken after major patches until an interested dev does the work to PR the fix? So we're just back to the devs maintaining their own sigs they're interested in.

@Critical-Impact
Copy link

I think the suggestion can be made at plugin submission time and should be, but holding up submission because you're waiting for a PR to be merged to clientstructs doesn't feel worth it. I'm not sure I agree with the argument of lightening maint burden on plugin devs, because it just shifts that burden to clientstructs maintainers. And don't a lot of sigs just stay broken after major patches until an interested dev does the work to PR the fix? So we're just back to the devs maintaining their own sigs they're interested in.

I agree, as an example there was a sig I pulled off "Good Memory" that allowed you to input the ID of item that can be acquired and it'd return whether or not you had it. I was able to replace it with my own set of 2 sigs and it mostly works but I don't have the full technical know how to know if this is actually the correct function to use. Reverse engineering is hard and sometimes the functions we rely on change enough that it takes some serious dedicated time to work out how to replace the function we rely on.

I also agree with NadyaNayme, I think outright banning them is unproductive and instead we should focus our efforts on making it easier to contribute to clientstructs/dalamud.

If I could boil down my concerns with limiting custom sigs in first party plugins they are

  • Are we going to just push everyone away to 2nd party/3rd party repositories if we make it too strict
  • Everyone here is doing this in their free time, if we make dalamud and clientstructs a central linchpin to making everything work, is this going to put too much strain on the maintainers?
  • If a plugin relies on a specific version of clientstructs and there's a lead time on that version coming out we'd have to work out a way to handle that.

@PunishedPineapple
Copy link

PunishedPineapple commented May 6, 2022

Understandable! I agree with that, but before committing to that I'd like to do a quick survey of the 1pp ecosystem to know how much breakage would ensue from this.

Just from the perspective of someone with a lot less of this type of RE experience: I do not feel confident at all documenting the things that I use as a part ClientStructs. I do try to understand as fully as I can what the functions and structures that I use do in their local context, as well as how they behave overall in the situations in which I use them, but I absolutely do not feel confident in naming and documenting them in the context of the entire game structure. My level of RE isn't suitable as-is for the entire community to use in a way that shows the "big picture". Whether that's intended to be an acceptable level of RE for first party plugins.

Anyway, if a no custom signatures rule were to be enforced; for the things that I've RE'd myself, this means that I'd either be giving someone my RE notes and asking them to document it, which would be a really crappy thing to ask of someone, or the plugins would either just be abandoned or go 3rd party (which I'd not like in general, because I do like having an experienced eye go over the general concept of what I'm doing, looking for any abject stupidity).

@anna-is-cute
Copy link

This is a non-starter.

Essentially, this would gimp 1PP and require new innovation and iteration to be shunted exclusively into 3PP, which are much less popular and would slow the entire process down. As other people have said, pushing things into ClientStructs increases the burden on those maintainers and unnecessarily bloats it, especially for random functions that are used by one plugin and aren't well-described. It's hard to know exactly where a function belongs (in what class, etc.) a lot of the time, unless you're aers.

How exactly do you use hooks or modify data at pointer offsets without being able to use sigs? I think you mostly don't, and most 1PP would be forced to move to 3PP or else severely curtail their functionality. I think in this scenario, I would just pull all my plugins and make them all third-party, and I wouldn't be surprised if others did the same.

Chat 2, for example, uses 40 signatures on top of existing ClientStructs functions. This is a plugin that certain people have been clamouring for and would be a fine 1PP, but some of the functions, hooks, and pointers it uses are very niche and not necessarily worth adding to ClientStructs, especially if only one plugin is consuming them. It's better to let the plugin maintainer handle that maintenance.

Finally, this will make 1PP less stable. The following example is assuming we're not adding every tiny thing relating to the game's code to ClientStructs, since that's unreasonable. Often, plugins write to struct/object offsets in the game's code, e.g. *(byte*) (somePtr + 0xA0) = 1;. Plugins can actually obtain the + 0xA0 from a signature and reference that offset, which makes the plugin more resilient to game updates. However, this can't be done in a situation where no custom signatures are allowed.

@philpax
Copy link
Contributor Author

philpax commented May 7, 2022

Alright, this is a pretty controversial proposal! I think it's pretty clear the full no-signatures-in-1PP proposal will be rejected. With that being said, I'd like to propose a smaller change: "safe" and "unsafe" plugins, with plugins defaulting to "safe" by default. You do not get access to signatures / direct game state under "safe" mode, and this would be specified as part of your manifest. "Unsafe" plugins get full access to the game.

This would let users know which plugins are using functionality purely from Dalamud, and are thus more likely to be updated faster / less likely to break during updates. This may also provide some gentle "passive pressure" to encourage developers to submit their findings upstream so that they can get that sweet [SAFE] badge and stroke their own ego.

Additionally, we could spend some time going through the sigs and manipulation that's being done throughout the ecosystem right now, and pull as much of it as we can into Dalamud. This is a net-positive move (nobody has to update their code, and nothing will break), and it'll allow for a wider range of plugins to be written.


Addressing the other feedback more concretely:

I think advocating for the use of ClientStructs and encouraging devs to add sigs they use, even if they continue using the sigs outside of CS, has a lot of merit. Making it easier to contribute to ClientStructs by lowering any barriers to contributing sigs would also make devs more likely to to do so. If the process is as painless as possible many devs even stated they'd prefer to move all their sigs into client structs even without being forced to do so.

At a minimum, we should try to make the process more painless. I'm not sure what the best way to do that is, but I've just suggested a reverse engineering subteam at #18 that could assist people with submitting their findings and write up guidelines for reversed code to submit.

For this to be ergonomic, we'd really need a way to make running with a custom Dalamud and/or ClientStructs as easy as possible, so that developers can add new sigs / whatever else and test them without dreading getting involved with the project at all.

I think the suggestion can be made at plugin submission time and should be, but holding up submission because you're waiting for a PR to be merged to clientstructs doesn't feel worth it. I'm not sure I agree with the argument of lightening maint burden on plugin devs, because it just shifts that burden to clientstructs maintainers. And don't a lot of sigs just stay broken after major patches until an interested dev does the work to PR the fix? So we're just back to the devs maintaining their own sigs they're interested in.

This is a fair point, and it's not wrong. That being said, the clientstructs folks have tools to match signatures and workflows around handling game version migrations, which means they're in a better position to handle it than Joe McRandoDev. It'd be good to get their perspective on it, if possible (@aers, @pohky)? We'd also be writing guidelines to help with reversing submissions.

And yeah, developers would still have to maintain their sigs to some extent, but in theory everyone benefits, instead of each developer who has the signature having to fix it themselves. Again, not necessarily a problem if we make the workflow as straightforward as possible.

I agree, as an example there was a sig I pulled off "Good Memory" that allowed you to input the ID of item that can be acquired and it'd return whether or not you had it. I was able to replace it with my own set of 2 sigs and it mostly works but I don't have the full technical know how to know if this is actually the correct function to use. Reverse engineering is hard and sometimes the functions we rely on change enough that it takes some serious dedicated time to work out how to replace the function we rely on.

If we develop a reverse engineering subteam, they'd be the people who'd help you come up with a consistent solution. That's still a hypothetical, though, and you're right that does move some of the burden to that subteam.

Anyway, if a no custom signatures rule were to be enforced; for the things that I've RE'd myself, this means that I'd either be giving someone my RE notes and asking them to document it, which would be a really crappy thing to ask of someone, or the plugins would either just be abandoned or go 3rd party (which I'd not like in general, because I do like having an experienced eye go over the general concept of what I'm doing, looking for any abject stupidity).

Yeah - again, I don't think we could do this without a dedicated reversing support team. I hadn't realised so many people were unconfident about their RE / organisation skills!

Essentially, this would gimp 1PP and require new innovation and iteration to be shunted exclusively into 3PP, which are much less popular and would slow the entire process down.

This would be combined with #4, so that innovation can still happen, and users can still easily test for trusted developers. If we went through with this to completion (which is unlikely), I suspect we'd make it only apply to mainline 1PP, so that you could still use signatures on testing to hash out your game interface before you submit it upstream.

#4 also means you could just keep all of your plugins on a blessed second-party repository and avoid mainline 1PP restrictions entirely. That would also be an option, if not mildly regrettable.

pushing things into ClientStructs increases the burden on those maintainers and unnecessarily bloats it

I'm not sure it's necessarily bloat if it's in the game and someone has used it, and someone else might use it. Point taken, though, it's a tough balance to strike.

How exactly do you use hooks or modify data at pointer offsets without being able to use sigs?

Good point - maybe this would be better viewed as a general restriction on unsafe functionality, instead of just signatures? Going to need to make your game interface quite deep and robust to ensure that you can always provide a safe interface, though.

Finally, this will make 1PP less stable.

Eh. I can see your line of reasoning here, but under the "no unsafe" model this wouldn't come up at all as it wouldn't be allowed to begin with. Even if we were in a scenario where 1PP signatures were banned for purpose of game state modification, but we still allowed modifying game state, we could allow readonly access to signatures. That's not a super-realistic case, though.


Based on the feedback, I think the full version of this proposal would be pretty unpopular. I'm interested to see what everyone thinks of:

  • explicitly safe/unsafe plugins, so that both users and developers can make informed choices and be encouraged to push upstream where possible. Safe plugins would ban the use of any unsafe code, and you would be forced to use Dalamud's game interface.
  • improving the workflow for submitting research, including finding the right people to talk to, having guidelines, and being able to test locally
  • spending some time upstreaming the existing sigs out there in the ecosystem, so that we have a better idea of what the discrepancy is between what's documented and what's out there

@MidoriKami
Copy link

MidoriKami commented May 13, 2022

I'm just now realizing that I should have been following the discussions in this repo for a while now. I am a bit late to the party but feel my input is important here since DailyDuty would be heavily effected by this.

Expanding on the concept of handing off the sigs to someone else and making them deal with it doesn't really help the situation where a sig didn't break, but it just changed, and potentially changed to some other unrelated function.

As an example, a sig Anna was using for Chat2 all of a sudden triggers a chocobo racing countdown thing. The sig itself didn't break, but the function it linked to is the wrong one. If this code was in ClientStructs then any plugin using that signature would now cause this behavior. Fortunately the chocobo racing countdown is innocent, but what if it was something more detectable? (mistaken assumption see Anna's response)

Since I am maintaining my own sigs for DailyDuty if something breaks, I know what breaks, where, and probably why, and can rather easily fix it. Being forced to upstream these changes would do nothing but complicate this process in my opinion.

I also feel that being labelled as "unsafe" when I am extremely careful about the behavior of the game functions I use seems a bit disingenuous.


Addressing the specific feedback that was requested

explicitly safe/unsafe plugins

To a user "unsafe" would imply that it is buggy, crashy, or problematic, is it really fair to the user to tell them "hey this is unsafe" when in reality it is perfectly safe?

improving the workflow for submitting research, including finding the right people to talk to, having guidelines, and being able to test locally

I think this is very much needed, I would love to more easily share my discoveries with those that could use them in other ways.

spending some time upstreaming the existing sigs out there in the ecosystem, so that we have a better idea of what the discrepancy is between what's documented and what's out there

This might turn into a lot more work than expected, not specifically relating to sigs, there are some data structures that I would love to push into client structs, but don't know how to. The additional work part is if these structs were going to be public then I feel I would have to work a bit harder on making them "nice" first.

@anna-is-cute
Copy link

As an example, a sig Anna was using for Chat2 all of a sudden triggers a chocobo racing countdown thing. The sig itself didn't break, but the function it linked to is the wrong one. If this code was in ClientStructs then any plugin using that signature would now cause this behavior. Fortunately the chocobo racing countdown is innocent, but what if it was something more detectable?

I hate to say it, but this is wrong. It wasn't a signature that broke. I was actually using ClientStructs! I was calling a virtual function by its index, and since new functions got added to UiModule, I was calling the wrong function.

@MidoriKami
Copy link

Oh that's an important detail. Then I retract my remarks about the risks there.

I had thought that with that being possible there could be significant risk of very unsafe code being called in some plugins my mistake.

@philpax
Copy link
Contributor Author

philpax commented May 14, 2022

To a user "unsafe" would imply that it is buggy, crashy, or problematic, is it really fair to the user to tell them "hey this is unsafe" when in reality it is perfectly safe?

Thanks for weighing in! Really appreciate it. I agree that the wording on "safe and unsafe" is probably not the best - you're right that it would absolutely dissuade people from downloading plugins that are entirely safe - but I think it's useful to capture that some plugins are doing freakier things with the game than most, and that might result in weird/unexpected behaviour, slow updates, or other some such things.

@kalilistic and @karashiiro have suggested a variant of this that's similar to permissions models on Android / iOS that explicitly spell out what the plugin's doing to your game, so that you can make a more informed decision. Problem is that it's hard to really consistently source that information, especially because plugins with signature/memory access can do whatever they want. Might be interesting to do the "signature upstreaming" work and then try to categorise it, so that users can be shown a list like

  • Can modify characters on-screen
  • Can send messages to chat
  • Can teleport you
  • Has unrestricted access to the game (the "unsafe" model I was discussing)

If that's more appealing, I'll close this issue and make a new one that describes that instead.

improving the workflow for submitting research, including finding the right people to talk to, having guidelines, and being able to test locally

I think this is very much needed, I would love to more easily share my discoveries with those that could use them in other ways.

spending some time upstreaming the existing sigs out there in the ecosystem, so that we have a better idea of what the discrepancy is between what's documented and what's out there

This might turn into a lot more work than expected, not specifically relating to sigs, there are some data structures that I would love to push into client structs, but don't know how to. The additional work part is if these structs were going to be public then I feel I would have to work a bit harder on making them "nice" first.

Yup :> We'll figure something out after #18, probably

@MidoriKami
Copy link

Another consideration is scope creep, when I started working on DailyDuty I didn't expect to do anything with sigs or reverse engineering the game, but as the project progressed further and further it just grew.
How would you handle this for multiple plugins at a time?

I think signature upstreaming as a priority might shed some light on some of thing people are in fact doing with their sigs. 👍

@philpax
Copy link
Contributor Author

philpax commented May 14, 2022

With the permissions model, I'd imagine that you'd just request additional permissions (that is, "permissions": ["unrestricted-memory-access"] in the manifest). With the original proposal, yeah, that would suck for rapid iteration.

@philpax
Copy link
Contributor Author

philpax commented Aug 28, 2022

Closed in favour of #53.

@philpax philpax closed this as completed Aug 28, 2022
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

No branches or pull requests

7 participants