Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JENKINS-19081] Removed DownloadSettings.checkSignature; back to usin…
…g a plain system property for this.

(cherry picked from commit df9c92e)
  • Loading branch information
jglick authored and olivergondza committed Apr 13, 2014
1 parent 38b5305 commit ca976da
Show file tree
Hide file tree
Showing 13 changed files with 26 additions and 70 deletions.
3 changes: 1 addition & 2 deletions core/src/main/java/hudson/PluginManager.java
Expand Up @@ -111,7 +111,6 @@
import hudson.model.DownloadService;
import hudson.util.FormValidation;
import static java.util.logging.Level.WARNING;
import jenkins.security.DownloadSettings;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

Expand Down Expand Up @@ -801,7 +800,7 @@ public HttpResponse doUploadPlugin(StaplerRequest req) throws IOException, Servl
@Restricted(NoExternalUse.class)
@RequirePOST public HttpResponse doCheckUpdatesServer() throws IOException {
for (UpdateSite site : Jenkins.getInstance().getUpdateCenter().getSites()) {
FormValidation v = site.updateDirectlyNow(DownloadSettings.get().isCheckSignature());
FormValidation v = site.updateDirectlyNow(DownloadService.signatureCheck);
if (v.kind != FormValidation.Kind.OK) {
// TODO crude but enough for now
return v;
Expand Down
11 changes: 8 additions & 3 deletions core/src/main/java/hudson/model/DownloadService.java
Expand Up @@ -38,7 +38,7 @@
import java.net.URL;
import java.net.URLEncoder;
import java.util.logging.Logger;
import jenkins.security.DownloadSettings;
import jenkins.model.DownloadSettings;
import jenkins.model.Jenkins;
import jenkins.util.JSONSignatureValidator;
import net.sf.json.JSONException;
Expand Down Expand Up @@ -302,7 +302,7 @@ public void doPostBack(StaplerRequest req, StaplerResponse rsp) throws IOExcepti
private FormValidation load(String json, long dataTimestamp) throws IOException {
JSONObject o = JSONObject.fromObject(json);

if (DownloadSettings.get().isCheckSignature()) {
if (signatureCheck) {
FormValidation e = new JSONSignatureValidator("downloadable '"+id+"'").verifySignature(o);
if (e.kind!= Kind.OK) {
return e;
Expand Down Expand Up @@ -346,7 +346,12 @@ public static Downloadable get(String id) {

public static boolean neverUpdate = Boolean.getBoolean(DownloadService.class.getName()+".never");

/** Now used only to set default value of, and enable UI switching of, {@link DownloadSettings#setIgnoreSignature}. */
/**
* May be used to temporarily disable signature checking on {@link DownloadService} and {@link UpdateCenter}.
* Useful when upstream signatures are broken, such as due to expired certificates.
* Should only be used when {@link DownloadSettings#isUseBrowser};
* disabling signature checks for in-browser downloads is <em>very dangerous</em> as unprivileged users could submit spoofed metadata!
*/
public static boolean signatureCheck = !Boolean.getBoolean(DownloadService.class.getName()+".noSignatureCheck");
}

3 changes: 1 addition & 2 deletions core/src/main/java/hudson/model/UpdateCenter.java
Expand Up @@ -88,7 +88,6 @@
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import jenkins.security.DownloadSettings;
import org.acegisecurity.context.SecurityContextHolder;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -631,7 +630,7 @@ public List<Plugin> getUpdates() {
public List<FormValidation> updateAllSites() throws InterruptedException, ExecutionException {
List <Future<FormValidation>> futures = new ArrayList<Future<FormValidation>>();
for (UpdateSite site : getSites()) {
Future<FormValidation> future = site.updateDirectly(DownloadSettings.get().isCheckSignature());
Future<FormValidation> future = site.updateDirectly(DownloadService.signatureCheck);
if (future != null) {
futures.add(future);
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/model/UpdateSite.java
Expand Up @@ -58,7 +58,7 @@
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import jenkins.model.Jenkins;
import jenkins.security.DownloadSettings;
import jenkins.model.DownloadSettings;
import jenkins.util.JSONSignatureValidator;
import net.sf.json.JSONException;
import net.sf.json.JSONObject;
Expand Down
Expand Up @@ -22,24 +22,19 @@
* THE SOFTWARE.
*/

package jenkins.security;
package jenkins.model;

import hudson.Extension;
import hudson.PluginManager;
import hudson.model.AsyncPeriodicWork;
import hudson.model.DownloadService;
import hudson.model.TaskListener;
import hudson.model.UpdateSite;
import hudson.util.FormValidation;
import java.io.IOException;
import jenkins.model.GlobalConfiguration;
import jenkins.model.GlobalConfigurationCategory;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;

/**
Expand All @@ -55,17 +50,11 @@ public static DownloadSettings get() {
}

private boolean useBrowser = true; // historical default, not necessarily recommended
@SuppressWarnings("deprecation")
private boolean checkSignature = DownloadService.signatureCheck;

public DownloadSettings() {
load();
}

@Override public GlobalConfigurationCategory getCategory() {
return GlobalConfigurationCategory.get(GlobalConfigurationCategory.Security.class);
}

@Override public boolean configure(StaplerRequest req, JSONObject json) throws FormException {
req.bindJSON(this, json);
return true;
Expand All @@ -80,30 +69,6 @@ public void setUseBrowser(boolean useBrowser) {
save();
}

public boolean isCheckSignature() {
return checkSignature;
}

public void setCheckSignature(boolean checkSignature) {
if (!checkSignature) {
// Just to be on the safe side. Normally this is implied by ADMINISTER, needed to configure the security screen anyway,
// but in case ADMINISTER but not CONFIGURE_UPDATECENTER is somehow granted, make sure signature checking cannot be disabled.
Jenkins.getInstance().checkPermission(PluginManager.CONFIGURE_UPDATECENTER);
}
this.checkSignature = checkSignature;
save();
}

public FormValidation doCheckCheckSignature(@QueryParameter boolean value, @QueryParameter boolean useBrowser) {
if (value) {
return FormValidation.ok();
} else if (useBrowser) {
return FormValidation.warningWithMarkup(Messages.DownloadSettings_disabling_signature_checks_for_in_browse());
} else {
return FormValidation.warningWithMarkup(Messages.DownloadSettings_disabling_signature_checks_is_not_recomm());
}
}

@Extension public static final class DailyCheck extends AsyncPeriodicWork {

public DailyCheck() {
Expand Down
Expand Up @@ -96,7 +96,7 @@ THE SOFTWARE.
</tr>
</table>
<div align="right" style="margin-top:1em">
<j:invokeStatic var="ds" className="jenkins.security.DownloadSettings" method="get"/>
<j:invokeStatic var="ds" className="jenkins.model.DownloadSettings" method="get"/>
<form method="post" action="${ds.useBrowser ? 'checkUpdates' : 'checkUpdatesServer'}">
${%lastUpdated(app.updateCenter.lastUpdatedString)}
<f:submit value="${%Check now}" />
Expand Down
Expand Up @@ -31,7 +31,7 @@ THE SOFTWARE.
-->
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<j:invokeStatic var="ds" className="jenkins.security.DownloadSettings" method="get"/>
<j:invokeStatic var="ds" className="jenkins.model.DownloadSettings" method="get"/>
<j:if test="${ds.useBrowser}">
<j:forEach var="site" items="${app.updateCenter.sites}">
<j:if test="${site.due or forcedUpdateCheck}">
Expand Down
@@ -0,0 +1,9 @@
package jenkins.security.DownloadSettings

def f = namespace(lib.FormTagLib)

f.section(title: _("Download Preferences")) {
f.entry(title: _("Use Browser"), field: "useBrowser") {
f.checkbox()
}
}

This file was deleted.

This file was deleted.

4 changes: 1 addition & 3 deletions core/src/main/resources/jenkins/security/Messages.properties
Expand Up @@ -22,6 +22,4 @@

ApiTokenProperty.DisplayName=API Token
ApiTokenProperty.ChangeToken.Success=<div>Updated</div>
DownloadSettings.disabling_signature_checks_for_in_browse=Disabling signature checks for in-browser downloads is <em>very dangerous</em> as unprivileged users could submit spoofed metadata!
DownloadSettings.disabling_signature_checks_is_not_recomm=Disabling signature checks is not recommended except as a temporary measure when upstream metadata is broken, such as due to expired certificates.
RekeySecretAdminMonitor.DisplayName=Re-keying
RekeySecretAdminMonitor.DisplayName=Re-keying
5 changes: 2 additions & 3 deletions test/src/test/java/hudson/model/DownloadServiceTest.java
Expand Up @@ -5,7 +5,6 @@
import java.net.URL;
import java.util.Set;
import java.util.TreeSet;
import jenkins.security.DownloadSettings;
import net.sf.json.JSONObject;
import org.jvnet.hudson.test.Bug;
import org.jvnet.hudson.test.HudsonTestCase;
Expand Down Expand Up @@ -41,13 +40,13 @@ public void testPost() throws Exception {
assertNull(job.getData());

// and now it should work
DownloadSettings.get().setCheckSignature(false);
DownloadService.signatureCheck = false;
try {
createWebClient().goTo("/self/testPost");
JSONObject d = job.getData();
assertEquals(hashCode(),d.getInt("hello"));
} finally {
DownloadSettings.get().setCheckSignature(true);
DownloadService.signatureCheck = true;
}

// TODO: test with a signature
Expand Down

0 comments on commit ca976da

Please sign in to comment.