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