Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide salted-sha512 encryption scheme #652

Closed
soullivaneuh opened this issue May 28, 2020 · 15 comments
Closed

Provide salted-sha512 encryption scheme #652

soullivaneuh opened this issue May 28, 2020 · 15 comments
Labels

Comments

@soullivaneuh
Copy link

Provide salted-sha512 encryption scheme

Problem

I would like to migrate my users database to FusionAuth while keeping the password safely moved.

The current encryption of my passwords is sha512 with a salt.

This encryption scheme is currently not provided.

Solution

Support this encryption schema? 😄

Alternatives/workarounds

I may write my own plugin, but it would be better to have it out of the box.

By the way, is there a place where you list external plugins? Maybe one already exist for me.

How to vote

Please give us a thumbs up or thumbs down as a reaction to help us prioritize this feature. Feel free to comment if you have a particular need or comment on how this feature should work.

@mooreds
Copy link
Collaborator

mooreds commented May 28, 2020

Thanks for your feedback!

I wasn't able to find any list of plugins (searching github and google with "io.fusionauth.plugin.spi.PluginModule" didn't turn up anything), so I started one in the forums.

If you write a plugin, please add it. Also, if it'd be of interest, we could consider a fusionauth-password-plugins open source repo, the same way we provide a repo, but not support for, https://github.com/fusionauth/fusionauth-containers

@soullivaneuh
Copy link
Author

I started creating this 👍

package com.nexylan.fusionauth.plugins;

import io.fusionauth.plugin.spi.security.PasswordEncryptor;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;

public class SaltedSha512PasswordEncryptor implements PasswordEncryptor {
  @Override
  public int defaultFactor() {
    return 1;
  }

  @Override
  public String encrypt(String password, String salt, int factor) {
    // @see https://stackoverflow.com/a/33085670/1731473
    String generatedPassword = null;

    try {
      MessageDigest md = MessageDigest.getInstance("SHA-512");
      md.update(salt.getBytes(StandardCharsets.UTF_8));
      byte[] bytes = md.digest(password.getBytes(StandardCharsets.UTF_8));

      StringBuilder sb = new StringBuilder();
      for (int i = 0; i < bytes.length; i++) {
        sb.append(Integer.toString((bytes[i] & 0xff) + 0x100, 16).substring(1));
      }
      generatedPassword = sb.toString();
    }
    catch (NoSuchAlgorithmException e) {
      return null;
    }

    return generatedPassword;
  }
}

But it does not seems to work correctly, the password mismatch after transfer.

Do you have any suggest to make a sha512 hash? Not a java expert here. 🙃

Also, if it'd be of interest, we could consider a fusionauth-password-plugins open source repo

It's a yes! Having example of other plugins like sha256 would show the way to go, as am I asking before. 👍

@soullivaneuh
Copy link
Author

Also: How am I supposed to deal with the factor parameter? 🤔

I don't have this kind of thing on my PHP project.

@mooreds
Copy link
Collaborator

mooreds commented May 28, 2020

So here's an example password plugin: https://github.com/FusionAuth/fusionauth-example-password-encryptor/

@mooreds
Copy link
Collaborator

mooreds commented May 28, 2020

Here's more information to read about the factor: https://fusionauth.io/docs/v1/tech/reference/password-encryptors

When selecting a load factor for the hashing scheme, the minimum values are provided for a starting guideline only. The factor should be set as high as possible such that the login times are still acceptable to your end users. Be cautious with setting the factor too high, especially with Bcrypt, setting the factor too high may cause login requests to take seconds or minutes to complete.

Also has some suggested factor values for various algorithms.

@soullivaneuh
Copy link
Author

@mooreds Well, this does not help me very much: https://github.com/FusionAuth/fusionauth-example-password-encryptor/blob/master/src/main/java/com/company/fusionauth/plugins/MyExamplePasswordEncryptor.java

The plugin works and is correctly register. The issue is the encrypt algorithm itself.

@soullivaneuh
Copy link
Author

To match the symfony method, I have to concatened the password and the salt like this:

  public String encrypt(String password, String salt, int factor) {
    // @see https://stackoverflow.com/a/33085670/1731473
    // @see https://stackoverflow.com/a/2624385/1731473
    String saltedPassword = password+'{'+salt+'}';

Is it the same way you are doing it for salted-sha256?

@mooreds
Copy link
Collaborator

mooreds commented May 28, 2020

Glad you have it installed and running.

As far as the encryption algorithm, I'm not too familiar with our implementations and so can't provide any specific guidance. However, this looks like a useful resource: http://tutorials.jenkov.com/java-cryptography/index.html

And here's a working example plugin: #204

I'll add that to the example project.

@soullivaneuh
Copy link
Author

soullivaneuh commented May 28, 2020

I finally found the way to copy the symfony encryption method:

package com.nexylan.fusionauth.plugins;

import io.fusionauth.plugin.spi.security.PasswordEncryptor;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.Base64;
import javax.xml.bind.DatatypeConverter;

public class SymfonySaltedSha512PasswordEncryptor implements PasswordEncryptor {
  @Override
  public int defaultFactor() {
    return 1;
  }

  @Override
  public String encrypt(String password, String salt, int factor) {
    if (factor <= 0) {
      throw new IllegalArgumentException("Invalid factor value [" + factor + "]");
    }

    // Basic translation of the following symfony method: https://github.com/symfony/symfony/blob/f9411ab4806f7f669deb4bea927cd2f251411c52/src/Symfony/Component/Security/Core/Encoder/MessageDigestPasswordEncoder.php#L50-L69
    byte[] salted = (password+'{'+salt+'}').getBytes();
    try {
      MessageDigest md = MessageDigest.getInstance("SHA-512");
      md.update(salted);
      byte[] digest = md.digest();

      for (int i = 1; i < factor; i++) {
        byte[] toDigest = new byte[digest.length + salted.length];
        System.arraycopy(digest, 0, toDigest, 0, digest.length);
        System.arraycopy(salted, 0, toDigest, digest.length, salted.length);

        digest = md.digest(toDigest);
      }

      return Base64.getEncoder().encodeToString(digest);
    } catch (NoSuchAlgorithmException e) {
      throw new IllegalArgumentException("No such algorithm [SHA-512]");
    }
  }
}

I renamed it to symfony-salted-sha512 because I'm not sure the method follow any language agnostic convention.

Having an open-source repository for the encryptors with the possibility to add some with contribution is a great idea.

Also, you may have to adjust the plugin API later to add more options.

Indeed, I use base64 encode at the end, but Symfony let us the choice between base64 and hex printing: https://github.com/symfony/symfony/blob/f9411ab4806f7f669deb4bea927cd2f251411c52/src/Symfony/Component/Security/Core/Encoder/MessageDigestPasswordEncoder.php#L68

If I want to implement both, I will have to create two separated encryptor for that. Not sure what is the best.

What do you think?

@robotdan
Copy link
Member

robotdan commented May 28, 2020

@soullivaneuh re: base64 and hex, I think what you're saying is the hash may be stored in either form?

We will still need it in base64, so as part of the migration you will just have to take an hex encoded hashes, and re-encode them using base64.

Or.. you could detect that in your plugin and get the value into a usable state regardless of how it was passed in.

Once you have a working plugin, we could also make a contrib folder or something like that in this repo and publish the code there for others to utilize.
https://github.com/FusionAuth/fusionauth-example-password-encryptor

@robotdan
Copy link
Member

@soullivaneuh here is the equivalent to the way our salted-sha256 algorithm works with SHA-512.
https://github.com/FusionAuth/fusionauth-example-password-encryptor/blob/master/src/main/java/com/mycompany/fusionauth/plugins/ExampleSaltedSHA512PasswordEncryptor.java

There are different ways to build the hash, so it will really depend on how Symfony builds their hash if you are using it to import users from their system.

@soullivaneuh
Copy link
Author

Now I have this error during the bulk import:

The [user.salt] is expected to be a Base64 encoded string. Invalid salt(s) [bUg14CPiIorJUrPa7UUyV6fPRbzRvX/HB63.oSsuLwI].

But if I encore with base64 from the source, the password are not correctly encrypted.

How should I solve this issue?

@robotdan
Copy link
Member

It is possible the incoming base64 encoded salt is using a different character set. Bcrypt does this, they use a . instead of a + - so the base64 validation is different.

The PasswordEncryptor has a default method you can override in your encryptor called validateSalt. The default value uses the normal Base64 character set.

For example, the bcrypt one overrides with the following value:

  /**
   * This pattern represents a modified Base64 encoding scheme used by Bcrypt.
   * <p>
   * The Bcrypt algorithm replaces the '+' with a '.' which is not valid in a standard Base64 encoding scheme.
   */
  private static final Pattern BcryptSaltPattern = Pattern.compile("^[0-9A-Za-z./]+=*$");

  @Override
  public boolean validateSalt(String salt) {
    return BcryptSaltPattern.matcher(salt).find();
  }

@soullivaneuh
Copy link
Author

@robotdan Nevermind, this was an error from I because of a missing base64 encoding. 🙃

@robotdan
Copy link
Member

I believe this has been resolved. Please re-open if you have questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants