Skip to content

Commit

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

Hopefully this is the last fix
(cherry picked from commit 94a8789)
  • Loading branch information
kohsuke committed Jan 6, 2013
1 parent e401c7c commit a411b0c
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 a411b0c

Please sign in to comment.