Skip to content

Commit

Permalink
Remove FileOnMasterKeySource and SECURITY-1322 migration
Browse files Browse the repository at this point in the history
It has been 5 years since the security fix was introduced and the
migration code occureed.

Whilst an admin could have still (ab)used the CLI/REST to set a
FileOnMaster they could not do this from the UI creating a disparity.

This change removes the migration and the ability to use this via
CLI/REST
  • Loading branch information
jtnord committed Jun 21, 2024
1 parent 5e08c0a commit 079a4cb
Show file tree
Hide file tree
Showing 13 changed files with 3 additions and 442 deletions.
3 changes: 3 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@
</goals>
</execution>
</executions>
<configuration>
<compatibleSinceVersion>1344</compatibleSinceVersion>
</configuration>
</plugin>
<plugin>
<groupId>org.antlr</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,10 @@
import hudson.util.FormValidation;
import hudson.util.Secret;
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.ObjectStreamException;
import java.io.Serializable;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Paths;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
Expand All @@ -58,7 +54,6 @@
import java.util.logging.LogRecord;
import java.util.logging.Logger;

import jenkins.model.Jenkins;
import net.jcip.annotations.GuardedBy;
import org.apache.commons.fileupload.FileItem;
import org.apache.commons.lang.StringUtils;
Expand Down Expand Up @@ -337,72 +332,6 @@ protected static FormValidation validateCertificateKeystore(String type, byte[]
}
}

/**
* Let the user reference a file on the disk.
* @deprecated This approach has security vulnerabilities and should be migrated to {@link UploadedKeyStoreSource}
*/
@Deprecated
public static class FileOnMasterKeyStoreSource extends KeyStoreSource {

/**
* Our logger.
*/
private static final Logger LOGGER = Logger.getLogger(FileOnMasterKeyStoreSource.class.getName());

/**
* The path of the file on the controller.
*/
private final String keyStoreFile;

public FileOnMasterKeyStoreSource(String keyStoreFile) {
this.keyStoreFile = keyStoreFile;
}

/**
* {@inheritDoc}
*/
@NonNull
@Override
public byte[] getKeyStoreBytes() {
try {
return Files.readAllBytes(Paths.get(keyStoreFile));
} catch (IOException | InvalidPathException e) {
LOGGER.log(Level.WARNING, "Could not read private key file " + keyStoreFile, e);
return new byte[0];
}
}

/**
* {@inheritDoc}
*/
@Override
public long getKeyStoreLastModified() {
return new File(keyStoreFile).lastModified();
}

/**
* {@inheritDoc}
*/
@Override
public String toString() {
return "FileOnMasterKeyStoreSource{" +
"keyStoreFile='" + keyStoreFile + '\'' +
"}";
}

private Object readResolve() {
if (!Jenkins.get().hasPermission(Jenkins.RUN_SCRIPTS)) {
LOGGER.warning("SECURITY-1322: Permission failure migrating FileOnMasterKeyStoreSource to UploadedKeyStoreSource for a Certificate. An administrator may need to perform the migration.");
Jenkins.get().checkPermission(Jenkins.RUN_SCRIPTS);
}

LOGGER.log(Level.INFO, "SECURITY-1322: Migrating FileOnMasterKeyStoreSource to UploadedKeyStoreSource. The containing item may need to be saved to complete the migration.");
SecretBytes secretBytes = SecretBytes.fromBytes(getKeyStoreBytes());
return new UploadedKeyStoreSource(secretBytes);
}

}

/**
* Let the user reference an uploaded file.
*/
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading

0 comments on commit 079a4cb

Please sign in to comment.