Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/model/

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/model/

Jesse Glick
[hidden email] wrote:

>>  Fixed ISSUE #171: "Guest users can start builds via direct URLs"
>>  
>> Url: https://hudson.dev.java.net/source/browse/hudson/hudson/main/core/src/main/java/hudson/model/Project.java?r1=1.8&r2=1.9
>>      public void doBuild( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
>> +        if(!Hudson.adminCheck(req,rsp))
>> +            return;
>> +
>>          scheduleBuild();
>>          rsp.forwardToPreviousPage(req);
>>      }

Hold on here. #171 says that the problem is for

   two buttons in Build View: "Keep this build forever" and
   "Delete this build"

and the suggested fix is that

   the buttons should not be present for Guests

But this check looks to be for something else: the button to schedule a
build for a project. That is not currently prohibited for guests and I
think it should continue to be allowed.

At least, if this patch does what it looks like it will break our build
system, which relies on a sendmail-launched script responding to
appropriate CVS commit messages by launching a build using wget: the
script would need to be changed to make wget go through the login page
with the admin username and password, then retain a login cookie before
requesting a build URL. Possible but cumbersome.

Is that really what was intended? Or am I misreading something?

-J.

--
[hidden email]  x22801  netbeans.org  ant.apache.org
       http://google.com/search?q=e%5E%28pi*i%29%2B1

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/model/

Vladimir Sizikov
Hi Jesse,

On Wed, Nov 22, 2006 at 01:55:34PM -0500, Jesse Glick wrote:

> [hidden email] wrote:
> >> Fixed ISSUE #171: "Guest users can start builds via direct URLs"
> >>
> >>Url:
> >>https://hudson.dev.java.net/source/browse/hudson/hudson/main/core/src/main/java/hudson/model/Project.java?r1=1.8&r2=1.9
> >>     public void doBuild( StaplerRequest req, StaplerResponse rsp )
> >>     throws IOException, ServletException {
> >>+        if(!Hudson.adminCheck(req,rsp))
> >>+            return;
> >>+
> >>         scheduleBuild();
> >>         rsp.forwardToPreviousPage(req);
> >>     }
>
> Hold on here. #171 says that the problem is for
>
>   two buttons in Build View: "Keep this build forever" and
>   "Delete this build"
>
> and the suggested fix is that
>
>   the buttons should not be present for Guests

Sorry, a typo in ISSUE number (d'oh!).
It should be ISSUE #177, not #171.

There are two different bugs here.

> But this check looks to be for something else: the button to schedule a
> build for a project. That is not currently prohibited for guests and I
> think it should continue to be allowed.

Ah, I was particularily concerned about Netbeans and Glassfish
installations, and thought that maintainers of those publicly
accessible servers will not like that _anybody_ in the Internet can
start builds.

But you're saying that you do want this.

> At least, if this patch does what it looks like it will break our build
> system, which relies on a sendmail-launched script responding to
> appropriate CVS commit messages by launching a build using wget: the
> script would need to be changed to make wget go through the login page
> with the admin username and password, then retain a login cookie before
> requesting a build URL. Possible but cumbersome.

Kohsuke also mentioned in the comments that ability to bulid was left
unprotected intentionally.

> Is that really what was intended? Or am I misreading something?

No, you're not missing anything.

The change was to protect "build" action.

I can back it out, sure, but I do feel that we have some kind of
problem here. Guests should not be permitted to start any actions on
servers, nor to look in configurations, etc.

Thanks,
  --Vladimir

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/model/

Jesse Glick
In reply to this post by Jesse Glick
Jesse Glick wrote:
> Hold on here. #171 says that the problem is for

I see. The change was really for #177, not for #171 as claimed. I don't
really like it but I guess I'll have to adapt. If I figure out what to
do with wget I'll patch build.html (the current instructions will no
longer work).

-J.

--
[hidden email]  x22801  netbeans.org  ant.apache.org
       http://google.com/search?q=e%5E%28pi*i%29%2B1

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/model/

Kohsuke Kawaguchi-2
In reply to this post by Jesse Glick
Jesse Glick wrote:

> Hold on here. #171 says that the problem is for
>
>    two buttons in Build View: "Keep this build forever" and
>    "Delete this build"
>
> and the suggested fix is that
>
>    the buttons should not be present for Guests
>
> But this check looks to be for something else: the button to schedule a
> build for a project. That is not currently prohibited for guests and I
> think it should continue to be allowed.
>
> At least, if this patch does what it looks like it will break our build
> system, which relies on a sendmail-launched script responding to
> appropriate CVS commit messages by launching a build using wget: the
> script would need to be changed to make wget go through the login page
> with the admin username and password, then retain a login cookie before
> requesting a build URL. Possible but cumbersome.
This is what I was afraid of. Existing deployments that rely on this
behavior. This is a bit tricky case, however, because this is a security
hole, in a way.

One solution is to define an option (or by default) to allow access from
localhost to have unlimited sys admin access. Does it make your case work?

Also, I believe for automation sake, I think you'd only need to send
HTTP Basic authentication headers. While you'd have to hard-code
password in clear text in wget commandline (bad!), I think at least it's
simpler than going through the login page and get a cookie.


--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]

smime.p7s (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/model/

Jesse Glick
In reply to this post by Vladimir Sizikov
Vladimir Sizikov wrote:
> Ah, I was particularily concerned about Netbeans and Glassfish
> installations, and thought that maintainers of those publicly
> accessible servers will not like that _anybody_ in the Internet can
> start builds.
>
> But you're saying that you do want this.

Well, we don't generally want anyone manually starting builds, even if
they are "trusted", since builds are triggered automatically. But it's
not a serious problem that I know of. You could probably think of
scenarios where it would be a security hole.

-J.

--
[hidden email]  x22801  netbeans.org  ant.apache.org
       http://google.com/search?q=e%5E%28pi*i%29%2B1

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/model/

Jesse Glick
In reply to this post by Kohsuke Kawaguchi-2
Kohsuke Kawaguchi wrote:
> One solution is to define an option (or by default) to allow access from
> localhost to have unlimited sys admin access. Does it make your case work?

It would, although that could be a (potentially worse) security hole in
its own right - anyone with a login to a server could run scripts as the
Hudson user ID.

> Also, I believe for automation sake, I think you'd only need to send
> HTTP Basic authentication headers.

Really? I just assumed Hudson required a login cookie / session ID,
since it asks for one, rather than prompting the browser for HTTP basic
auth credentials. Does it accept either? Is this just handled implicitly
by the servlet container? (I am using Tomcat for now.)

I'll try it soon.

> While you'd have to hard-code password in clear text in wget
> commandline (bad!)

Well, Tomcat config seems to keep passwords in clear text anyway.

-J.

--
[hidden email]  x22801  netbeans.org  ant.apache.org
       http://google.com/search?q=e%5E%28pi*i%29%2B1

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/model/

Kohsuke Kawaguchi-2
Jesse Glick wrote:
> Kohsuke Kawaguchi wrote:
>> One solution is to define an option (or by default) to allow access from
>> localhost to have unlimited sys admin access. Does it make your case work?
>
> It would, although that could be a (potentially worse) security hole in
> its own right - anyone with a login to a server could run scripts as the
> Hudson user ID.

OK, so doing it by default might be a bad idea, but in most of the
internet-facing machines that run Hudson, you can trust local users.


>> Also, I believe for automation sake, I think you'd only need to send
>> HTTP Basic authentication headers.
>
> Really? I just assumed Hudson required a login cookie / session ID,
> since it asks for one, rather than prompting the browser for HTTP basic
> auth credentials. Does it accept either? Is this just handled implicitly
> by the servlet container? (I am using Tomcat for now.)

I hope the container works that way, but I don't know for sure.

>> While you'd have to hard-code password in clear text in wget
>> commandline (bad!)
>
> Well, Tomcat config seems to keep passwords in clear text anyway.

You can use different realm implementation. For example, LDAP-based
realm won't store password in clear (although it sends one in clear)

--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]

smime.p7s (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/model/

Vladimir Sizikov
In reply to this post by Jesse Glick
Hi Jesse, folks,

I've been thinking about this issue since the bugfix clearly made some
things harder to do in Hudson.

And it seems to me that requiring users to know full admin password
(and this password is not only for Hudson admins, but for _entire_
server, such as Tomcat) is rather dangerous, not to say, quite
inconvenient for for script writers.

We need more fine-tuned approach to security. And it seems that there
are two major use cases here:

1. Hudson administration via GUI. Users should know admin password.

2. Remotely triggering the build via scripts, hooks, etc. Obviously,
   those who want this functionality, don't need to know full admin
   password.

As for the solution that satisfies both cases,
how about, for example, Project-level configuration option, like
"An authorization token to remotely trigger the build", and a string
value.

Then, projects who would like to have remotely-triggered builds, would
enable that option, and provide a string (sort of like password).

Next, any access to URL like
hudson/job/name/build?token=AUTH_TOKEN_HERE
would trigger the build.

By using this or similar approach we'll allow to start builds remotely
without much problems (no auth via POST, cookies, sessions and all
that craziness) and at the same time we'll not compromise full Hudson
and server security. And those projects that don't want this
functionality, will have it disabled in config altogether.

What do you think?

Thanks,
  --Vladimir

On Wed, Nov 22, 2006 at 02:51:52PM -0500, Jesse Glick wrote:

> Kohsuke Kawaguchi wrote:
> >One solution is to define an option (or by default) to allow access from
> >localhost to have unlimited sys admin access. Does it make your case work?
>
> It would, although that could be a (potentially worse) security hole in
> its own right - anyone with a login to a server could run scripts as the
> Hudson user ID.
>
> >Also, I believe for automation sake, I think you'd only need to send
> >HTTP Basic authentication headers.
>
> Really? I just assumed Hudson required a login cookie / session ID,
> since it asks for one, rather than prompting the browser for HTTP basic
> auth credentials. Does it accept either? Is this just handled implicitly
> by the servlet container? (I am using Tomcat for now.)
>
> I'll try it soon.
>
> >While you'd have to hard-code password in clear text in wget
> >commandline (bad!)
>
> Well, Tomcat config seems to keep passwords in clear text anyway.
>
> -J.
>
> --
> [hidden email]  x22801  netbeans.org  ant.apache.org
>       http://google.com/search?q=e%5E%28pi*i%29%2B1
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: CVS update: /hudson/hudson/main/core/src/main/java/hudson/model/

Vladimir Sizikov
Hi,

FYI for those who don't read CVS/Issues aliases,
this issue has been logged as ISSUE #178 and has been fixed and
committed to the repository.

See the following URL for more details on how to trigger builds
remotely when Hudson security is enabled:

https://hudson.dev.java.net/build.html

Thanks,
  --Vladimir

On Thu, Nov 23, 2006 at 10:45:07AM -0800, Vladimir Sizikov wrote:

> Hi Jesse, folks,
>
> I've been thinking about this issue since the bugfix clearly made some
> things harder to do in Hudson.
>
> And it seems to me that requiring users to know full admin password
> (and this password is not only for Hudson admins, but for _entire_
> server, such as Tomcat) is rather dangerous, not to say, quite
> inconvenient for for script writers.
>
> We need more fine-tuned approach to security. And it seems that there
> are two major use cases here:
>
> 1. Hudson administration via GUI. Users should know admin password.
>
> 2. Remotely triggering the build via scripts, hooks, etc. Obviously,
>    those who want this functionality, don't need to know full admin
>    password.
>
> As for the solution that satisfies both cases,
> how about, for example, Project-level configuration option, like
> "An authorization token to remotely trigger the build", and a string
> value.
>
> Then, projects who would like to have remotely-triggered builds, would
> enable that option, and provide a string (sort of like password).
>
> Next, any access to URL like
> hudson/job/name/build?token=AUTH_TOKEN_HERE
> would trigger the build.
>
> By using this or similar approach we'll allow to start builds remotely
> without much problems (no auth via POST, cookies, sessions and all
> that craziness) and at the same time we'll not compromise full Hudson
> and server security. And those projects that don't want this
> functionality, will have it disabled in config altogether.
>
> What do you think?
>
> Thanks,
>   --Vladimir
>
> On Wed, Nov 22, 2006 at 02:51:52PM -0500, Jesse Glick wrote:
> > Kohsuke Kawaguchi wrote:
> > >One solution is to define an option (or by default) to allow access from
> > >localhost to have unlimited sys admin access. Does it make your case work?
> >
> > It would, although that could be a (potentially worse) security hole in
> > its own right - anyone with a login to a server could run scripts as the
> > Hudson user ID.
> >
> > >Also, I believe for automation sake, I think you'd only need to send
> > >HTTP Basic authentication headers.
> >
> > Really? I just assumed Hudson required a login cookie / session ID,
> > since it asks for one, rather than prompting the browser for HTTP basic
> > auth credentials. Does it accept either? Is this just handled implicitly
> > by the servlet container? (I am using Tomcat for now.)
> >
> > I'll try it soon.
> >
> > >While you'd have to hard-code password in clear text in wget
> > >commandline (bad!)
> >
> > Well, Tomcat config seems to keep passwords in clear text anyway.
> >
> > -J.
> >
> > --
> > [hidden email]  x22801  netbeans.org  ant.apache.org
> >       http://google.com/search?q=e%5E%28pi*i%29%2B1
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]