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

OpenID Client Auto-Generated Password Is Not Cryptographically Strong #1106

Closed
earezki opened this Issue Aug 16, 2018 · 1 comment

Comments

Projects
None yet
1 participant
@earezki
Contributor

earezki commented Aug 16, 2018

Description:

The generated OpenID password doesn't contain any special chars, and implementation uses java.util.Random which is not strong for password generation.

Steps to reproduce:

  • Open OxTrust
  • Go to OpenID Connect
  • Go to Clients
  • Select any client
  • Click on 'Auto-Generate Secret Key'

Actual:

The password contain upper and lower case chars, and numbers.
Expected:
The password contain upper and lower case chars, numbers and special chars.

image

The code that generates the password: org.gluu.oxtrust.action.UpdateClientAction#generatePassword

public void generatePassword() throws EncryptionException {
	String characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
	String pwd = RandomStringUtils.random(24, characters);
	this.client.setOxAuthClientSecret(pwd);
	this.client.setEncodedClientSecret(encryptionService.encrypt(pwd));
}

What we should have (not working just POC):

public void generatePassword() throws EncryptionException {
	String characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789~`!@#$%^&*()-_=+[{]}\\|;:\'\",<.>/?";
	String pwd = RandomStringUtils.random(24, characters, new SecureRandom());
	this.client.setOxAuthClientSecret(pwd);
	this.client.setEncodedClientSecret(encryptionService.encrypt(pwd));
}

@earezki earezki added the enhancement label Aug 16, 2018

@earezki earezki added this to the 4.0 milestone Aug 16, 2018

@earezki earezki self-assigned this Aug 16, 2018

@earezki

This comment has been minimized.

Contributor

earezki commented Aug 16, 2018

Looks like the no special char was a requirement: #953
Hence, let's just use a SecureRandom instead of Random.

earezki added a commit to earezki/oxTrust that referenced this issue Aug 18, 2018

earezki added a commit to earezki/oxTrust that referenced this issue Aug 18, 2018

earezki added a commit to earezki/oxTrust that referenced this issue Aug 21, 2018

earezki added a commit to earezki/oxTrust that referenced this issue Aug 21, 2018

yurem added a commit that referenced this issue Aug 22, 2018

Merge pull request #1113 from earezki/oxTrust/issues/#1106
#1106 use SecureRandom to generate passwords

@earezki earezki closed this Aug 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment