Skip to content

CKS: generate a strong random password for CKS user #11637

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

CKS: generate a strong random password for CKS user #11637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/src/main/java/com/cloud/user/User.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public interface User extends OwnedBy, InternalIdentity {

// UNKNOWN and NATIVE can be used interchangeably
public enum Source {
OAUTH2, LDAP, SAML2, SAML2DISABLED, UNKNOWN, NATIVE
OAUTH2, LDAP, SAML2, SAML2DISABLED, UNKNOWN, NATIVE, CKS
}

public static final long UID_SYSTEM = 1;
Expand Down
20 changes: 16 additions & 4 deletions ...etes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.user.PasswordPolicy;
import com.cloud.uservm.UserVm;
import com.cloud.utils.PasswordGenerator;
import com.cloud.vm.UserVmService;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.Role;
Expand Down Expand Up @@ -242,6 +244,7 @@ public class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne
);
private static final String PROJECT_KUBERNETES_ACCOUNT_FIRST_NAME = "Kubernetes";
private static final String PROJECT_KUBERNETES_ACCOUNT_LAST_NAME = "Service User";
private static final int CKS_USER_MIN_PASSWORD_LENGTH = 12;


private static final String DEFAULT_NETWORK_OFFERING_FOR_KUBERNETES_SERVICE_DISPLAY_TEXT = "Network Offering used for CloudStack Kubernetes service";
Expand Down Expand Up @@ -1499,6 +1502,14 @@ protected String[] createUserApiKeyAndSecretKey(long userId) {
}
}

protected String generateRandomUserPassword(Long domainId) {
Integer passwordPolicyMinimumLength = PasswordPolicy.PasswordPolicyMinimumLength.valueIn(domainId);
if (passwordPolicyMinimumLength == null || passwordPolicyMinimumLength < CKS_USER_MIN_PASSWORD_LENGTH) {
passwordPolicyMinimumLength = CKS_USER_MIN_PASSWORD_LENGTH;
Copy link
Contributor

@sureshanaparti sureshanaparti Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

operator doesn't know that the min length is implicitly set here if it's configured between 1 and 11. Update the config description that min 12 length is considered for CKS users, or better we restrict the min length to be >= 12 for all (to be strong for all cases).

Copy link
Member Author

@weizhouapache weizhouapache Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sureshanaparti
I do not understand your concern
this is used to generate a strong random password for CKS user (the password is unknown for every one)

Copy link
Contributor

@sureshanaparti sureshanaparti Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weizhouapache not the password, the minimum length considered for generating it (as the config setting is overridden if it set between 1 and 11)

Copy link
Member Author

@weizhouapache weizhouapache Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think nobody will care about the length of password of CKS user, as nobody knows and uses the password.

12 is good enough in my opinion. But if user wants to have user passwords with more length, just respect it.

Copy link
Contributor

@sureshanaparti sureshanaparti Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as you are addressing it, better not allow less than 10 as minimum length for passwords (if not in 4.20.2, we can address in 4.22, need to add a check for config)

Copy link
Member Author

@weizhouapache weizhouapache Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually CKS user password policy check is ignored, that's the main change of this PR

        if (!User.Source.CKS.equals(source)) {
            passwordPolicy.verifyIfPasswordCompliesWithPasswordPolicies(password, userName, getAccount(accountId).getDomainId());
        }

the password policy check has several checks (regex, username, min length, min uppcase, min lowercase, min digits)
ideally there should be a method to generate a random password which matches all the password requirements, it looks difficult so far.

}
return PasswordGenerator.generateRandomPassword(passwordPolicyMinimumLength);
}

protected String[] getServiceUserKeys(Account owner) {
String username = owner.getAccountName();
if (!username.startsWith(KUBEADMIN_ACCOUNT_NAME + "-")) {
Expand All @@ -1507,8 +1518,9 @@ protected String[] getServiceUserKeys(Account owner) {
UserAccount kubeadmin = accountService.getActiveUserAccount(username, owner.getDomainId());
String[] keys;
if (kubeadmin == null) {
User kube = userDao.persist(new UserVO(owner.getAccountId(), username, UUID.randomUUID().toString(), owner.getAccountName(),
KUBEADMIN_ACCOUNT_NAME, "kubeadmin", null, UUID.randomUUID().toString(), User.Source.UNKNOWN));
Integer passwordPolicyMinimumLength = PasswordPolicy.PasswordPolicyMinimumLength.valueIn(owner.getDomainId());
User kube = userDao.persist(new UserVO(owner.getAccountId(), username, generateRandomUserPassword(owner.getDomainId()), owner.getAccountName(),
KUBEADMIN_ACCOUNT_NAME, "kubeadmin", null, UUID.randomUUID().toString(), User.Source.CKS));
keys = createUserApiKeyAndSecretKey(kube.getId());
} else {
String apiKey = kubeadmin.getApiKey();
Expand Down Expand Up @@ -1551,9 +1563,9 @@ protected Account createProjectKubernetesAccount(final Project project, final St
try {
Role role = getProjectKubernetesAccountRole();
UserAccount userAccount = accountService.createUserAccount(accountName,
UuidUtils.first(UUID.randomUUID().toString()), PROJECT_KUBERNETES_ACCOUNT_FIRST_NAME,
generateRandomUserPassword(project.getDomainId()), PROJECT_KUBERNETES_ACCOUNT_FIRST_NAME,
PROJECT_KUBERNETES_ACCOUNT_LAST_NAME, null, null, accountName, Account.Type.NORMAL, role.getId(),
project.getDomainId(), null, null, null, null, User.Source.NATIVE);
project.getDomainId(), null, null, null, null, User.Source.CKS);
projectManager.assignAccountToProject(project, userAccount.getAccountId(), ProjectAccount.Role.Regular,
userAccount.getId(), null);
Account account = accountService.getAccount(userAccount.getAccountId());
Expand Down
4 changes: 3 additions & 1 deletion server/src/main/java/com/cloud/user/AccountManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2747,7 +2747,9 @@ protected UserVO createUser(long accountId, String userName, String password, St
logger.debug("Creating user: " + userName + ", accountId: " + accountId + " timezone:" + timezone);
}

passwordPolicy.verifyIfPasswordCompliesWithPasswordPolicies(password, userName, getAccount(accountId).getDomainId());
if (!User.Source.CKS.equals(source)) {
passwordPolicy.verifyIfPasswordCompliesWithPasswordPolicies(password, userName, getAccount(accountId).getDomainId());
}

String encodedPassword = null;
for (UserAuthenticator authenticator : _userPasswordEncoders) {
Expand Down