[keycloak-dev] OTP string based secrets

[Dobbels, Andy]

Hi Stian,

In short: Keycloak uses the system's default character set (typically UTF8) to convert the secret to and from binary/string. This prevents people from moving existing token secrets into Keycloak.
See https://github.com/keycloak/keycloak/blob/master/services/src/main/java/org/keycloak/utils/TotpUtils.java#L37 getBytes() call.

Longer version:
Keycloak:
Secret generation: New secret = Random alphanumeric String(20)
Secret storage: varchar(4000) (in MS Sql Server) 
Conversion of Value column to binary for use in TOTP algo:  totpSecret.getBytes() which takes a string and gets the binary representation of that string using the default string encoding
Conversion of totpSecret to Base32 for consumption by end user: Base32.encode(binary totpSecret)

Most other TOTP implementations:
Secret generation: New secret = Random array of bytes.
Secret storage: Base32/Base64/HEX varchar or just a binary blob (some might also encrypt the value in the db before storing)
Conversion of Value column to binary totpSecret:  Base32/Base64/Hex decode or nothing if it's already stored as binary.
Conversion of totpSecret to Base32 for consumption by end user: Base32.encode(totpSecret)

The problem is that you cannot migrate any of your typical secrets into Keycloak as your random array of bytes cannot be reliably stored as a string as they cannot be reliably converted  because of low asci/UTF8 values or special UTF8 encodings.

It's about the storage and Keycloak relying on string encoding for conversion to binary and not about the presentation to the user (which I think you're referring to). If keycloak treated the secret as an array of bytes and stored it as BaseXX then there would be no problem at all.

My suggestions below were to allow a migration to a better way of storing secrets while not breaking existing implementations. This relies on detecting how the secret is encoded. The default is whatever the current system's default character set is.

Another way other than the 2 suggestions below to detect that the secret is stored as Base32 would be to check the length of the strings. Current length is 20, the equivalent Base32  would be 33. However, this might be deemed to be too obscure and fail if someone imports a shorter secret that ends up as length 20 as well. Perhaps it should be a configuration option that allows users to specify what encoding is used for the secret. UTF8 (default), Base32/64 or Hex.

I hope that clears up the problem.

Thanks,

Andy

From: Stian Thorgersen [mailto:sthorger at redhat.com] 
Sent: 14 July 2017 05:15
To: Dobbels, Andy
Cc: keycloak-dev at lists.jboss.org
Subject: Re: [keycloak-dev] OTP string based secrets

The OTP strings we have today are already encoded with Base32 as the OTP specs mandates [1] so I don't really understand what this whole thread is about.

[1] https://github.com/keycloak/keycloak/blob/master/services/src/main/java/org/keycloak/utils/TotpUtils.java

On 13 July 2017 at 17:17, Dobbels, Andy <adobbels at bottomline.com> wrote:
Hi Bill,

Thanks for the response and sorry for the double post.

My main concern is interoperability and not being able to import the secrets from existing OTP solutions even though they are all based on the same RFC. Creating an SPI to allow the secret to be stored as a Base32 string instead of plain text doesn't seem right. The rest of the code is fine and it's all there. If you don't mind I would like to explore a few options that don't require an update of all existing credentials.

1: Prefix the Base32 strings with an identifier. E.g. "Base32:{secret}"  That way we can keep the existing data as is. If the prefix isn't there then it's plain text.
or
2: Add a column that indicates what format the credentials.value property is encoded in. Values could be Plain or Base32. Someone could easily add Base64 or Hex if that helps their adoption/migration of otp. Perhaps later on this could open the door to encrypting the secret by having a value called "encrypted".

Perhaps there are other options?
Is the undocumented SPI purely dealing with how the value is encoded?

Thanks,

Andy


-----Original Message-----
From: keycloak-dev-bounces at lists.jboss.org [mailto:keycloak-dev-bounces at lists.jboss.org] On Behalf Of Bill Burke
Sent: 12 July 2017 23:16
To: keycloak-dev at lists.jboss.org
Subject: Re: [keycloak-dev] OTP string based secrets



On 7/12/17 1:39 PM, Dobbels, Andy wrote:
> Hi,
>
> We are adopting Keycloak and are trying to move our OTP tokens over to Keycloak. However, Keycloak can only use secrets that are alphanumeric strings whereas our existing implementation and most hard and software tokens we have used use the full range of binary values when generating secrets.

> 2 questions:
> 1: Is the lower entropy of the secrets generated by Keycloak a concern?
Should it be a concern?  Its currently a randomly generated 20 character alpha-numeric string.  That's not enough entropy?


> 2: If we provided a PR that migrated the existing data by re-encoding all existing secrets as Base32 and updated the code to assume Base32 instead of string be acceptable?
> This would be a non breaking change but allow anyone using existing OTP tokens to migrate their secrets which I don't think they can at the moment.
We have undocumented SPIs to support other storage options for different
credential types.  If you want to use the data model that's currently
there you have to encode your secrets as strings. We're limited in the
fact that our current OTP storage must be backward compatible.  Also,
don't want to have to recalculate storage for every single OTP record of
existing deployments when migrating.

We could though absolutely change how future secrets are generated if
you feel the entropy is a concern.

Bill
_______________________________________________
keycloak-dev mailing list
keycloak-dev at lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev

_______________________________________________
keycloak-dev mailing list
keycloak-dev at lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev