You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2020/02/26 16:31:12 UTC

[Bug 64180] New: secretRequred=false is ignored if secret=

https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

            Bug ID: 64180
           Summary: secretRequred=false is ignored if secret=<anything>
           Product: Tomcat 9
           Version: 9.0.31
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Connectors
          Assignee: dev@tomcat.apache.org
          Reporter: tparker@cbnco.com
  Target Milestone: -----

I have defined my server.xml as follows to allow the ajp.secretRequired and
ajp.secret values as command line properties so I don't have to edit the
server.xml on each server.

    <!-- Define an AJP 1.3 Connector on port 8009 -->
    <Connector protocol="AJP/1.3"
        address="${ajp.address}"
        connectionTimeout="20000"
        acceptCount="100"
        maxThreads="1600"
        minSpareThreads="100"
        port="8009"
        redirectPort="8443"
        secretRequired="${ajp.secretRequired}"
        secret="${ajp.secret}"
    />

If ajp.secretRequired=true and ajp.secret is set to something it all works as
expected.  However, if ajp.secretRequired=false and ajp.secret="" (or is not
set at all) the AJP connector still requires the secret to be passed.  

Steps to reproduce:  

1.  Set secretRequired=false secret="" in the server.xml 
2.  Try to connect from a client that does not pass a secret

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

Remy Maucherat <re...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|REOPENED                    |RESOLVED

--- Comment #6 from Remy Maucherat <re...@apache.org> ---
This is mostly wontfix as I am certain the request of the reporter is that
secretRequired disables the need for the client to send the secret.

10.0.0-M2, 9.0.32, 8.5.52 and 7.0.101 will now allow setting a secret as "",
and it will be treated as it it had not been set (= null). No doc fix was
needed, secretRequired was properly documented already.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

--- Comment #4 from Tom <tp...@cbnco.com> ---
The proposed enhancement still does not address the ability to parametrize the
server.xml.  

Once the keyword secret= is there, it becomes mandatory even if it's "".  

With your proposal would my server.xml below work as expected if
secretRequired=false? (no secret required) or would tomcat fail to start
because secret="" is invalid?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

--- Comment #5 from mgrigorov <mg...@apache.org> ---
(In reply to Tom from comment #4)
> The proposed enhancement still does not address the ability to parametrize
> the server.xml.  
> 
> Once the keyword secret= is there, it becomes mandatory even if it's "".  
> 
> With your proposal would my server.xml below work as expected if
> secretRequired=false? (no secret required) or would tomcat fail to start
> because secret="" is invalid?


If secret="" is treated the same way as missing attribute, i.e. =null, then it
will work.

I haven't tried it myself but maye you can script it today with:

ajp.secretAttribute=secret=abcd1234

or

ajp.secretAttribute=           # no value, i.e. ""

and then


<Connector protocol="AJP/1.3"
        ...
        ${ajp.secretAttribute}
    />

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

--- Comment #10 from Tom <tp...@cbnco.com> ---
(In reply to Remy Maucherat from comment #6)
> This is mostly wontfix as I am certain the request of the reporter is that
> secretRequired disables the need for the client to send the secret.
> 
> 10.0.0-M2, 9.0.32, 8.5.52 and 7.0.101 will now allow setting a secret as "",
> and it will be treated as it it had not been set (= null). No doc fix was
> needed, secretRequired was properly documented already.

This should fix my usecase perfectly.  

Thank you.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

Remy Maucherat <re...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |INVALID

--- Comment #1 from Remy Maucherat <re...@apache.org> ---
If a secret is configured, it needs to be submitted by the client.
secretRequired only forces the user (by default) to configure a secret in
server.xml.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

Tom <tp...@cbnco.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tparker@cbnco.com

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

--- Comment #7 from Mark Thomas <ma...@apache.org> ---
I agree the wording was correct but I had some text already committed locally
to make it even more explicit so I push those changes it as they can't do any
harm and may help as it is clear that some users were reading the docs and
taking away a different meaning from the one intended.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement

--- Comment #3 from Mark Thomas <ma...@apache.org> ---
The current behaviour is as expected / intended.

I can see merit in changing the behaviour so empty string and null (not set)
are equivalent for secret. We could also clarify that secretRequired indicates
whether the secret attribute MUST be set, not whether the client must provide a
secret and that (independent of secretRequired) the client MUST provide the
correct secret if secret is non-null and non-zero length.

Moving this to a proposed enhancement request for the changes described in this
comment.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

--- Comment #9 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Remy Maucherat from comment #6)
> This is mostly wontfix as I am certain the request of the reporter is that
> secretRequired disables the need for the client to send the secret.
> 
> 10.0.0-M2, 9.0.32, 8.5.52 and 7.0.101 will now allow setting a secret as "",
> and it will be treated as it it had not been set (= null). No doc fix was
> needed, secretRequired was properly documented already.

Is this how mod_jk/mod_proxy_ajp are working as well?

For example, in mod_* is it possible to use a "secret" which is the
empty-string? Or do they also treat empty-secret as "no secret"?

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

--- Comment #8 from Remy Maucherat <re...@apache.org> ---
After reading it, I agree. It's always possible to document things better :)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

--- Comment #11 from mgrigorov <mg...@apache.org> ---
(In reply to Christopher Schultz from comment #9)
> (In reply to Remy Maucherat from comment #6)
> > This is mostly wontfix as I am certain the request of the reporter is that
> > secretRequired disables the need for the client to send the secret.
> > 
> > 10.0.0-M2, 9.0.32, 8.5.52 and 7.0.101 will now allow setting a secret as "",
> > and it will be treated as it it had not been set (= null). No doc fix was
> > needed, secretRequired was properly documented already.
> 
> Is this how mod_jk/mod_proxy_ajp are working as well?
> 
> For example, in mod_* is it possible to use a "secret" which is the
> empty-string? Or do they also treat empty-secret as "no secret"?

https://github.com/apache/httpd/commit/d8b6d798c177dfdb90cef1a29395afcc043f3c86#diff-8992fe85968a8915e13ad663eb47d62fR206

I.e. if the value is "" it will be set.
If *conn->worker->s->secret is NULL it will set "secret" to NULL, i.e. do
nothing. I haven't coded in C for many years but I have the feeling this 'if'
is not really needed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

zac spitzer <za...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zac.spitzer@gmail.com

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 64180] secretRequred=false is ignored if secret=

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=64180

Tom <tp...@cbnco.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |---

--- Comment #2 from Tom <tp...@cbnco.com> ---
Would this be a change that you would be willing to consider?  

The current behaviour is non-intuitive and makes working with parametrized
server.xml files impossible.  

As soon as secret is in there anywhere (even an invalid one, it accepts "")
then it's mandatory.   

As an admin and package maintainer I feel that the the secretRequired field
should trump any setting in secret based on the current description in the
documentation.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org