You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Konstantin Kolinko <kn...@gmail.com> on 2014/10/17 15:13:02 UTC
Re: digest.bat (RealmBase.main()) is broken in current Tomcat 8
trunk. Tomcat 8.0.14 is OK.
2014-09-30 19:22 GMT+04:00 Konstantin Kolinko <kn...@gmail.com>:
> 2014-09-29 14:43 GMT+04:00 Mark Thomas <ma...@apache.org>:
>> On 27/09/2014 15:52, Konstantin Kolinko wrote:
> (....)
>
>>> 4) The current javadoc for RealmBase.main() says that algorithm (-a)
>>> is not required and "If not specified a default of SHA-512 will be
>>> used."
>>>
>>> I wonder whether that is justified.
>>
>> That is what is currently implemented. Happy to discuss changes but
>> SHA-512 doesn't seem unreasonable to me.
>
>
> I think there is a contradiction between -a <algorithm> and -h
> <credential handler implementation class> keys:
> 1) If -h is used I think it shall default to whatever default
> algorithm the credential handler implements.
> 2) Custom credential handler implementations may lack setAlgorithm() method.
>
> I think that one of (-a, -h) is required, with no default for either.
> The old code had no default for algorithm.
>
>> String encoding = "UTF-8";
>
> I think it shall use system encoding, because the value is passed on
> the command line and is not read from file etc.
>
> The old code used system encoding by default. The system encoding is
> what the system uses, so it is reasonable.
>
> Note the following text (I am linking to Tomcat 7 docs),
> -> Realms and AAA -> Common Features -> Digested passwords
>
> http://tomcat.apache.org/tomcat-7.0-doc/realm-howto.html#Digested_Passwords
>
> [quote]
> Non-ASCII usernames and/or passwords are supported using
>
> CATALINA_HOME/bin/digest.[bat|sh] -a {algorithm} -e {encoding} {input}
>
> but care is required to ensure that the non-ASCII input is correctly
> passed to the digester. The digester returns {input}:{digest}. If the
> input appears corrupted in the return, the digest will be invalid.
> [/quote]
>
> BTW, That chapter in realm-howto in Tomcat 8 needs an update for the
> new features of digest.sh / RealmBase.main().
>
I think that this have to be fixed before tagging next Tomcat 8 release.
1. Remove default value for algorithm. Ask the caller to provide
either "-a" or "-h" option explicitly.
Motivation:
- Revert to previous behaviour.
- I see contradiction between -a and -h, as I wrote above.
2. Use system default encoding instead of UTF-8 by default.
Motivation:
- Revert to previous behaviour. It makes sense to expect system
encoding when you are calling something from the command line, as that
is the encoding that command line uses.
3. Update realm-howto.html#Digested_Passwords
It does not document the new -h option.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: digest.bat (RealmBase.main()) is broken in current Tomcat 8 trunk.
Tomcat 8.0.14 is OK.
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Konstantin,
On 10/17/14 9:13 AM, Konstantin Kolinko wrote:
> 2014-09-30 19:22 GMT+04:00 Konstantin Kolinko <kn...@gmail.com>:
>> 2014-09-29 14:43 GMT+04:00 Mark Thomas <ma...@apache.org>:
>>> On 27/09/2014 15:52, Konstantin Kolinko wrote:
>> (....)
>>
>>>> 4) The current javadoc for RealmBase.main() says that algorithm (-a)
>>>> is not required and "If not specified a default of SHA-512 will be
>>>> used."
>>>>
>>>> I wonder whether that is justified.
>>>
>>> That is what is currently implemented. Happy to discuss changes but
>>> SHA-512 doesn't seem unreasonable to me.
>>
>>
>> I think there is a contradiction between -a <algorithm> and -h
>> <credential handler implementation class> keys:
>> 1) If -h is used I think it shall default to whatever default
>> algorithm the credential handler implements.
>> 2) Custom credential handler implementations may lack setAlgorithm() method.
>>
>> I think that one of (-a, -h) is required, with no default for either.
>> The old code had no default for algorithm.
>>
>>> String encoding = "UTF-8";
>>
>> I think it shall use system encoding, because the value is passed on
>> the command line and is not read from file etc.
>>
>> The old code used system encoding by default. The system encoding is
>> what the system uses, so it is reasonable.
>>
>> Note the following text (I am linking to Tomcat 7 docs),
>> -> Realms and AAA -> Common Features -> Digested passwords
>>
>> http://tomcat.apache.org/tomcat-7.0-doc/realm-howto.html#Digested_Passwords
>>
>> [quote]
>> Non-ASCII usernames and/or passwords are supported using
>>
>> CATALINA_HOME/bin/digest.[bat|sh] -a {algorithm} -e {encoding} {input}
>>
>> but care is required to ensure that the non-ASCII input is correctly
>> passed to the digester. The digester returns {input}:{digest}. If the
>> input appears corrupted in the return, the digest will be invalid.
>> [/quote]
>>
>> BTW, That chapter in realm-howto in Tomcat 8 needs an update for the
>> new features of digest.sh / RealmBase.main().
>>
>
> I think that this have to be fixed before tagging next Tomcat 8 release.
>
> 1. Remove default value for algorithm. Ask the caller to provide
> either "-a" or "-h" option explicitly.
>
> Motivation:
> - Revert to previous behaviour.
> - I see contradiction between -a and -h, as I wrote above.
+1
> 2. Use system default encoding instead of UTF-8 by default.
>
> Motivation:
> - Revert to previous behaviour. It makes sense to expect system
> encoding when you are calling something from the command line, as that
> is the encoding that command line uses.
+1
We might want to
a) Allow credentials on stdin to avoid system encoding
b) Instruct the user to use -Dfile.encoding to alter the encoding
instead of using "-e"
> 3. Update realm-howto.html#Digested_Passwords
>
> It does not document the new -h option.
I can fix that, but only after we agree on these changes.
-chris
Re: digest.bat (RealmBase.main()) is broken in current Tomcat 8
trunk. Tomcat 8.0.14 is OK.
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-10-22 14:22 GMT+04:00 Mark Thomas <ma...@apache.org>:
> On 17/10/2014 14:13, Konstantin Kolinko wrote:
>> 2014-09-30 19:22 GMT+04:00 Konstantin Kolinko <kn...@gmail.com>:
>>> 2014-09-29 14:43 GMT+04:00 Mark Thomas <ma...@apache.org>:
>>>> On 27/09/2014 15:52, Konstantin Kolinko wrote:
>>> (....)
>>>
>>>>> 4) The current javadoc for RealmBase.main() says that algorithm (-a)
>>>>> is not required and "If not specified a default of SHA-512 will be
>>>>> used."
>>>>>
>>>>> I wonder whether that is justified.
>>>>
>>>> That is what is currently implemented. Happy to discuss changes but
>>>> SHA-512 doesn't seem unreasonable to me.
>>>
>>>
>>> I think there is a contradiction between -a <algorithm> and -h
>>> <credential handler implementation class> keys:
>>> 1) If -h is used I think it shall default to whatever default
>>> algorithm the credential handler implements.
>>> 2) Custom credential handler implementations may lack setAlgorithm() method.
>>>
>>> I think that one of (-a, -h) is required, with no default for either.
>>> The old code had no default for algorithm.
>
> I agree with the two issues above but I have a different solution.
>
> If neither -a or -h is specified, SHA-512 and
> MessageDigestCredentialHandler will be used.
>
> If only -a is specified, the built-in handlers will be searched in order
> (MessageDigestCredentialHandler, SecretKeyCredentialHandler) and the
> first handler that supports the algorithm will be used.
>
> If only -h is specified, no default will be used for -a. The handler may
> or may nor support -a and may or may not supply a sensible default.
OK for me, if you find SHA-512 default useful. It is just my personal
preference to ask the caller to specify algorithm name explicitly.
(The actual algorithm that a user needs depends upon how the realm is
configured in server.xml/context.xml. I think that not much typing is
saved by having a default here.
I think that many tools such as openssl do not have default algorithm
names in their command line. E.g. I do not see any default for
"genpkey" command
https://www.openssl.org/docs/apps/genpkey.html
)
I filed an issue for further improvements,
https://issues.apache.org/bugzilla/show_bug.cgi?id=57130
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: digest.bat (RealmBase.main()) is broken in current Tomcat 8 trunk.
Tomcat 8.0.14 is OK.
Posted by Mark Thomas <ma...@apache.org>.
On 17/10/2014 14:13, Konstantin Kolinko wrote:
> 2014-09-30 19:22 GMT+04:00 Konstantin Kolinko <kn...@gmail.com>:
>> 2014-09-29 14:43 GMT+04:00 Mark Thomas <ma...@apache.org>:
>>> On 27/09/2014 15:52, Konstantin Kolinko wrote:
>> (....)
>>
>>>> 4) The current javadoc for RealmBase.main() says that algorithm (-a)
>>>> is not required and "If not specified a default of SHA-512 will be
>>>> used."
>>>>
>>>> I wonder whether that is justified.
>>>
>>> That is what is currently implemented. Happy to discuss changes but
>>> SHA-512 doesn't seem unreasonable to me.
>>
>>
>> I think there is a contradiction between -a <algorithm> and -h
>> <credential handler implementation class> keys:
>> 1) If -h is used I think it shall default to whatever default
>> algorithm the credential handler implements.
>> 2) Custom credential handler implementations may lack setAlgorithm() method.
>>
>> I think that one of (-a, -h) is required, with no default for either.
>> The old code had no default for algorithm.
I agree with the two issues above but I have a different solution.
If neither -a or -h is specified, SHA-512 and
MessageDigestCredentialHandler will be used.
If only -a is specified, the built-in handlers will be searched in order
(MessageDigestCredentialHandler, SecretKeyCredentialHandler) and the
first handler that supports the algorithm will be used.
If only -h is specified, no default will be used for -a. The handler may
or may nor support -a and may or may not supply a sensible default.
>>> String encoding = "UTF-8";
>>
>> I think it shall use system encoding, because the value is passed on
>> the command line and is not read from file etc.
Fixed.
>> BTW, That chapter in realm-howto in Tomcat 8 needs an update for the
>> new features of digest.sh / RealmBase.main().
Fixed.
> I think that this have to be fixed before tagging next Tomcat 8 release.
I believe I have address all the outstanding concerns with these
changes. Let me know if I have missed something.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org