perf: move sandboxed environment to the C side (#2058)

This PR uses `zend_array_dup` to simplify and optimize the environment sandboxing
logic. It also guarantees no environment leakage on FrankenPHP restarts.
This commit is contained in:
Alexander Stecher
2026-02-26 22:34:54 +01:00
committed by GitHub
parent 25ed020036
commit 8f4412cbbf
14 changed files with 158 additions and 213 deletions

View File

@@ -61,6 +61,16 @@ func escapeMetricLabel(s string) string {
return strings.ReplaceAll(s, "\\", "\\\\")
}
func TestMain(m *testing.M) {
// setup custom environment vars for TestOsEnv
if os.Setenv("ENV1", "value1") != nil || os.Setenv("ENV2", "value2") != nil {
fmt.Println("Failed to set environment variables for tests")
os.Exit(1)
}
os.Exit(m.Run())
}
func TestPHP(t *testing.T) {
var wg sync.WaitGroup
tester := caddytest.NewTester(t)
@@ -957,9 +967,6 @@ func testSingleIniConfiguration(tester *caddytest.Tester, key string, value stri
}
func TestOsEnv(t *testing.T) {
require.NoError(t, os.Setenv("ENV1", "value1"))
require.NoError(t, os.Setenv("ENV2", "value2"))
tester := caddytest.NewTester(t)
tester.InitServer(`
{

View File

@@ -5,13 +5,13 @@ package caddy
import (
"encoding/json"
"errors"
"os"
"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/caddyconfig"
"github.com/caddyserver/caddy/v2/modules/caddyhttp"
"github.com/dunglas/frankenphp"
"github.com/dunglas/mercure"
mercureCaddy "github.com/dunglas/mercure/caddy"
"os"
)
func init() {
@@ -67,5 +67,5 @@ func createMercureRoute() (caddyhttp.Route, error) {
},
}
return mercureRoute, nil;
return mercureRoute, nil
}

View File

@@ -253,8 +253,8 @@ func cmdPHPServer(fs caddycmd.Flags) (int, error) {
if mercure {
mercureRoute, err := createMercureRoute()
if err != nil {
return caddy.ExitCodeFailedStartup, err
}
return caddy.ExitCodeFailedStartup, err
}
subroute.Routes = append(caddyhttp.RouteList{mercureRoute}, subroute.Routes...)
}

110
env.go
View File

@@ -3,114 +3,32 @@ package frankenphp
// #cgo nocallback frankenphp_init_persistent_string
// #cgo noescape frankenphp_init_persistent_string
// #include "frankenphp.h"
// #include <Zend/zend_API.h>
// #include "types.h"
import "C"
import (
"os"
"strings"
"unsafe"
)
func initializeEnv() map[string]*C.zend_string {
env := os.Environ()
envMap := make(map[string]*C.zend_string, len(env))
for _, envVar := range env {
//export go_init_os_env
func go_init_os_env(mainThreadEnv *C.zend_array) {
for _, envVar := range os.Environ() {
key, val, _ := strings.Cut(envVar, "=")
envMap[key] = C.frankenphp_init_persistent_string(toUnsafeChar(val), C.size_t(len(val)))
}
return envMap
}
// get the main thread env or the thread specific env
func getSandboxedEnv(thread *phpThread) map[string]*C.zend_string {
if thread.sandboxedEnv != nil {
return thread.sandboxedEnv
}
return mainThread.sandboxedEnv
}
func clearSandboxedEnv(thread *phpThread) {
if thread.sandboxedEnv == nil {
return
}
for key, val := range thread.sandboxedEnv {
valInMainThread, ok := mainThread.sandboxedEnv[key]
if !ok || val != valInMainThread {
C.free(unsafe.Pointer(val))
}
}
thread.sandboxedEnv = nil
}
// if an env var already exists, it needs to be freed
func removeEnvFromThread(thread *phpThread, key string) {
valueInThread, existsInThread := thread.sandboxedEnv[key]
if !existsInThread {
return
}
valueInMainThread, ok := mainThread.sandboxedEnv[key]
if !ok || valueInThread != valueInMainThread {
C.free(unsafe.Pointer(valueInThread))
}
delete(thread.sandboxedEnv, key)
}
// copy the main thread env to the thread specific env
func cloneSandboxedEnv(thread *phpThread) {
if thread.sandboxedEnv != nil {
return
}
thread.sandboxedEnv = make(map[string]*C.zend_string, len(mainThread.sandboxedEnv))
for key, value := range mainThread.sandboxedEnv {
thread.sandboxedEnv[key] = value
zkey := C.frankenphp_init_persistent_string(toUnsafeChar(key), C.size_t(len(key)))
zStr := C.frankenphp_init_persistent_string(toUnsafeChar(val), C.size_t(len(val)))
C.__hash_update_string__(mainThreadEnv, zkey, zStr)
}
}
//export go_putenv
func go_putenv(threadIndex C.uintptr_t, str *C.char, length C.int) C.bool {
thread := phpThreads[threadIndex]
envString := C.GoStringN(str, length)
cloneSandboxedEnv(thread)
func go_putenv(name *C.char, nameLen C.int, val *C.char, valLen C.int) C.bool {
goName := C.GoStringN(name, nameLen)
// Check if '=' is present in the string
if key, val, found := strings.Cut(envString, "="); found {
removeEnvFromThread(thread, key)
thread.sandboxedEnv[key] = C.frankenphp_init_persistent_string(toUnsafeChar(val), C.size_t(len(val)))
return os.Setenv(key, val) == nil
if val == nil {
// If no "=" is present, unset the environment variable
return C.bool(os.Unsetenv(goName) == nil)
}
// No '=', unset the environment variable
removeEnvFromThread(thread, envString)
return os.Unsetenv(envString) == nil
}
//export go_getfullenv
func go_getfullenv(threadIndex C.uintptr_t, trackVarsArray *C.zval) {
thread := phpThreads[threadIndex]
env := getSandboxedEnv(thread)
for key, val := range env {
C.add_assoc_str_ex(trackVarsArray, toUnsafeChar(key), C.size_t(len(key)), val)
}
}
//export go_getenv
func go_getenv(threadIndex C.uintptr_t, name *C.char) (C.bool, *C.zend_string) {
thread := phpThreads[threadIndex]
// Get the environment variable value
envValue, exists := getSandboxedEnv(thread)[C.GoString(name)]
if !exists {
// Environment variable does not exist
return false, nil // Return 0 to indicate failure
}
return true, envValue // Return 1 to indicate success
goVal := C.GoStringN(val, valLen)
return C.bool(os.Setenv(goName, goVal) == nil)
}

View File

@@ -80,10 +80,11 @@ frankenphp_config frankenphp_get_config() {
bool should_filter_var = 0;
bool original_user_abort_setting = 0;
HashTable *main_thread_env = NULL;
__thread uintptr_t thread_index;
__thread bool is_worker_thread = false;
__thread zval *os_environment = NULL;
__thread HashTable *sandboxed_env = NULL;
void frankenphp_update_local_thread_context(bool is_worker) {
is_worker_thread = is_worker;
@@ -286,7 +287,7 @@ bool frankenphp_shutdown_dummy_request(void) {
}
void get_full_env(zval *track_vars_array) {
go_getfullenv(thread_index, track_vars_array);
zend_hash_copy(Z_ARR_P(track_vars_array), main_thread_env, NULL);
}
/* Adapted from php_request_startup() */
@@ -383,39 +384,72 @@ PHP_FUNCTION(frankenphp_putenv) {
RETURN_FALSE;
}
if (go_putenv(thread_index, setting, (int)setting_len)) {
RETURN_TRUE;
} else {
RETURN_FALSE;
if (setting_len == 0 || setting[0] == '=') {
zend_argument_value_error(1, "must have a valid syntax");
RETURN_THROWS();
}
if (sandboxed_env == NULL) {
sandboxed_env = zend_array_dup(main_thread_env);
}
/* cut at null byte to stay consistent with regular putenv */
char *null_pos = memchr(setting, '\0', setting_len);
if (null_pos != NULL) {
setting_len = null_pos - setting;
}
/* cut the string at the first '=' */
char *eq_pos = memchr(setting, '=', setting_len);
bool success = true;
/* no '=' found, delete the variable */
if (eq_pos == NULL) {
success = go_putenv(setting, (int)setting_len, NULL, 0);
if (success) {
zend_hash_str_del(sandboxed_env, setting, setting_len);
}
RETURN_BOOL(success);
}
size_t name_len = eq_pos - setting;
size_t value_len =
(setting_len > name_len + 1) ? (setting_len - name_len - 1) : 0;
success = go_putenv(setting, (int)name_len, eq_pos + 1, (int)value_len);
if (success) {
zval val = {0};
ZVAL_STRINGL(&val, eq_pos + 1, value_len);
zend_hash_str_update(sandboxed_env, setting, name_len, &val);
}
RETURN_BOOL(success);
} /* }}} */
/* {{{ Call go's getenv to prevent race conditions */
/* {{{ Get the env from the sandboxed environment */
PHP_FUNCTION(frankenphp_getenv) {
char *name = NULL;
size_t name_len = 0;
zend_string *name = NULL;
bool local_only = 0;
ZEND_PARSE_PARAMETERS_START(0, 2)
Z_PARAM_OPTIONAL
Z_PARAM_STRING_OR_NULL(name, name_len)
Z_PARAM_STR_OR_NULL(name)
Z_PARAM_BOOL(local_only)
ZEND_PARSE_PARAMETERS_END();
if (!name) {
array_init(return_value);
get_full_env(return_value);
HashTable *ht = sandboxed_env ? sandboxed_env : main_thread_env;
if (!name) {
RETURN_ARR(zend_array_dup(ht));
return;
}
struct go_getenv_return result = go_getenv(thread_index, name);
if (result.r0) {
// Return the single environment variable as a string
RETVAL_STR(result.r1);
zval *env_val = zend_hash_find(ht, name);
if (env_val && Z_TYPE_P(env_val) == IS_STRING) {
zend_string *str = Z_STR_P(env_val);
zend_string_addref(str);
RETVAL_STR(str);
} else {
// Environment variable does not exist
RETVAL_FALSE;
}
} /* }}} */
@@ -688,11 +722,6 @@ static zend_module_entry frankenphp_module = {
TOSTRING(FRANKENPHP_VERSION),
STANDARD_MODULE_PROPERTIES};
static void frankenphp_request_shutdown() {
php_request_shutdown((void *)0);
frankenphp_free_request_context();
}
static int frankenphp_startup(sapi_module_struct *sapi_module) {
php_import_environment_variables = get_full_env;
@@ -915,32 +944,11 @@ static inline void register_server_variable_filtered(const char *key,
static void frankenphp_register_variables(zval *track_vars_array) {
/* https://www.php.net/manual/en/reserved.variables.server.php */
/* In CGI mode, we consider the environment to be a part of the server
* variables.
/* In CGI mode, the environment is part of the $_SERVER variables.
* $_SERVER and $_ENV should only contain values from the original
* environment, not values added though putenv
*/
/* in non-worker mode we import the os environment regularly */
if (!is_worker_thread) {
get_full_env(track_vars_array);
// php_import_environment_variables(track_vars_array);
go_register_variables(thread_index, track_vars_array);
return;
}
/* In worker mode we cache the os environment */
if (os_environment == NULL) {
os_environment = malloc(sizeof(zval));
if (os_environment == NULL) {
php_error(E_ERROR, "Failed to allocate memory for os_environment");
return;
}
array_init(os_environment);
get_full_env(os_environment);
// php_import_environment_variables(os_environment);
}
zend_hash_copy(Z_ARR_P(track_vars_array), Z_ARR_P(os_environment),
(copy_ctor_func_t)zval_add_ref);
zend_hash_copy(Z_ARR_P(track_vars_array), main_thread_env, NULL);
go_register_variables(thread_index, track_vars_array);
}
@@ -950,10 +958,12 @@ static void frankenphp_log_message(const char *message, int syslog_type_int) {
}
static char *frankenphp_getenv(const char *name, size_t name_len) {
struct go_getenv_return result = go_getenv(thread_index, (char *)name);
HashTable *ht = sandboxed_env ? sandboxed_env : main_thread_env;
if (result.r0) {
return result.r1->val;
zval *env_val = zend_hash_str_find(ht, name, name_len);
if (env_val && Z_TYPE_P(env_val) == IS_STRING) {
zend_string *str = Z_STR_P(env_val);
return ZSTR_VAL(str);
}
return NULL;
@@ -1092,6 +1102,13 @@ static void *php_main(void *arg) {
should_filter_var = default_filter != NULL;
original_user_abort_setting = PG(ignore_user_abort);
/* take a snapshot of the environment for sandboxing */
if (main_thread_env == NULL) {
main_thread_env = pemalloc(sizeof(HashTable), 1);
zend_hash_init(main_thread_env, 8, NULL, NULL, 1);
go_init_os_env(main_thread_env);
}
go_frankenphp_main_thread_is_ready();
/* channel closed, shutdown gracefully */
@@ -1138,7 +1155,8 @@ static int frankenphp_request_startup() {
return SUCCESS;
}
frankenphp_request_shutdown();
php_request_shutdown((void *)0);
frankenphp_free_request_context();
return FAILURE;
}
@@ -1164,16 +1182,16 @@ int frankenphp_execute_script(char *file_name) {
zend_catch { status = EG(exit_status); }
zend_end_try();
// free the cached os environment before shutting down the script
if (os_environment != NULL) {
zval_ptr_dtor(os_environment);
free(os_environment);
os_environment = NULL;
}
zend_destroy_file_handle(&file_handle);
frankenphp_request_shutdown();
/* Reset the sandboxed environment */
if (sandboxed_env != NULL) {
zend_hash_release(sandboxed_env);
sandboxed_env = NULL;
}
php_request_shutdown((void *)0);
frankenphp_free_request_context();
return status;
}

View File

@@ -140,6 +140,12 @@ func TestMain(m *testing.M) {
slog.SetDefault(slog.New(slog.DiscardHandler))
}
// setup custom environment var for TestWorkerHasOSEnvironmentVariableInSERVER
if os.Setenv("CUSTOM_OS_ENV_VARIABLE", "custom_env_variable_value") != nil {
fmt.Println("Failed to set environment variable for tests")
os.Exit(1)
}
os.Exit(m.Run())
}
@@ -695,11 +701,11 @@ func TestFailingWorker(t *testing.T) {
assert.Error(t, err, "should return an immediate error if workers fail on startup")
}
func TestEnv(t *testing.T) {
testEnv(t, &testOptions{nbParallelRequests: 1})
func TestEnv_module(t *testing.T) {
testEnv(t, &testOptions{nbParallelRequests: 1, phpIni: map[string]string{"variables_order": "EGPCS"}})
}
func TestEnvWorker(t *testing.T) {
testEnv(t, &testOptions{nbParallelRequests: 1, workerScript: "env/test-env.php"})
func TestEnv_worker(t *testing.T) {
testEnv(t, &testOptions{nbParallelRequests: 1, workerScript: "env/test-env.php", phpIni: map[string]string{"variables_order": "EGPCS"}})
}
// testEnv cannot be run in parallel due to https://github.com/golang/go/issues/63567
@@ -714,7 +720,7 @@ func testEnv(t *testing.T, opts *testOptions) {
stdoutStderr, err := cmd.CombinedOutput()
if err != nil {
// php is not installed or other issue, use the hardcoded output below:
stdoutStderr = []byte("Set MY_VAR successfully.\nMY_VAR = HelloWorld\nUnset MY_VAR successfully.\nMY_VAR is unset.\nMY_VAR set to empty successfully.\nMY_VAR = \nUnset NON_EXISTING_VAR successfully.\n")
stdoutStderr = []byte("Set MY_VAR successfully.\nMY_VAR = HelloWorld\nMY_VAR not found in $_ENV.\nMY_VAR not found in $_SERVER.\nUnset MY_VAR successfully.\nMY_VAR is unset.\nMY_VAR set to empty successfully.\nMY_VAR = \nUnset NON_EXISTING_VAR successfully.\nInvalid value was not inserted.\n")
}
assert.Equal(t, string(stdoutStderr), body)

View File

@@ -27,7 +27,6 @@ type phpMainThread struct {
phpIni map[string]string
commonHeaders map[string]*C.zend_string
knownServerKeys map[string]*C.zend_string
sandboxedEnv map[string]*C.zend_string
}
var (
@@ -40,12 +39,11 @@ var (
// and reserves a fixed number of possible PHP threads
func initPHPThreads(numThreads int, numMaxThreads int, phpIni map[string]string) (*phpMainThread, error) {
mainThread = &phpMainThread{
state: state.NewThreadState(),
done: make(chan struct{}),
numThreads: numThreads,
maxThreads: numMaxThreads,
phpIni: phpIni,
sandboxedEnv: initializeEnv(),
state: state.NewThreadState(),
done: make(chan struct{}),
numThreads: numThreads,
maxThreads: numMaxThreads,
phpIni: phpIni,
}
// initialize the first thread

View File

@@ -16,13 +16,12 @@ import (
// identified by the index in the phpThreads slice
type phpThread struct {
runtime.Pinner
threadIndex int
requestChan chan contextHolder
drainChan chan struct{}
handlerMu sync.RWMutex
handler threadHandler
state *state.ThreadState
sandboxedEnv map[string]*C.zend_string
threadIndex int
requestChan chan contextHolder
drainChan chan struct{}
handlerMu sync.RWMutex
handler threadHandler
state *state.ThreadState
}
// threadHandler defines how the callbacks from the C thread should be handled

View File

@@ -1,28 +1,27 @@
<?php
require_once __DIR__.'/../_executor.php';
require_once __DIR__ . '/../_executor.php';
return function() {
return function () {
$var = 'MY_VAR_' . ($_GET['var'] ?? '');
// Setting an environment variable
$result = putenv("$var=HelloWorld");
if ($result) {
echo "Set MY_VAR successfully.\n";
echo "MY_VAR = " . getenv($var) . "\n";
} else {
echo "Failed to set MY_VAR.\n";
}
echo $result ? "Set MY_VAR successfully.\nMY_VAR = " . getenv($var) . "\n" : "Failed to set MY_VAR.\n";
// putenv should not affect $_ENV
$result = $_ENV[$var] ?? null;
echo $result === null ? "MY_VAR not found in \$_ENV.\n" : "MY_VAR is in \$_ENV (not expected)\n";
// putenv should not affect $_SERVER
$result = $_SERVER[$var] ?? null;
echo $result === null ? "MY_VAR not found in \$_SERVER.\n" : "MY_VAR is in \$_SERVER (not expected)\n";
// Unsetting the environment variable
$result = putenv($var);
if ($result) {
echo "Unset MY_VAR successfully.\n";
$value = getenv($var);
if ($value === false) {
echo "MY_VAR is unset.\n";
} else {
echo "MY_VAR = " . $value . "\n";
}
echo $value === false ? "MY_VAR is unset.\n" : "MY_VAR = $value\n";
} else {
echo "Failed to unset MY_VAR.\n";
}
@@ -31,21 +30,23 @@ return function() {
if ($result) {
echo "MY_VAR set to empty successfully.\n";
$value = getenv($var);
if ($value === false) {
echo "MY_VAR is unset.\n";
} else {
echo "MY_VAR = " . $value . "\n";
}
echo $value === false ? "MY_VAR is unset.\n" : "MY_VAR = $value\n";
} else {
echo "Failed to set MY_VAR.\n";
}
// Attempt to unset a non-existing variable
$result = putenv('NON_EXISTING_VAR' . ($_GET['var'] ?? ''));
if ($result) {
echo "Unset NON_EXISTING_VAR successfully.\n";
echo $result ? "Unset NON_EXISTING_VAR successfully.\n" : "Failed to unset NON_EXISTING_VAR.\n";
// Inserting an invalid variable should fail (null byte in key)
$result = putenv("INVALID\x0_VAR=value");
if (getenv("INVALID\x0_VAR")) {
echo "Invalid value was inserted (unexpected).\n";
} else if ($result) {
echo "Invalid value was not inserted.\n";
} else {
echo "Failed to unset NON_EXISTING_VAR.\n";
echo "Invalid value was not inserted, but regular PHP should still return 'true' here.\n";
}
getenv();

View File

@@ -76,9 +76,6 @@ func (handler *regularThread) name() string {
}
func (handler *regularThread) waitForRequest() string {
// clear any previously sandboxed env
clearSandboxedEnv(handler.thread)
handler.state.MarkAsWaiting(true)
var ch contextHolder

View File

@@ -116,7 +116,6 @@ func setupWorkerScript(handler *workerThread, worker *worker) {
handler.dummyFrankenPHPContext = fc
handler.dummyContext = ctx
handler.isBootingScript = true
clearSandboxedEnv(handler.thread)
if globalLogger.Enabled(ctx, slog.LevelDebug) {
globalLogger.LogAttrs(ctx, slog.LevelDebug, "starting", slog.String("worker", worker.name), slog.Int("thread", handler.thread.threadIndex))

View File

@@ -23,6 +23,12 @@ void __zend_hash_init__(HashTable *ht, uint32_t nSize, dtor_func_t pDestructor,
zend_hash_init(ht, nSize, NULL, pDestructor, persistent);
}
void __hash_update_string__(zend_array *ht, zend_string *k, zend_string *v) {
zval zv = {0};
ZVAL_STR(&zv, v);
zend_hash_update(ht, k, &zv);
}
void __zval_null__(zval *zv) { ZVAL_NULL(zv); }
void __zval_bool__(zval *zv, bool val) { ZVAL_BOOL(zv, val); }

View File

@@ -15,6 +15,7 @@ void *__emalloc__(size_t size);
void __efree__(void *ptr);
void __zend_hash_init__(HashTable *ht, uint32_t nSize, dtor_func_t pDestructor,
bool persistent);
void __hash_update_string__(zend_array *ht, zend_string *k, zend_string *v);
int __zend_is_callable__(zval *cb);
int __call_user_function__(zval *function_name, zval *retval,

View File

@@ -8,13 +8,10 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"os"
"strconv"
"strings"
"testing"
"github.com/stretchr/testify/require"
"github.com/dunglas/frankenphp"
"github.com/stretchr/testify/assert"
)
@@ -144,8 +141,6 @@ func ExampleServeHTTP_workers() {
}
func TestWorkerHasOSEnvironmentVariableInSERVER(t *testing.T) {
require.NoError(t, os.Setenv("CUSTOM_OS_ENV_VARIABLE", "custom_env_variable_value"))
runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) {
req := httptest.NewRequest("GET", "http://example.com/worker.php", nil)
w := httptest.NewRecorder()