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

MSFT_AADUser: hard coded password for new user #2599

Closed
andreasmarx opened this issue Nov 27, 2022 · 8 comments · Fixed by #2637 or #2667
Closed

MSFT_AADUser: hard coded password for new user #2599

andreasmarx opened this issue Nov 27, 2022 · 8 comments · Fixed by #2637 or #2667
Labels
Bug Something isn't working Entra ID V1.22.1109.1 Version 1.22.1109.1

Comments

@andreasmarx
Copy link

Details of the scenario you tried and the problem that is occurring

The password for new users is hard coded in line 495. The parameter "password" you can pass is ignored and never used. This is critical, because every user created with M365DSC will have the same public available password.

Verbose logs showing the problem

no error/no logs

Suggested solution to the issue

Make sure, that the password provided as a parameter is used in the resource.

The DSC configuration that is used to reproduce the issue (as detailed as possible)

Password = New-Object System.Management.Automation.PSCredential('Password', (ConvertTo-SecureString 'your password' -AsPlainText -Force));;

The operating system the target node is running

N/A

Version of the DSC module that was used ('dev' if using current dev branch)

1.22.1109.1 (Resource code did not change in the newer releases)

@andikrueger andikrueger added Bug Something isn't working Entra ID V1.22.1109.1 Version 1.22.1109.1 labels Nov 28, 2022
@andikrueger
Copy link
Collaborator

There is a method GeneratePassword within the System.Web.Security namespace. This could be used for a generic password creation.

[System.Web.Security.Membership]::GeneratePassword(10, 2)

Would generate a password with 10 characters and 2 special characters.

https://learn.microsoft.com/de-de/dotnet/api/system.web.security.membership.generatepassword?view=netframework-4.8

@andreasmarx
Copy link
Author

Sounds good. So I would suggest to change the resource as follows:

old code:
$PasswordProfile = @{
Password = 'TempP@ss'
}

new code:
If ($Password -eq "") {
$PasswordProfile = @{
Password = [System.Web.Security.Membership]::GeneratePassword(30, 2)
}
}
Else {
$PasswordProfile = @{
Password = $password
}
}

So only if you do not pass a password by parameter a random password (should be 30 characters minimum) is generated, otherwise the passed password is used.

The password passed as a parameter has to be a cleartext string then. The type of this parameter would have to be changed from "[System.Management.Automation.PSCredential]" to "string".

Don't know if passing the password as a cleartext string would impact security? I'm an still not so familiar with the module, so don't know if that would be somewhere logged? Quite sure it would be in the mof file as cleartext?

Maybe it would be better to remove the password parameter (did not have any effect either) and always let a random password be generated. Then one would have to reset the password later ... That should be clearly explained in the resource reference then.

What do you think?

@andikrueger
Copy link
Collaborator

I just did a review of the resource and there is a bigger issue. The used Graph cmdLet do require string input for the password. The current implementation of this resource is somehow misleading.

  1. The current implementation does not use the password of the account to create the account.
  2. There is no way to return the password of a user
  3. New users are created with the default password TempP@ss

I would suggest the following:

  1. Update the code as described above to use a strong password
  2. Mark the $Password parameter as deprecated and remove it from exports

@NikCharlebois am I missing something?

@NikCharlebois
Copy link
Collaborator

The challenge is that if the code generates a random strong password, there is no way for us to communicate what it is back to the user. I believe the solution here would be to continue to accept the Password as a PSCredential so that it gets encrypted in the MOF, but honor it from the resource by then passing it as string to the Graph cmdlet.

@NikCharlebois
Copy link
Collaborator

NikCharlebois commented Dec 8, 2022

I am recommending the following changes to be made to the Set-TargetResource. Would that solve the initial issue @andreasmarx where you wanted the provided password value to be honored by the ressource?

$passwordValue = 'TempP@ss'
if ($null -ne $Password)
{
    $passwordValue = $Password.GetNetworkCredential().Password
}
$PasswordProfile = @{
    Password = $passwordValue
}

@andikrueger
Copy link
Collaborator

Do you really it’s a plausible use case the create user accounts with passwords with M365DSC? I see a high risk in this approach? Why should an administrator manage all passwords for a whole tenant - he would know all passwords in this case. Further more we are not able to export passwords…

NikCharlebois added a commit to NikCharlebois/Microsoft365DSC that referenced this issue Dec 8, 2022
@NikCharlebois
Copy link
Collaborator

Agreed, but in order to create a user, we need to provide a password. Therefore, we need to agree on an approach

@andikrueger
Copy link
Collaborator

Would go for a strong and random password then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Entra ID V1.22.1109.1 Version 1.22.1109.1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants