Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix issue #1179 - OneDev should use crypto strong random string for a…
…ccess token and password reset
  • Loading branch information
robinshine committed Feb 7, 2023
1 parent 92c3a12 commit d67dd96
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 133 deletions.
22 changes: 10 additions & 12 deletions server-core/src/main/java/io/onedev/server/git/hook/HookUtils.java
@@ -1,26 +1,24 @@
package io.onedev.server.git.hook;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.Charset;
import java.util.HashMap;
import java.util.Map;

import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.RandomStringUtils;

import com.google.common.base.Preconditions;

import io.onedev.commons.utils.FileUtils;
import io.onedev.commons.utils.StringUtils;
import io.onedev.server.OneDev;
import io.onedev.server.ServerConfig;
import io.onedev.server.entitymanager.SettingManager;
import io.onedev.server.util.CryptoUtils;
import org.apache.commons.io.IOUtils;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.Charset;
import java.util.HashMap;
import java.util.Map;

public class HookUtils {

public static final String HOOK_TOKEN = RandomStringUtils.randomAlphanumeric(20);
public static final String HOOK_TOKEN = CryptoUtils.generateSecret();

private static final String gitReceiveHook;

Expand Down
Expand Up @@ -10,9 +10,9 @@
import io.onedev.server.markdown.MarkdownManager;
import io.onedev.server.markdown.MentionParser;
import io.onedev.server.model.*;
import io.onedev.server.util.CryptoUtils;
import io.onedev.server.util.Pair;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.tuple.Triple;
import org.dom4j.Element;
import org.dom4j.Node;
Expand Down Expand Up @@ -2146,7 +2146,7 @@ private void migrate42(File dataDir, Stack<Integer> versions) {
} else if (file.getName().startsWith("Users.xml")) {
VersionedXmlDoc dom = VersionedXmlDoc.fromFile(file);
for (Element element: dom.getRootElement().elements()) {
element.addElement("accessToken").setText(RandomStringUtils.randomAlphanumeric(40));
element.addElement("accessToken").setText(CryptoUtils.generateSecret());
element.addElement("ssoInfo").addElement("subject").setText(UUID.randomUUID().toString());
}
dom.writeToFile(file, false);
Expand Down Expand Up @@ -2692,7 +2692,7 @@ private void migrate61(File dataDir, Stack<Integer> versions) {
element.addElement("ssoInfo").addElement("subject").setText(UUID.randomUUID().toString());
element.addElement("email").setText("unknown email");
element.addElement("alternateEmails");
element.addElement("accessToken").setText(RandomStringUtils.randomAlphanumeric(User.ACCESS_TOKEN_LEN));
element.addElement("accessToken").setText(CryptoUtils.generateSecret());
element.addElement("userProjectQueries");
element.addElement("userIssueQueries");
element.addElement("userIssueQueryWatches");
Expand Down Expand Up @@ -4831,5 +4831,18 @@ private void migrate109(File dataDir, Stack<Integer> versions) {
}
}
}

private void migrate110(File dataDir, Stack<Integer> versions) {
var updateIds = new HashSet<>();
for (File file: dataDir.listFiles()) {
if (file.getName().startsWith("Users.xml")) {
VersionedXmlDoc dom = VersionedXmlDoc.fromFile(file);
for (Element element: dom.getRootElement().elements()) {
element.element("accessToken").setText(CryptoUtils.generateSecret());
}
dom.writeToFile(file, false);
}
}
}

}
22 changes: 7 additions & 15 deletions server-core/src/main/java/io/onedev/server/model/EmailAddress.java
@@ -1,24 +1,16 @@
package io.onedev.server.model;

import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.FetchType;
import javax.persistence.Index;
import javax.persistence.JoinColumn;
import javax.persistence.ManyToOne;
import javax.persistence.Table;

import org.apache.commons.lang3.RandomStringUtils;
import com.fasterxml.jackson.annotation.JsonIgnore;
import io.onedev.server.util.CryptoUtils;
import io.onedev.server.util.facade.EmailAddressFacade;
import io.onedev.server.web.editable.annotation.Editable;
import org.hibernate.annotations.Cache;
import org.hibernate.annotations.CacheConcurrencyStrategy;

import javax.persistence.*;
import javax.validation.constraints.Email;
import javax.validation.constraints.NotEmpty;

import com.fasterxml.jackson.annotation.JsonIgnore;

import io.onedev.server.util.facade.EmailAddressFacade;
import io.onedev.server.web.editable.annotation.Editable;

@Editable
@Entity
@Table(indexes={@Index(columnList="o_owner_id"), @Index(columnList="value")})
Expand All @@ -35,7 +27,7 @@ public class EmailAddress extends AbstractEntity {
private String value;

@JsonIgnore
private String verificationCode = RandomStringUtils.randomAlphanumeric(16);
private String verificationCode = CryptoUtils.generateSecret();

private boolean primary;

Expand Down
54 changes: 17 additions & 37 deletions server-core/src/main/java/io/onedev/server/model/User.java
@@ -1,41 +1,7 @@
package io.onedev.server.model;

import static io.onedev.server.model.User.PROP_ACCESS_TOKEN;
import static io.onedev.server.model.User.PROP_FULL_NAME;
import static io.onedev.server.model.User.PROP_NAME;
import static io.onedev.server.model.User.PROP_SSO_CONNECTOR;

import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Optional;
import java.util.Stack;
import java.util.stream.Collectors;

import javax.annotation.Nullable;
import javax.persistence.CascadeType;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Index;
import javax.persistence.Lob;
import javax.persistence.OneToMany;
import javax.persistence.Table;

import org.apache.commons.lang3.RandomStringUtils;
import org.apache.shiro.authc.AuthenticationInfo;
import org.apache.shiro.subject.PrincipalCollection;
import org.apache.shiro.subject.SimplePrincipalCollection;
import org.apache.shiro.subject.Subject;
import org.eclipse.jgit.lib.PersonIdent;
import org.hibernate.annotations.Cache;
import org.hibernate.annotations.CacheConcurrencyStrategy;
import javax.validation.constraints.NotEmpty;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.google.common.base.MoreObjects;

import edu.emory.mathcs.backport.java.util.Collections;
import io.onedev.commons.utils.ExplicitException;
import io.onedev.server.OneDev;
Expand All @@ -51,12 +17,28 @@
import io.onedev.server.model.support.issue.NamedIssueQuery;
import io.onedev.server.model.support.pullrequest.NamedPullRequestQuery;
import io.onedev.server.security.SecurityUtils;
import io.onedev.server.util.CryptoUtils;
import io.onedev.server.util.facade.UserFacade;
import io.onedev.server.util.validation.annotation.UserName;
import io.onedev.server.util.watch.QuerySubscriptionSupport;
import io.onedev.server.util.watch.QueryWatchSupport;
import io.onedev.server.web.editable.annotation.Editable;
import io.onedev.server.web.editable.annotation.Password;
import org.apache.shiro.authc.AuthenticationInfo;
import org.apache.shiro.subject.PrincipalCollection;
import org.apache.shiro.subject.SimplePrincipalCollection;
import org.apache.shiro.subject.Subject;
import org.eclipse.jgit.lib.PersonIdent;
import org.hibernate.annotations.Cache;
import org.hibernate.annotations.CacheConcurrencyStrategy;

import javax.annotation.Nullable;
import javax.persistence.*;
import javax.validation.constraints.NotEmpty;
import java.util.*;
import java.util.stream.Collectors;

import static io.onedev.server.model.User.*;

@Entity
@Table(
Expand All @@ -68,8 +50,6 @@ public class User extends AbstractEntity implements AuthenticationInfo {

private static final long serialVersionUID = 1L;

public static final int ACCESS_TOKEN_LEN = 40;

public static final Long UNKNOWN_ID = -2L;

public static final Long SYSTEM_ID = -1L;
Expand Down Expand Up @@ -117,7 +97,7 @@ protected Stack<User> initialValue() {

@Column(unique=true, nullable=false)
@JsonIgnore
private String accessToken = RandomStringUtils.randomAlphanumeric(ACCESS_TOKEN_LEN);
private String accessToken = CryptoUtils.generateSecret();

@JsonIgnore
@Lob
Expand Down
@@ -1,21 +1,19 @@
package io.onedev.server.model.support;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;

import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.Size;

import org.apache.commons.lang3.RandomStringUtils;

import io.onedev.server.event.project.RefUpdated;
import io.onedev.server.event.project.build.BuildEvent;
import io.onedev.server.event.project.codecomment.CodeCommentEvent;
import io.onedev.server.event.project.issue.IssueEvent;
import io.onedev.server.event.project.pullrequest.PullRequestEvent;
import io.onedev.server.util.CryptoUtils;
import io.onedev.server.web.editable.annotation.Editable;

import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.Size;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;

@Editable
public class WebHook implements Serializable {

Expand Down Expand Up @@ -70,7 +68,7 @@ public boolean includes(Object event) {

private List<EventType> eventTypes = new ArrayList<>();

private String secret = RandomStringUtils.randomAlphanumeric(20);
private String secret = CryptoUtils.generateSecret();

@Editable(order=100, description="The URL of the server endpoint that will receive the webhook POST requests")
@NotEmpty
Expand Down
31 changes: 21 additions & 10 deletions server-core/src/main/java/io/onedev/server/util/CryptoUtils.java
@@ -1,22 +1,24 @@
package io.onedev.server.util;

import java.security.NoSuchAlgorithmException;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.KeySpec;

import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;

import io.onedev.server.OneDev;
import io.onedev.server.entitymanager.SettingManager;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.shiro.crypto.AesCipherService;
import org.apache.sshd.common.digest.BaseDigest;
import org.apache.sshd.common.digest.Digest;

import io.onedev.server.OneDev;
import io.onedev.server.entitymanager.SettingManager;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.KeySpec;

public class CryptoUtils {


private static final int DEFAULT_SECRET_LEN = 40;

public static final Digest DIGEST_FORMAT = new BaseDigest("MD5", 512);

private static volatile KeyPair keyPair;
Expand All @@ -40,7 +42,16 @@ public static byte[] getCipherKey() {
}
return keyPair.getCipherKey();
}

public static String generateSecret(int count) {
return RandomStringUtils.random(count, 0, 0, true, true, null,
new SecureRandom());
}

public static String generateSecret() {
return generateSecret(DEFAULT_SECRET_LEN);
}

public static byte[] encrypt(byte[] data) {
return cipherService.encrypt(data, getCipherKey()).getBytes();
}
Expand Down
@@ -1,19 +1,18 @@
package io.onedev.server.web.component.user.accesstoken;

import org.apache.commons.lang3.RandomStringUtils;
import io.onedev.server.OneDev;
import io.onedev.server.entitymanager.UserManager;
import io.onedev.server.model.User;
import io.onedev.server.util.CryptoUtils;
import io.onedev.server.web.component.link.copytoclipboard.CopyToClipboardLink;
import io.onedev.server.web.util.ConfirmClickModifier;
import org.apache.wicket.Session;
import org.apache.wicket.markup.html.form.TextField;
import org.apache.wicket.markup.html.link.Link;
import org.apache.wicket.markup.html.panel.Panel;
import org.apache.wicket.model.AbstractReadOnlyModel;
import org.apache.wicket.model.IModel;

import io.onedev.server.OneDev;
import io.onedev.server.entitymanager.UserManager;
import io.onedev.server.model.User;
import io.onedev.server.web.component.link.copytoclipboard.CopyToClipboardLink;
import io.onedev.server.web.util.ConfirmClickModifier;

@SuppressWarnings("serial")
public abstract class AccessTokenPanel extends Panel {

Expand Down Expand Up @@ -50,7 +49,7 @@ protected String[] getInputTypes() {

@Override
public void onClick() {
getUser().setAccessToken(RandomStringUtils.randomAlphanumeric(User.ACCESS_TOKEN_LEN));
getUser().setAccessToken(CryptoUtils.generateSecret());
OneDev.getInstance(UserManager.class).save(getUser());
Session.get().success("Access token regenerated");
setResponsePage(getPage());
Expand Down
Expand Up @@ -46,10 +46,7 @@ <h5 id="modal-title" class="modal-title">Set Up Two-factor Authentication</h5>
can not access the authentication application. They will <b>NOT</b> be displayed again
</div>
<div>
<div wicket:id="recoveryCodes" class="d-flex justify-content-between text-monospace font-size-lg font-weight-bold">
<div wicket:id="left"></div>
<div wicket:id="right"></div>
</div>
<div wicket:id="recoveryCodes" class="d-flex justify-content-center text-monospace font-size-sm font-weight-bold"></div>
</div>
</div>
<div class="modal-footer d-flex justify-content-center">
Expand Down

0 comments on commit d67dd96

Please sign in to comment.