OpenSSH with Wilkinson patch on OS X 10.2
Steven Michaud
smichaud at pobox.com
Tue May 6 18:33:01 EDT 2003
> I don't see how Apple could have got the SessionCreate() calls working
> from my hacked sshd without fixing all the problems that I reported to
> this list on 11-11-02. But appearances can be deceiving. So if
> anyone on this list knows more about this than I do, please do chime
> in.
Apple recently released the source code for the Security framework in
OS X versions 10.2.4 and 10.2.5.
http://www.opensource.apple.com/darwinsource/index.html
So I'm now able to answer my own question: Apple didn't fix all the
problems I reported on 11-11-02, but they did fix enough to get my
OpenSSH patch's very simple use of SessionCreate() and SessionGetInfo()
to work correctly.
Basically, Apple put in code to reinitialize several structures every
time the process id changes (which will happen every time a new
process is fork-ed or exec-ed). They also included a workaround
(quite different from the one I originally suggested) for the problem
of thread-local-storage not being properly initialized in a process
that had been fork-ed (or exec-ed).
This took care of the problems I reported under "Don't Forget fork()
and exec()", "MIT/Apple Kerberos Framework Incompatible with
SessionCreate()" and even "Security Framework Error Reporting Broken"
(though Apple didn't use my fix for the error reporting). Apple still
doesn't "Let Clients Call SessionCreate() At Any Time". But
SessionCreate() will now work once in every newly fork-ed (or exec-ed)
process, as long as it's the very first "Session" call -- you still
must call SessionCreate() before you call SessionGetInfo().
I'll append a patch which shows the changes Apple made. To confirm
that these changes are the relevant ones, I patched Security-54 (the
version of the Security framework that I originally patched in
November of last year) with them and installed it -- OpenSSH's
behavior with regard to SessionCreate() and SessionGetInfo() became
identical to its behavior in OS X versions 10.2.4 and 10.2.5. (I am,
of course, speaking of my patched version of OpenSSH, as altered from
Simon Wilkinson's patched version.)
On Mon, 3 Mar 2003, Steven Michaud wrote:
> Last November (11-11-02) I sent a message to the krbdev list
> (http://mailman.mit.edu/pipermail/krbdev/2002/000907.html) describing
> some of the problems I'd had getting OpenSSH with Simon Wilkinson's
> patch to work on Mac OS X 10.2 with its built-in MIT Kerberos
> libraries.
>
> I attached a bunch of patches that worked around all these problems.
> But one of the patches was to Darwin/OS X's Security framework
> (Security-54). For various reasons it's difficult (if not impossible)
> to keep this patch up to date. And if you leave the Security
> framework's problems unfixed, your system is at least theoretically
> open to exploits by people with accounts on your system (because
> everyone who makes an ssh connection will have access to the root
> authorization session). So my workarounds weren't suitable for
> production systems.
>
> (In January (1-12-03) I posted updated versions of some of these
> patches, plus an update to Simon Wilkinson's patch --
> http://mailman.mit.edu/pipermail/krbdev/2003/001078.html)
>
> But recently I noticed that, with OS X 10.2.4, Apple _seems_ to have
> resolved all the problems I found in their Security framework. If my
> Security framework patches can be dropped, then I do believe that the
> rest of my workarounds _are_ suitable for productions systems.
>
> Congratulations, Apple! That was quick work.
>
> At least I _think_ it was ... because though the evidence does suggest
> that Apple has fixed the Security framework problems I reported, I
> haven't been able to get anyone to confirm this. If someone on this
> list (especially someone from Apple) can confirm that SessionCreate()
> now (as of 10.2.4) works in apps that use fork() or exec() and which
> link to the MIT Kerberos libraries, there's a pretty long list of
> people who'd be grateful.
>
> My evidence is pretty basic: After I'd upgraded to 10.2.4 (I used
> Software Update), I recompiled OpenSSH 3.5p1 (with Simon Wilkinson's
> and my updated patches) to link against the 10.2.4 Security framework.
> Then I noticed that my single call to SessionCreate() from sshd (in
> the connected user's context) was no longer failing.
>
> I don't believe that the Kerberos framework was updated in 10.2.4.
> But to be sure Apple hadn't "fixed" my problems by, say, stopping the
> Kerberos framework's initialization code from making calls to
> SessionGetInfo(), I restored my hacked Security framework and relinked
> my hacked sshd to it -- I could still see multiple calls to
> SessionGetInfo() before main() in sshd.
>
> I don't see how Apple could have got the SessionCreate() calls working
> from my hacked sshd without fixing all the problems that I reported to
> this list on 11-11-02. But appearances can be deceiving. So if
> anyone on this list knows more about this than I do, please to chime
> in.
>
> _______________________________________________
> krbdev mailing list krbdev at mit.edu
> https://mailman.mit.edu/mailman/listinfo/krbdev
>
>
Here's the patch with Apple's "fork" fixes (which first appeared in OS
X 10.2.4). This is a patch against Apple's "Security-54" distribution
of the Security framework source-code. It won't apply "as is" --
additional changes are needed to the project.pbxproj file in the
Security.pbproj directory, whose inclusion would just have made the
patch harder to read.
diff -c -r src.old/AppleCSPDL/SSDatabase.cpp src/AppleCSPDL/SSDatabase.cpp
*** src.old/AppleCSPDL/SSDatabase.cpp Tue May 6 15:13:57 2003
--- src/AppleCSPDL/SSDatabase.cpp Tue May 6 15:13:53 2003
***************
*** 119,124 ****
--- 119,131 ----
SSDatabaseImpl::dbHandle()
{
activate();
+ if (mForked()) {
+ // re-establish the dbHandle with the SecurityServer
+ CssmDataContainer dbb(allocator());
+ mDbBlobId->get(NULL, &dbb);
+ mSSDbHandle = mClientSession.decodeDb(mIdentifier,
+ AccessCredentials::overlay(accessCredentials()), dbb);
+ }
return mSSDbHandle;
}
diff -c -r src.old/AppleCSPDL/SSDatabase.h src/AppleCSPDL/SSDatabase.h
*** src.old/AppleCSPDL/SSDatabase.h Tue May 6 15:13:57 2003
--- src/AppleCSPDL/SSDatabase.h Tue May 6 15:13:53 2003
***************
*** 72,77 ****
--- 72,80 ----
kDefaultLockOnSleep = true
};
+ DLDbIdentifier mIdentifier;
+ UnixPlusPlus::ForkMonitor mForked;
+
SecurityServer::ClientSession &mClientSession;
SecurityServer::DbHandle mSSDbHandle;
CssmClient::DbUniqueRecord mDbBlobId;
diff -c -r src.old/SecurityServer/ssclient.cpp src/SecurityServer/ssclient.cpp
*** src.old/SecurityServer/ssclient.cpp Tue May 6 15:14:00 2003
--- src/SecurityServer/ssclient.cpp Tue May 6 15:13:55 2003
***************
*** 35,40 ****
--- 35,41 ----
//
// The process-global object
//
+ UnixPlusPlus::StaticForkMonitor ClientSession::mHasForked;
ModuleNexus<ClientSession::Global> ClientSession::mGlobal;
bool ClientSession::mSetupSession;
***************
*** 60,65 ****
--- 61,75 ----
//
void ClientSession::activate()
{
+ // Guard against fork-without-exec. If we are the child of a fork
+ // (that has not exec'ed), our apparent connection to SecurityServer
+ // is just a mirage, and we better reset it.
+ if (mHasForked()) {
+ debug("SSclnt", "process has forked (now pid=%d) - resetting connection object", getpid());
+ mGlobal.reset();
+ }
+
+ // now pick up the (new or existing) connection state
Global &global = mGlobal();
Thread &thread = global.thread();
if (!thread) {
diff -c -r src.old/SecurityServer/ssclient.h src/SecurityServer/ssclient.h
*** src.old/SecurityServer/ssclient.h Tue May 6 15:14:00 2003
--- src/SecurityServer/ssclient.h Tue May 6 15:13:56 2003
***************
*** 32,37 ****
--- 32,38 ----
#include <Security/cssmacl.h>
#include <Security/context.h>
#include <Security/globalizer.h>
+ #include <Security/unix++.h>
#include <Security/mach++.h>
#include <Security/cssmdb.h>
#include <Security/osxsigning.h>
***************
*** 285,290 ****
--- 286,293 ----
const AclOwnerPrototype &edit);
private:
+ static UnixPlusPlus::StaticForkMonitor mHasForked; // global fork indicator
+
struct Thread {
Thread() : registered(false) { }
operator bool() const { return registered; }
diff -c -r src.old/cdsa/cdsa_utilities/threading.cpp src/cdsa/cdsa_utilities/threading.cpp
*** src.old/cdsa/cdsa_utilities/threading.cpp Tue May 6 15:13:58 2003
--- src/cdsa/cdsa_utilities/threading.cpp Tue May 6 15:13:54 2003
***************
*** 46,52 ****
--- 46,55 ----
{
//@@@ if we wanted to dispose of pending task objects, we'd have
//@@@ to keep a set of them and delete them explicitly here
+ #if BUG_2998157
+ // @@@ bug 2998157 does not clear slots on delete or allocate. Leak them for now
pthread_key_delete(mKey);
+ #endif //BUG_2998157
}
#endif
diff -c -r src.old/cdsa/cdsa_utilities/unix++.h src/cdsa/cdsa_utilities/unix++.h
*** src.old/cdsa/cdsa_utilities/unix++.h Tue May 6 15:13:59 2003
--- src/cdsa/cdsa_utilities/unix++.h Tue May 6 15:13:54 2003
***************
*** 138,143 ****
--- 138,169 ----
~AutoFileDesc() { close(); }
};
+ //
+ // A ForkMonitor determines whether the current thread is a (fork) child of
+ // the thread that last checked it. Essentially, it checks for pid changes.
+ //
+ class StaticForkMonitor {
+ public:
+ bool operator () () const
+ {
+ if (mLastPid == 0) {
+ mLastPid = getpid();
+ return false;
+ } else if (getpid() != mLastPid) {
+ mLastPid = getpid();
+ return true;
+ }
+ return false;
+ }
+
+ protected:
+ mutable pid_t mLastPid;
+ };
+
+ class ForkMonitor : public StaticForkMonitor {
+ public:
+ ForkMonitor() { mLastPid = getpid(); }
+ };
} // end namespace UnixPlusPlus
} // end namespace Security
More information about the krbdev
mailing list