Forum OpenACS Development: Re: a predefined variable like argv0, except for calling proc?

Hi ben,

it is probably easier to check [string match *sched* [ns_thread name]]

-g

Thank you, Gustaf. I may end up doing that.

I've written some procs that would ideally be prevented from execution in other contexts including other scheduling.

As I understand it, declaring a proc "ad_proc -private" only hides the proc from api-doc public view; Private does not prevent execution in other contexts.

The user_id, package_id permissions paradigm works for connections.

For some highly sensitive procs run occasionally by a scheduled proc, user_id and instance_id are not directly available.

Normally this situation is avoided by keeping the code in a page that requires a connection. For these use cases, keeping the code in a page is not practical.

It would be best if these sensitive procs could detect any case outside of expected context ie. a little more specific than *sched*. For example, [string match ${app_prefix}* $calling_proc_name] where app_prefix is like "ad_" defining the package proc context.

It looks like ad_proc could make implementing this easy:

Following this line in ad_proc:

set code_block [lindex $args end]

with something like this:

If { $callback eq "" } {
set code_block "set __calling_proc_name $proc_name_as_passed \n ${code_block}"
}

Anyway, I'll run some experiments in a few days. Some unrelated bugs are taking precedence.

I'll post results here.

cheers,
Ben

Hi Ben,

if you want to make sure, that a proc is not run in any connection thread, the right test is probably [ns_conn isconnected] == 0. You can as well use the tcl callstack (info level, info frame [1]), although i would not recommend it for the scenario you are describing.

Concerning the -public/-private flag for ad_proc: This was added in ancient times to signal a user of a package, which API functions should be used by consumers from other packages. Therefore it is not offered by the API-browser as prime choice. From its intentions -public/-private is similar to "namespace export" in Tcl.

-g

[1] http://www.tcl.tk/man/tcl8.5/TclCmd/info.htm

Thank you, Gustaf, for confirming about the -public/-private flag, ns_conn isconnected, and mentioning the tcl callstack.

I'll avoid the tcl callstack per your suggestion.

"ad_conn isconnected" is in code to check if okay to call ad_conn user_id and package_id.

The extra test of a proc_name check may seem overly cautious. Yet, it doesn't hurt to have an extra hurdle in the event there is a partial security lapse somewhere else in an installation. Packages tend to have different implementations of security requirements. A proc_name check might provide a valuable counterpart to acs permissions if it is fairly simple to implement.

I report back findings..
cheers,

Here's what I've figure out, but first, why this is important:

In some website platforms with plugins, adding an insecure plugin can compromise an entire site. Openacs Pacakges via the package repository are somewhat akin to plugins. Any package that requires a high level of security, such as with health or ecommerce, needs to anticipate that it might be used alongside other packages that have not been vetted as thoroughly for vulnerabilities. Adding a little hurdle can offer some extra time for detecting and stopping an attack in progress without sustaining excessive losses.

Adding " set __calling_proc $proc_name_as_passed" to the body of code defined by each ad_proc doesn't work well. It's too easy to spoof by writing over the set value.

Below is example code that chains a value using upvar from the first proc to all dependent procs that need checking.

The name of the variable is dynamic, so it would take a little work to decode. This variable is audited against the same variable and value defined in the thread's namespace. The name of the variable is defined by ns_thread info, which means that it could be figured out with some work, but I anticipate this is about as much protection as can be expected.

This method could be improved if there is some thread-level information available to the thread via an ns_* call that is not shared with other threads --maybe the time thread began for example.

Anyway, here is the code:

ad_proc -private proc_context_set {
} {
Set floating context in the first proc that calls other procs.
} {
set a [ns_thread id]
upvar 1 $a b
set b $a
set n "::app-namespace::"
set ${n}${a} $a
return 1
}

ad_proc -private proc_in_context_q {
{namespace_name "::app-namespace::"}
} {
Checks if a scheduled proc is running in context of its namespace.
} {
# To work as expected, each proc in namespace must call this function
set a [ns_thread id]
upvar 3 $a $a
if { ![info exists $a] || ![info exists ${namespace_name}${a} ] || [set $a] ne [set ${namespace_name}${a} ]} {
ns_log Warning "proc_in_context_q: namespace '${namespace_name}' no! ns_thread id '${a}' info level '[info level]' namespace current '[namespace current]' "
set context_p 0
} else {
upvar 2 $a $a
set context_p 1
}
return $context_p
}

ad_proc -private proc_that_tests_context_checking {
} {
This is a dummy proc that checks if context checker is working.
} {
ns_log Notice "proc_that_tests_context_checking: info level '[info level]' namespace current '[namespace current]'"
set allowed_p [go_ahead ]
ns_log Notice "proc_that_tests_context_checking: context check. PASSED."
}

proc go_ahead calls proc_in_context_q among other things.

Criticisms and comments welcome,
(^Ben

ps.

An immediate thought is that it could be rendered useless with "info vars", but AFAICT that requires executing inside one of the procs that has the variable defined..

"info vars" returns the variables from the current context. With the right upvar, or with a specified namespace prefix one can get information about variables in different contexts.

The questions is, whether you want to protect against incorrect usage or against intruders. If your system has already an intruder, that can execute Tcl code, you are in a bad shape, since every proc/cmd you define for protection can be reprogrammed on the fly to behave differently, functions can be renamed, variables can be altered, etc. Also all of the Tcl builtins can be altered. For untrusted code, the most promising approach for Tcl is to work with safe slave interpreters [1], which provide a limited environment, containing just a subset of the commands. However, don't expect that much of OpenACS will run there.

So, it is primarily important to protect against intruders. Over there last weeks i have done some work with open source and commercial vulnerability scanners on openacs (some people might have observed some reboots and worse response times as usual of openacs.org). As a result, i have already submitted a series of security updates, and more will come.

all the best
-g
[1] http://www.tcl.tk/man/tcl8.5/TclCmd/interp.htm

LOL, so hiding a variable is a feeble way to protect against intruders --if an attacker has access to code. That's acceptable. It's been a fun exercise. I've learned a lot from it. It was intended to only protect against "incorrect usage" ie calls outside of the UI/api. The ecommerce package avoided this issue by using monolithic code blocks via connections only --difficult to debug or revise.

I'm so glad that security audits continue so proactively. This is so important for maintaining successful app frameworks --including open source ones-- and especially for a time-tested, mature one like OpenACS. Thank you.

I might try Next Scripting next year --or when it comes time to re-factor completed apps. After learning some polymorphism/object oriented principles, I am re-factoring code under development --cutting line count in half while producing cleaner, more legible code.

cheers,
Ben