Skip to content
Permalink
Browse files Browse the repository at this point in the history
[SECURITY-49] actively invalidate bad API tokens.
If the user still has the API token that's generated from secret.key,
don't accept that.

Hopefully this is the last fix
  • Loading branch information
kohsuke committed Jan 6, 2013
1 parent 3dc13b9 commit 94a8789
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
11 changes: 9 additions & 2 deletions core/src/main/java/jenkins/security/ApiTokenProperty.java
Expand Up @@ -31,6 +31,7 @@
import hudson.model.UserPropertyDescriptor;
import hudson.util.HttpResponses;
import hudson.util.Secret;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
Expand Down Expand Up @@ -61,12 +62,18 @@ public ApiTokenProperty() {
* We don't let the external code set the API token,
* but for the initial value of the token we need to compute the seed by ourselves.
*/
private ApiTokenProperty(String seed) {
/*package*/ ApiTokenProperty(String seed) {
apiToken = Secret.fromString(seed);
}

public String getApiToken() {
return Util.getDigestOf(apiToken.getPlainText());
String p = apiToken.getPlainText();
if (p.equals(Util.getDigestOf(Jenkins.getInstance().getSecretKey()+":"+user.getId()))) {
// if the current token is the initial value created by pre SECURITY-49 Jenkins, we can't use that.
// force using the newer value
apiToken = Secret.fromString(p=API_KEY_SEED.mac(user.getId()));
}
return Util.getDigestOf(p);
}

public boolean matchesPassword(String password) {
Expand Down
25 changes: 25 additions & 0 deletions test/src/test/java/jenkins/security/ApiTokenPropertyTest.java
Expand Up @@ -3,7 +3,9 @@
import com.gargoylesoftware.htmlunit.HttpWebConnection;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import hudson.Util;
import hudson.model.User;
import jenkins.model.Jenkins;
import org.apache.commons.httpclient.Credentials;
import org.apache.commons.httpclient.HttpClient;
import org.apache.commons.httpclient.UsernamePasswordCredentials;
Expand Down Expand Up @@ -60,4 +62,27 @@ public User call() throws Exception {
}
}));
}

public void testSecurity49Upgrade() throws Exception {
jenkins.setSecurityRealm(createDummySecurityRealm());
User u = User.get("foo");
String historicalInitialValue = Util.getDigestOf(Jenkins.getInstance().getSecretKey() + ":" + u.getId());

// we won't accept historically used initial value as it may be compromised
ApiTokenProperty t = new ApiTokenProperty(historicalInitialValue);
u.addProperty(t);
String apiToken1 = t.getApiToken();
assertFalse(apiToken1.equals(Util.getDigestOf(historicalInitialValue)));

// the replacement for the compromised value must be consistent and cannot be random
ApiTokenProperty t2 = new ApiTokenProperty(historicalInitialValue);
u.addProperty(t2);
assertEquals(apiToken1,t2.getApiToken());

// any other value is OK. those are changed values
t = new ApiTokenProperty(historicalInitialValue+"somethingElse");
u.addProperty(t);
assertTrue(t.getApiToken().equals(Util.getDigestOf(historicalInitialValue+"somethingElse")));

}
}

0 comments on commit 94a8789

Please sign in to comment.