[jira] [Comment Edited] (ARTEMIS-4154) DefaultSensitiveStringCodec fails decoding one specific password

["Justin Bertram (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/ARTEMIS-4154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17684234#comment-17684234 ] 

Justin Bertram edited comment on ARTEMIS-4154 at 2/5/23 5:06 AM:
-----------------------------------------------------------------

I believe the problem is the use of {{BigInteger}} as a vehicle to make the value "human readable" as hexadecimal. Unfortunately there are some situations where the {{byte[]}} passed when constructing a {{BigInteger}} _does not_ match what is then returned by {{BigInteger.getByteArray()}}. Here's a little test I reverse engineered based on the password in your description:
{code:java}
   @Test
   public void testBigIntegerByteArrayEquivalency() {
      byte[] originalBytes = new byte[] {-1, -2};
      BigInteger original = new BigInteger(originalBytes);
      assertArrayEquals(originalBytes, original.toByteArray());
   }{code}
This test will fail, and _any_ encoded password whose first byte is {{-1}} and second byte is negative will fail to be converted to/from a {{BigInteger}} properly.

I see two potential solutions here:
# Perform a sanity check when running the {{mask}} command to ensure the password can be decoded properly and issue a warning or error if it can't. This has the benefit of backwards compatibility, but it means that folks may have to come up with a different password from time to time.
# Change the implementation of {{DefaultSensitiveStringCodec}} (e.g. to use Base64 instead of {{BigInteger}}), but that means that any user upgrading to the new version will have to *re-mask all their passwords*. This has the benefit of always providing a valid encoding, but of course it's not backwards compatible.


was (Author: jbertram):
I believe the problem is the use of {{BigInteger}} as a vehicle to make the value "human readable" as hexadecimal. Unfortunately there are some situations where the {{byte[]}} passed when constructing a {{BigInteger}} _does not_ match what is then returned by {{BigInteger.getByteArray()}}. Here's a little test I reverse engineered based on the password in your description:
{code:java}
   @Test
   public void testBigIntegerByteArrayEquivalency() {
      byte[] originalBytes = new byte[] {-1, -2};
      BigInteger original = new BigInteger(originalBytes);
      assertArrayEquals(originalBytes, original.toByteArray());
   }{code}
This test will fail, and _any_ encoded password whose first byte is {{-1}} and second byte is negative will fail to be converted to/from a {{BigInteger}} properly.

I see two potential solutions here:
# Perform a sanity check when running the {{mask}} command to ensure the password can be decoded properly and issue a warning or error if it can't. This has the benefit of backwards compatibility, but it means that folks may have to come up with a different password from time to time.
# Change the implementation of {{DefaultSensitiveStringCodec}} (e.g. to use Base64 instead of {{BigInteger}}), but that means that any user upgrading to the new version will have to **re-mask all their passwords**. This has the benefit of always providing a valid encoding, but of course it's not backwards compatible.

> DefaultSensitiveStringCodec fails decoding one specific password
> ----------------------------------------------------------------
>
>                 Key: ARTEMIS-4154
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-4154
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>          Components: Broker
>    Affects Versions: 2.19.1
>            Reporter: Bahri Gencsoy
>            Priority: Major
>
> Today we encountered one specific randomly generated password failed to be decoded. It doesn't seem to be related to the string length.
> Here is the password:
>     {{jhpwtq8NdCBzNFYoWD3B}}
> Here is the encoded form:
>     {{-de884782f1dc50d5982a3c36328f65c9defd145c31633}}
> Here is the (wrongly) decoded form:
>     {{9}W/}}
>     {{y^�dCBzNFYoWD3B}}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)