| SDP-67 | Defend against environment hijacking of P4ENVIRO. == Background == A core... benefit of the SDP is the controlled, easily understood environment, both shell environment and Perforce context settings. These configuration items are carefully controlled in the SDP's config files and environment loading mechanism in p4_vars. The new-ish (2015.1) behavior of 'p4 set' to persist values on Unix/Linux is detrimental in an SDP environment, which is designed for live production servers. That hijacking capability is perfect for what it was intended for (mainly for DVCS usage, so a 'p4 clone' can hijack an individual user's P4CONFIG setting). But I am not sure there's a need for that on the production server, and allowing it can impact reliability/robustness of core SDP operations. Folks can always set P4ENVIRO separately or unset it (defaulting to ~/.p4enviro) if they want for some non-SDP reason on the production server. Ditto for P4CONFIG. == Shell Environment Management == The p4d trigger and p4broker filter scripts inherit their shell environment from the corresponding p4d or p4broker server process. Thus, the benefit of the SDP controlled environment mechanism includes supplemental automation, such as triggers and broker filter scripts. While the shell environment can't change after the server processes start, the Perforce context settings (P4PORT, P4CLIENT, P4USER, etc.) can be changed at any time by adjusting the contents of the P4CONFIG and/or P4ENVIRO files. Historically, P4CONFIG (which has been around for a long while) hasn't been an issue with the SDP, perhaps because changing the context settings is well-understood, and/or perhaps because it requires modifying a file on disk, which seems like a "big deal" when you're working on a production server machine. Lately, I've been finding that P4ENVIRO is a bit more prone to abuse. Maybe that's because folks don't understand it or its impact well; they do a 'p4 set' command to persist values for context settings (e.g. P4CONFIG) while logged in as 'perforce' on the production server, thinking they're just affecting their own shell environment, and inadvetently busting triggers or broker scripts. An admin might do a 'p4 set' command for personal reasons in an SDP-managed environment, and unexpectedly sabotage checkpoint scripts (e.g. p4 set P4PORT=9999), since values persisted with 'p4 set' outrank those defined in the SDP's carefully controlled shell environment. P4ENVIRO outranks even P4CONFIG, and P4ENVIRO also has capability to hijack the P4CONFIG value itself. == The Goal == The goal with this proposed change is to expliclty disable the two mechanisms, P4CONFIG and P4ENVIRO, which can impact the Perforce context settings after the server process has started. The change: Simply point both P4CONFIG and P4ENVIRO to invalid paths under /dev/null. Now, one could argue that 'p4 set' is functioning as designed, and that disabling this default behavior (a feature of the product), is more unexpected than leaving it alone. I submit that in an SDP environment designed to live on production servers, this is OK, as the higher objective is stability for production environments. == Side Effects for P4CONFIG == AFAIK, the P4CONFIG file defined in the SDP is not used. It just defines a value that points to an empty file (e.g. /p4/1/.p4config), then we don't put anything into it. This change would make it impossible to put anything into it. == Side Effects for P4ENVIRO == This change would effectively disable the ability to use the 'p4 set' command to persist values in a Unix/Linux environment. The 'p4 set' command can still be used to query values. Attempts to persist values, e.g. 'p4 set P4PORT=9999', are greeted with this error: Perforce client error: Can't set registry on UNIX. open for write: /dev/null/.p4enviro: Not a directory == Scope == This job is for the Unix/Linx SDP only. The concepts applies as ready for Windows, but the mechanics are dramatically different, and considerations are different due to the reliance on the Windows registry on Windows. That entire discussion deserves a separate job. « | |
| Add Job | ||