directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Karasulu" <akaras...@apache.org>
Subject Re: Possible NPE when shutting down server??
Date Thu, 09 Aug 2007 23:33:51 GMT
Hi David,

On 8/9/07, David Jencks <david_jencks@yahoo.com> wrote:
>
>
> On Aug 9, 2007, at 3:05 PM, Alex Karasulu wrote:
>
> I would rather fix the NPE and indicate to the failure to be due to
> someone not being authorized to shutdown the server.
>
>
> Sure, but currently what is happening is
> FIRST we shut down the server
> SECOND we see if some credentials were supplied
> NEVER we check if the credentials were valid.
>

Well that's not good at all.  For now we should check the credentials.

Given this I think the best first step is eliminating (2) entirely since
> it's basically meaningless.  I also am not quite sure I understand why in-vm
> shutdown should require credentials at all.  But then I don't understand why
> the server is started and stopped through new InitialDirContext(props)
> either.
>

Long story short, we need to build in better security into the server to
sandbox stored procedures
so they can only do things as the user that owns it.  This was setup
originally to enforce that but
it fails miserably.

There are many things to do to set things straight but for the time being I
suggest we just patch
this security issue by checking the credentials then making sure an
informative error message
with the correct exception (a ConfigurationException) is thrown when these
required parameters
are not provided.

I'd like to elaborate further on this but there are some issues which still
remain weak in my mind
but I will figure them out and get back to this thread.

Regards,
Alex

thanks
> david jencks
>
>
> Alex
>
> On 8/9/07, David Jencks <david_jencks@yahoo.com> wrote:
> >
> > I don't seem to have noted in this thread that I created
> > DIRSERVER-1002 on this issue.  I'm finding it a bit annoying that I
> > have to provide these bogus security credentials to avoid an NPE.
> > Would anyone mind if I fixed this either by removing the
> > DefaultDirectoryServer.checkSecuritySettings method or by changing
> > AbstractContextFactory.getInitialContext around line 116 to
> >
> >          if ( cfg instanceof ShutdownConfiguration )
> >          {
> >              service.shutdown();
> >              return new DeadContext();
> >          }
> >
> > thanks
> > david jencks
> >
> > On Jul 21, 2007, at 1:57 PM, Emmanuel Lecharny wrote:
> >
> > > Damn, I now see your point...
> > >
> > > Ok, if we are not insane people, when setting up a LDAP server, we
> > > don't want anyone to shutdown it (assuming it is run with a dedicated
> > > user, or as root - yuk -).
> > >
> > > So if you can shutdown the server without providing some credentials
> > > should not be possible, and in any case, it should not produce a nasty
> >
> > > NPE...
> > >
> > > JIRA ?
> > >
> > > Thanks !
> > >
> > > On 7/21/07, David Jencks <david_jencks@yahoo.com> wrote:
> > >>
> > >> On Jul 21, 2007, at 11:42 AM, Emmanuel Lecharny wrote:
> > >>
> > >> > Hi David,
> > >> >
> > >> >
> > >> > strange problem... Have you had a look at the AbstractServerTest
> > >> class
> > >> > we are using for our integration tests (it's in server-unit
> > >> project)?
> > >> >
> > >> > http://svn.apache.org/viewvc/directory/apacheds/trunk/server-unit/
> > >> > src/main/java/org/apache/directory/server/unit/
> > >> > AbstractServerTest.java?revision=542692&view=markup
> > >> >
> > >> > We are extending tests from this class so that the setup() and
> > >> > teardown() methods are common.
> > >> >
> > >> > Let us know if you have the same pb with it.
> > >>
> > >> I haven't tried it but imagine it would work since it supplies
> > >> identity and security credentials to shut down the server, and the
> > >> npe I saw was caused by them not being there.  I think more and more
> > >> there's a bad design or bug here:  either the security credentials
> > >> should be checked BEFORE shutting down the server (and problems
> > >> reported with something other than an NPE) or you shouldn't need
> > >> security credentials from inside the same vm.
> > >>
> > >> Let me ask again.... checkSecuritySettings is a half-hearted attempt
> > >> to see if a vague resemblance to security credentials were supplied.
> > >> Why is this check even present in AbstractContextFactory?  It seems
> > >> to me that this class should either rely on something else taking
> > >> care of security (in which case the checkSecuritySettings method
> > >> should be removed) or it should actually check that the credentials
> > >> are valid and that they authorize the action being attempted.
> > >>
> > >> Again, the behavior I'm seeing is that when you try to shut down the
> > >> server and don't supply any credentials, the server is in fact shut
> > >> down, and then it throws an NPE.  I'm fairly sure that this is not
> > >> appropriate behavior, but I don't know whether the shutdown should
> > >> succeed with no objection or if an exception (probably not an NPE)
> > >> should be thrown and the server remain running.
> > >>
> > >> thanks
> > >> david jencks
> > >>
> > >> >
> > >> > On 7/19/07, David Jencks <david_jencks@yahoo.com> wrote:
> > >> >> I'm trying to resuscitate and update the geronimo-apacheds
> > >> >> integration and have run into some behavior I don't
> > >> understand.  Due
> > >> >> to lack of internet access when I started I'm using up to date
> > >> trunk
> > >> >> with my xbean-spring mods, but they don't affect this situation
> > >> >> compared to regular trunk.
> > >> >>
> > >> >> In a unit test I'm starting the server with
> > >> >>
> > >> >> new InitialDirectoryContext(env); which appears to be working
ok
> > >> >>
> > >> >> and stopping it with
> > >> >>
> > >> >> new InitialDirectoryContext(env) from a shutdown config which
is
> > >> >> getting an NPE which seems less than appropriate.
> > >> >>
> > >> >> The code gets into AbstractContextFactory.getInitialContext and
on
> > >> >> line 115 the server is shut down setting the
> > >> >> DefaultDirectoryService.startupConfiguration to null, as seems
> > >> >> appropriate.
> > >> >>
> > >> >> Then on line 146 of AbstractContextFactory we call
> > >> >> service.getJndiContext which first does this:
> > >> >>
> > >> >>          checkSecuritySettings( principal, credential,
> > >> >> authentication );
> > >> >>
> > >> >>          if ( !started )
> > >> >>          {
> > >> >>              return new DeadContext();
> > >> >>          }
> > >> >>
> > >> >> and checkSecuritySettings uses startupConfiguration on line
> > >> 437, thus
> > >> >> throwing the NPE.
> > >> >>
> > >> >> So, this doesn't seem quite right to me.
> > >> >>
> > >> >> 1. The server has already been shut down by the time we are
> > >> trying to
> > >> >> get the context, so either returning a DeadContext() from
> > >> >> AbstractContextFactory after shutting down the server or from
> > >> >> DefaultDirectoryService.getJndiContext before checking the
> > >> security
> > >> >> settings seems reasonable.  This would fix my immediate problem
> > >> >>
> > >> >> 2.  Why is DefaultDirectoryService checking security settings
at
> > >> >> all?  It's not checking authentication or authorization, just
> > >> doing
> > >> >> some minimal consistency checks, so why not rely on whatever is
> > >> >> checking those?  This seems to me like a big separation of
> > >> concerns
> > >> >> problem.
> > >> >>
> > >> >> 3. Have I missed something or is the usual way to start and
> > >> stop the
> > >> >> server through these new IniitialDirectoryContext() calls?  Is
> > >> there
> > >> >> a reason it isn't started more directly through spring?  I
> > >> don't see
> > >> >> how your'e going to fire up a new jvm through a new
> > >> >> InitialDirectoryContext() call, so starting a server is in vm
> > >> anyway,
> > >> >> and allowing remote stopping seems.... like something you might
> > >> not
> > >> >> want to make too easy.
> > >> >>
> > >> >> thanks
> > >> >> david jencks
> > >> >>
> > >> >
> > >> >
> > >> > --
> > >> > Regards,
> > >> > Cordialement,
> > >> > Emmanuel L├ęcharny
> > >> > www.iktek.com
> > >>
> > >>
> > >
> > >
> > > --
> > > Regards,
> > > Cordialement,
> > > Emmanuel L├ęcharny
> > > www.iktek.com
> >
> >
>
>

Mime
View raw message