[Mod] FSC [fsc] Secure Formspec Context

Post Reply
sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

[Mod] FSC [fsc] Secure Formspec Context

by sofar » Post

fsc

This mod is designed to help write more secure formspec handling code. It achieves this by throwing out the concept of "formspec names" entirely and giving each formspec shown to the player a unique, random ID. The player can only then submit form data using this unique ID, and, the handling code can invalidate the ID during processing automatically.

This reduces the risk that an attacker can forge formspec data and send uninvited packets to the server. The server will discard any form data that appears to come from a client that is attempting to use old or incorrect fsc-created forms and will note this event in the minetest log.

Because of the simplicity of the approach, mods will no longer need to focus on basic formspec handling code and can instead spent their time verifying the proper permissions and input data correctness.

This mod also provides a much more simple way to maintain a formspec "context" and pass it along to subsequent formspecs. This makes writing formspec code simpler as the context does not need to be maintained outside the formspec handling or creation code, and no memory leakage needs to be worried about.

A player can also only ever obtain one context, and attempting to use an invalid or outdated context will result in all current valid formspec contexts being revoked. Combined together, all these features make formspecs a lot safer to work with.

Usage

The basic workflow of `fsc` contains of a single function call. Outside of this function call, there are no other API functions or data.

Code: Select all

function fsc.show(name, formspec, context, callback)
	-- `name`: a playername,
	-- `formspec`: a valid formspec string,
	-- `context`: any data, may be `nil`
	-- `callback`: function(player, fields, context).
The return value of `fsc.show()` is always `nil` - it returns nothing.

The callback function will only be called if basic sanity checks on the data pass requirements. You can implement it simply as follows:

Code: Select all

local function callback(player, fields, context)
	-- `player`: player object,
	-- `fields`: table containing formspec data returned by the player client,
	-- `context`: any data, will never be `nil`. If no context was passed
	--            to `fsc.show()`, it will contain `{}`
	return true
end
The return value of the callback may be `nil` or `true`. If you return `nil`, the context is not invalidated, and the player may submit the formspec using the same ID again. This is useful if the player merely selects a list item or otherwise performs an action in the form that does not cause the form to be closed on the client, and you wish to
keep the form open.

If you return `true`, or if you return `nil` and `fields.quit` is set, then the fsc code will invalidate the ID and close the formspec. You should return `true` unless you want to keep the form open to the player.

Making a simple callback handler that shows a new form is therefore relatively straightforward. The below example passes the current context data through to the new form. The old form will close, and the new form will appear to the player with the new content.

Code: Select all

local function callback(player, fields, context)
	local name = player:get_player_name()
	if fields.rename then
		fsc.show(name,
			"field[new_name;What is the new name?;" .. minetest.formspec_escape(context.old_name) .. "]",
			context,
			callback)
		return
	else
		-- do something else
		return true
	end
end
In some cases, you may wish to show a form without having the need for a callback, in case the content is just informational and non-interactive. In that case, you can omit a callback handler by just inserting an empty callback handler, as follows:

Code: Select all

fsc.show(name, formspec, {}, function() end)
Node Formspecs

Node formspecs are not handled. Due to the nature of node/inventory formspecs, it is inherently impossible to perform the same checks on node/inventory formspecs as `fsc` can do for (normal) formspecs.

License: FormSpec Context ('fsc') mod for minetest, licensed under the `ISC` license:
Dependencies: none

Github: https://github.com/sofar/fsc
Archive (master): https://github.com/sofar/fsc/archive/master.zip

User avatar
rnd
Member
Posts: 220
Joined: Sun Dec 28, 2014 12:24
GitHub: ac-minetest
IRC: ac_minetest
In-game: rnd

Re: [mod]FSC[fsc] Secure Formspec Context

by rnd » Post

Much better than default way. Instead of sending names with sometimes weird extra(position/other stuff inside name) you simply send randomized 'token' and remember you sent it, and you can store 'extra data' inside that 'context' _data table.

1.I think this comment is not accurate:

Code: Select all

if data.name ~= name then
minetest.log("error", "fsc: possible hash collision or exploit (name mismatch)")
If this were to happen, it would mean that player 'name' would have gotten written some other 'name1' when doing fsc.show but clearly

Code: Select all

_data[name] = {
		id = id,
		name = name,
		context = context,
		callback = callback,
}
it sets same 'name'. What you are implying is saying there is BUG IN LUA itself, in implementation of its hashtable, so that _data[name] and _data[name1] can point to same object sometimes. There is a thing called 'collision resolution' which ensures this does not happen - ever. Either by chaining, open addressing,...

2. misleading naming: variable 'context'. What is context is actually '_data' table, variable 'context' used here is just extra data you remember inside '_data' that goes together with form, you never do anything else with it - at least if you compare with what was said at 'secure formspec' forum post.
1EvCmxbzl5KDu6XAunE1K853Lq6VVOsT

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [mod]FSC[fsc] Secure Formspec Context

by sofar » Post

Thanks. I've updated the comments and error message here, although it's somewhat circumstantial and doesn't change any functionality itself. Still, I appreciate the review!

User avatar
stu
Member
Posts: 923
Joined: Sat Feb 02, 2013 02:51
GitHub: stujones11
Location: United Kingdom

Re: [Mod] FSC [fsc] Secure Formspec Context

by stu » Post

I really like what this does but surely things as important as this belong in the builtin scripts. You are a core developer, right?

Nice job all the same, let's hope you can encourage enough modders to actually use it :)

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] FSC [fsc] Secure Formspec Context

by sofar » Post

stu wrote:I really like what this does but surely things as important as this belong in the builtin scripts. You are a core developer, right?

Nice job all the same, let's hope you can encourage enough modders to actually use it :)
Yes, this fits my standards for `it should be default`. However, as I wrote this, I quickly realized that it changes formspec handling code in a subtle way that may require significant rewrite and/or thought, plus it makes some methods of calling handlers entirely obsolete, stuff used in the inventory formspec for instance works very different.

So, as usual with code I write, I tend to post PoC or working and solid code separately first and see whether the interest and support for the mechanism is there. The API isn't particularly simple, although it's arguably a lot better than the old code. So I might have to adjust little things like documentation, and even though it's small, there might still be a bug in it.

Also, `fsc` is still a terrible name ;)

Chem871
Member
Posts: 999
Joined: Sat Aug 19, 2017 21:49
GitHub: Chemguy99
In-game: Chem Nyx
Location: My Basement's Attic

Re: [Mod] FSC [fsc] Secure Formspec Context

by Chem871 » Post

I think you got th "f" and the "s" in the mod name switched.
What is SCP-055?

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] FSC [fsc] Secure Formspec Context

by sofar » Post

Chem871 wrote:I think you got th "f" and the "s" in the mod name switched.
lol, no. The original acronym stood for "FormSpec Context"... =)

bell07
Member
Posts: 604
Joined: Sun Sep 04, 2016 15:15
GitHub: bell07

Re: [Mod] FSC [fsc] Secure Formspec Context

by bell07 » Post

This mod introduces context in formspecs in other words a state or session. It is an design decision that should not be default. Each mod should decide byself if the implementation is stateful or stateless.

For full-stated mods the smartfs exists, that support states for all inventory types including nodemeta and inventory. Maybe overdone, the state is for each formspec element oO

As food for thought for default/engine, maybe the REST concept should be implemented in formspec like the URL-Parameters in HTTP. See https://en.wikipedia.org/wiki/Represent ... e_transfer
In the current implementation only "formspec name" and "fields" table ist transferred to indentiy the state. Sending of andditional custom data back from client could be usefull for state transfer. An proposal: Maybe a hidden 'context[a;b]' formspec element can be implemented as next that sends allways a fields.a = 'b' back to the form each input?

At this point I have again the question why the minetest implements an own proprietary formspec format instead of using something standardized, like SOAP-XML + XSL client-site or HTML/CSS

User avatar
Wuzzy
Member
Posts: 4786
Joined: Mon Sep 24, 2012 15:01
GitHub: Wuzzy2
IRC: Wuzzy
In-game: Wuzzy
Contact:

Re: [Mod] FSC [fsc] Secure Formspec Context

by Wuzzy » Post

Thank you for trying to solve the problem, but: It's only a mod. :-(
If I get a dollar for every mod which supposedly fixes an engine bug / shortcoming … xD

There's a simple test for mods like this: Is this a mod which practically every subgame wants/needs? If the answer is “yes”, then it probably needs to go into builtin or even engine. And for this mod, the answer is definitely “yes”. :P

The real problem is that the current official formspec API makes it is incredibly easy to screw up, as you demonstrated here: viewtopic.php?f=47&t=19129

It's an obvious design flaw that modders have to jump through a lot of (undocumented!) hoops in order to secure their formspecs. And none of this is properly documented. No, a forum post is NOT proper documentation. :P

IMO formspec API needs a redesign anyway. This is not even controversial. Official docs state that formspecs are experimental. The modders have been warned. Minetest is still in Alpha. So please don't be afraid to break things if the new revision is a major improvement. Especially if it's fixing some serious security/design flaws. And formspecs are in serious need of a redesign anyway. Security is just another of many things wrong with formspecs.
Maybe it would be a good idea to analyze the current (bad) formspec API, collect all the design flaws and problems (like weird coordinates, security fails, …), then use the new information to carefully design a new (breaking) API and fix everything at once. The idea is to break the API only once, but do it right. :-)

IMO it's incredibly harmful for Minetest to be overly conservative, especially at this stage of development, because you will stick with legacy and halfplemented or half-broken features forever.

User avatar
ExeterDad
Member
Posts: 1717
Joined: Sun Jun 01, 2014 20:00
In-game: ExeterDad
Location: New Hampshire U.S.A

Re: [Mod] FSC [fsc] Secure Formspec Context

by ExeterDad » Post

Wuzzy wrote:it's incredibly harmful for Minetest to be overly conservative, especially at this stage of development, because you will stick with legacy and halfplemented or half-broken features forever.
Beautifully written.
/me get's emotional

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] FSC [fsc] Secure Formspec Context

by sofar » Post

Wuzzy wrote: There's a simple test for mods like this: Is this a mod which practically every subgame wants/needs? If the answer is “yes”, then it probably needs to go into builtin or even engine. And for this mod, the answer is definitely “yes”.
I already stated before:
sofar wrote:Yes, this fits my standards for `it should be default`.
sofar wrote:So, as usual with code I write, I tend to post PoC or working and solid code separately first and see whether the interest and support for the mechanism is there.
How fast and in what shape this makes it into core, default or something else is likely inevitable, but it's not clear to me how it would look. With code, there's something to discuss and play with, which only will speed it up.

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] FSC [fsc] Secure Formspec Context

by sofar » Post

bell07 wrote:An proposal: Maybe a hidden 'context[a;b]' formspec element can be implemented as next that sends allways a fields.a = 'b' back to the form each input?
We're trying to improve security, not make it worse. That is just a terrible idea, sorry.

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] FSC [fsc] Secure Formspec Context

by sofar » Post

bell07 wrote:At this point I have again the question why the minetest implements an own proprietary formspec format instead of using something standardized
I don't think you understand the meaning of the word "proprietary". Being OSS, formspecs as minetest implements them are per definition not proprietary.

bell07
Member
Posts: 604
Joined: Sun Sep 04, 2016 15:15
GitHub: bell07

Re: [Mod] FSC [fsc] Secure Formspec Context

by bell07 » Post

We're trying to improve security, not make it worse. That is just a terrible idea, sorry.
I did not realized the focus on security. Agree my proposal take it more worse for modders without security knowledge. But used right the proposed formspec enhancement can be usefull. For example to store the selected page in inventory formspec for example (button="1" on page="2" instead of button "page2_button1")

For security the
1. TOCTOU
2. Never Trust User Provided Data
is most important as you described. The "scrambling" formspec names does not help for this both points, to enforce the server-site context state does help only partially for 2.

I think for beter security the permission checks should be more supported and connected between formspec creation and provided data. That means if you define in formspec definition the field "target" is allowed only for server privilege, this privilege is checked again if data is received back automatically for example.
Agree the node position in formspec name is bad by design. We have nodemeta-formspecs for this use case already. But the nodemeta-formspecs are created without player-relation so no permissions are possible at formspec creation time.
I don't think you understand the meaning of the word "proprietary"
Agree, I use the word wider then defined. All non-standard formats, protocols or dialects used exclusively in just one solution is proprietär for me. German "proprietär" definition is between the english and my one ;-)
But it is not the point, my question was why the wheel was reinvented again.

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] FSC [fsc] Secure Formspec Context

by sofar » Post

bell07 wrote:Agree my proposal take it more worse for modders without security knowledge. But used right the proposed formspec enhancement can be usefull. For example to store the selected page in inventory formspec for example (button="1" on page="2" instead of button "page2_button1")
Lua doesn't offer you any way of determining that "fields.a" was provided by yourself and trustworthy, or that "fields.a" was provided by the player and thus unsafe. This is why you have to separate "fields" from "context" and maintain the separation. Mixing data of the 2 sources in "fields" would make it impossible (not difficult!) to maintain security properly.

bell07
Member
Posts: 604
Joined: Sun Sep 04, 2016 15:15
GitHub: bell07

Re: [Mod] FSC [fsc] Secure Formspec Context

by bell07 » Post

Lua doesn't offer you any way of determining that "fields.a" was provided by yourself and trustworthy, or that "fields.a" was provided by the player and thus unsafe.
I know ;-). Never Trust User Provided Data.
But it allow client-site to request or trigger anything directly (stateless) from page 3 without previous paging to the page 3. In interaction with client-site mods very usefull.
Of course TOCTOU needs to be happen to be sure the player is allowed to do anything on page 3.
As I read the first time about REST concepts with all session related data in URL I had the same concerns. But it enforces and educates the developer to consider the TOCTOU. But agree this enhancement proposal is off-topic, as I said I overlooked the security relation in your topic.

Back to the security: The nodemeta formspecs are maybe the beter example for the mentioned security issue. The formspecs are generated without the player relation, or maybe with the placer-relation in "on_place", or maybe with last-used-player relation. Between the formspec creation and usage maybe the server was restarted and mods updated or replaced. It is most difficult example but if solution found, the solution will be more general. Maybe you enhance your PoC to the secure nodemeta formspecs?
My proposal is to store the "private context" in playermeta for "usual" formspecs and in nodemeta for nodes.
I seen the https://github.com/minetest/minetest/pull/5702/ but not tested yet. Usable to secure the node+player related context? Can the "formspec" be secured on node?

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] FSC [fsc] Secure Formspec Context

by sofar » Post

bell07 wrote:The nodemeta formspecs are maybe the beter example for the mentioned security issue. The formspecs are generated without the player relation, or maybe with the placer-relation in "on_place", or maybe with last-used-player relation. Between the formspec creation and usage maybe the server was restarted and mods updated or replaced. It is most difficult example but if solution found, the solution will be more general. Maybe you enhance your PoC to the secure nodemeta formspecs?
I don't believe it's possible. There is no guarantee in the engine that it can't be bypassed. The only sane thing to do is to remove all node formspecs and call `fsc.show` in the on_rightclick() methods.

Exploiting node formspecs is trivial, too. The engine has only very limited tools to prevent abuse.

Byakuren
Member
Posts: 818
Joined: Tue Apr 14, 2015 01:59
GitHub: raymoo
IRC: Hijiri
In-game: Raymoo + Clownpiece

Re: [Mod] FSC [fsc] Secure Formspec Context

by Byakuren » Post

This seems like something I would use. Better than keeping a table for every stateful formspec.
Every time a mod API is left undocumented, a koala dies.

User avatar
Wuzzy
Member
Posts: 4786
Joined: Mon Sep 24, 2012 15:01
GitHub: Wuzzy2
IRC: Wuzzy
In-game: Wuzzy
Contact:

Re: [Mod] FSC [fsc] Secure Formspec Context

by Wuzzy » Post

So what would need to be done in order to make this go into the official Lua API?

So is this mod basically a proof of concept so far?

sofar
Developer
Posts: 2146
Joined: Fri Jan 16, 2015 07:31
GitHub: sofar
IRC: sofar
In-game: sofar

Re: [Mod] FSC [fsc] Secure Formspec Context

by sofar » Post

Wuzzy wrote:So what would need to be done in order to make this go into the official Lua API?

So is this mod basically a proof of concept so far?
ITB uses it everywhere. It's also stable as-is - the API isn't going to evolve any further at this point I think. I don't have an urge to submit this, though.

User avatar
Festus1965
Member
Posts: 4181
Joined: Sun Jan 03, 2016 11:58
GitHub: Festus1965
In-game: Festus1965 Thomas Thailand Explorer
Location: Thailand ChiangMai
Contact:

Re: [Mod] FSC [fsc] Secure Formspec Context

by Festus1965 » Post

sofar wrote:
Thu Dec 19, 2019 17:40
ITB uses it everywhere. It's also stable as-is - the API isn't going to evolve any further at this point I think. I don't have an urge to submit this, though.
AS that mod is uses as depends off Zoonami I wanna know if I should be worried, as player very often, some near with wanted ...

Code: Select all

ACTION[Server]: 'player' submitted formspec ('fsc:code') but the name of the formspec doesn't match the expected name ('fsc:code'), possible exploitation attempt
and that sometimes in a long row repeating ...
Human has no future (climate change)
If urgend, you find me in Roblox (as CNXThomas)

isaiah658
Member
Posts: 168
Joined: Sun Jan 24, 2016 14:58
Contact:

Re: [Mod] FSC [fsc] Secure Formspec Context

by isaiah658 » Post

Festus1965 wrote:
Fri Dec 29, 2023 15:49
AS that mod is uses as depends off Zoonami I wanna know if I should be worried, as player very often, some near with wanted ...

Code: Select all

ACTION[Server]: 'player' submitted formspec ('fsc:code') but the name of the formspec doesn't match the expected name ('fsc:code'), possible exploitation attempt
and that sometimes in a long row repeating ...
The first possibility is that this could be a bug with my mod. There might be a problem with one of my formspecs. If a formspec sends an "update" to FSC, but the player is still shown the old formspec, that will cause this problem. If I had to guess, this is probably what is happening.

The second possibility is that a player is using a hacked client to send false formspec data. I find this unlikely, but I'm not too familiar with how often players try to cheat on servers.

If you happen to figure out more about what is causing it, please let me know. I will try to get the error to occur on a single player world.

User avatar
Festus1965
Member
Posts: 4181
Joined: Sun Jan 03, 2016 11:58
GitHub: Festus1965
In-game: Festus1965 Thomas Thailand Explorer
Location: Thailand ChiangMai
Contact:

Re: [Mod] FSC [fsc] Secure Formspec Context

by Festus1965 » Post

isaiah658 wrote:
Sat Dec 30, 2023 19:37
The first possibility is that this could be a bug with my mod. There might be a problem with one of my formspecs. If a formspec sends an "update" to FSC, but the player is still shown the old formspec, that will cause this problem.
Until now I thought that player can use an command like /fsc to interact,
but I didn't see any option a player can 'send' a code.

I also used the gui from zoonami to open, close, switch between windows as a normal player and keep a watch on the terminal.
I was NOT causing any of this Warning messages.

So I have to learn how fsc is used/called inside zoonami to get a glimpse what might happen to trigger this.
Human has no future (climate change)
If urgend, you find me in Roblox (as CNXThomas)

Post Reply

Who is online

Users browsing this forum: No registered users and 24 guests