Skip to content

System properties to harden file reading/writing, http behavior #1213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Nov 25, 2023
8 changes: 0 additions & 8 deletions js/qz-tray.js
Original file line number Diff line number Diff line change
Expand Up @@ -1074,14 +1074,6 @@ var qz = (function() {
if (typeof newPrinter === 'string') {
newPrinter = { name: newPrinter };
}

if(newPrinter && newPrinter.file) {
// TODO: Warn for UNC paths too https://github.com/qzind/tray/issues/730
if(newPrinter.file.indexOf("\\\\") != 0) {
_qz.log.warn("Printing to file is deprecated. See https://github.com/qzind/tray/issues/730");
}
}

this.printer = newPrinter;
};

Expand Down
36 changes: 1 addition & 35 deletions sample.html
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ <h3 class="panel-title">Printer</h3>
<div class="form-group">
<div class="btn-group" role="group">
<button type="button" class="btn btn-default btn-sm" onclick="setPrinter($('#printerSearch').val());">Set To Search</button>
<button type="button" class="btn btn-default btn-sm" data-toggle="modal" data-target="#askFileModal">Set To File</button>
<button type="button" class="btn btn-default btn-sm" data-toggle="modal" data-target="#askHostModal">Set To Host</button>
</div>
<button type="button" class="btn btn-warning btn-sm" onclick="clearQueue($('#printerSearch').val());">Clear Queue</button>
Expand Down Expand Up @@ -1152,7 +1151,7 @@ <h3>Printer Status</h3>
<div class="col-md-6 pull-right">
<div class="form-group form-inline">
<label class="tip" for="maxJobData" data-toggle="tooltip" title="Maximum size in bytes of spool file content to return (Windows only)">
Raw Job Data Size
Raw Job Data Size
</label>
<input type="number" id="maxJobData" class="pull-right"/>
</div>
Expand Down Expand Up @@ -1354,27 +1353,6 @@ <h4 class="panel-title">Options</h4>
</div>
</div>


<div class="modal fade" id="askFileModal" role="dialog">
<div class="modal-dialog modal-sm" role="document">
<div class="modal-content">
<div class="modal-body">
<div class="form-group">
<label for="askFile">File:</label>
<input type="text" id="askFile" class="form-control" value="C:\tmp\example-file.txt" />
<hr />
<p><span class="text-danger" style="font-weight:bold;"><span class="fa fa-warning"></span> WARNING:</span> This feature has been deprecated. Please configure a local raw <code>FILE:</code> printer, or use <code>File IO</code></a> instead. For more
information please see <a href="https://github.com/qzind/tray/issues/730">issue&nbsp;<code>#730</code>.</a></p>
</div>
</div>
<div class="modal-footer">
<button type="button" class="btn btn-default" data-dismiss="modal">Cancel</button>
<button type="button" class="btn btn-primary" onclick="setPrintFile();">Set</button>
</div>
</div>
</div>
</div>

<div class="modal fade" id="askHostModal" role="dialog">
<div class="modal-dialog modal-sm" role="document">
<div class="modal-content">
Expand Down Expand Up @@ -2982,13 +2960,6 @@ <h4 class="panel-title">Options</h4>
}).catch(displayError);
}

$("#askFileModal").on("shown.bs.modal", function() {
$("#askFile").focus().select();
});
$("#askHostModal").on("shown.bs.modal", function() {
$("#askHost").focus().select();
});

//make dirty when changed
$("input").add("select").on('change', function() {
$(this).addClass("dirty");
Expand Down Expand Up @@ -3375,11 +3346,6 @@ <h4 class="panel-title">Options</h4>
return options;
}

function setPrintFile() {
setPrinter({ file: $("#askFile").val() });
$("#askFileModal").modal('hide');
}

function setPrintHost() {
setPrinter({ host: $("#askHost").val(), port: $("#askPort").val() });
$("#askHostModal").modal('hide');
Expand Down
2 changes: 1 addition & 1 deletion src/qz/auth/CRL.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public static CRL getInstance() {
public void run() {
log.info("Loading CRL from {}", CRL_URL);

try(BufferedReader br = new BufferedReader(new InputStreamReader(ConnectionUtilities.getInputStream(CRL_URL)))) {
try(BufferedReader br = new BufferedReader(new InputStreamReader(ConnectionUtilities.getInputStream(CRL_URL, false)))) {
String line;
while((line = br.readLine()) != null) {
//Ignore empty and commented lines
Expand Down
13 changes: 7 additions & 6 deletions src/qz/common/TrayManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -534,9 +534,10 @@ private void blackList(Certificate cert) {
}
}

public void setServer(Server server, int insecurePortIndex) {
public void setServer(Server server, int insecurePortInUse, int securePortInUse) {
if (server != null && server.getConnectors().length > 0) {
singleInstanceCheck(PrintSocketServer.INSECURE_PORTS, insecurePortIndex);
singleInstanceCheck(PrintSocketServer.INSECURE_PORTS, insecurePortInUse, false);
singleInstanceCheck(PrintSocketServer.SECURE_PORTS, securePortInUse, true);

displayInfoMessage("Server started on port(s) " + PrintSocketServer.getPorts(server));

Expand Down Expand Up @@ -632,10 +633,10 @@ private void displayMessage(final String caption, final String text, final TrayI
}
}

public void singleInstanceCheck(java.util.List<Integer> insecurePorts, Integer insecurePortIndex) {
for(int port : insecurePorts) {
if (port != insecurePorts.get(insecurePortIndex)) {
new SingleInstanceChecker(this, port);
public void singleInstanceCheck(java.util.List<Integer> ports, Integer portInUse, boolean usingSecure) {
for(int port : ports) {
if (portInUse == -1 || port != ports.get(portInUse)) {
new SingleInstanceChecker(this, port, usingSecure);
}
}
}
Expand Down
6 changes: 2 additions & 4 deletions src/qz/installer/certificate/CertificateManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.logging.log4j.Logger;
import qz.common.Constants;
import qz.installer.Installer;
import qz.utils.ArgValue;
import qz.utils.FileUtilities;
import qz.utils.MacUtilities;
import qz.utils.SystemUtilities;
Expand Down Expand Up @@ -60,10 +61,7 @@ public class CertificateManager {

public static String DEFAULT_KEYSTORE_FORMAT = "PKCS12";
public static String DEFAULT_KEYSTORE_EXTENSION = ".p12";

public static String DEFAULT_CERTIFICATE_EXTENSION = ".crt";

private static String DEFAULT_HOST_SCOPE = "0.0.0.0";
private static int DEFAULT_PASSWORD_BITS = 100;

private boolean needsInstall;
Expand Down Expand Up @@ -331,7 +329,7 @@ public Properties writeKeystore(Properties props, KeyPairWrapper.Type type) thro
props.putIfAbsent(String.format("%s.alias", keyPair.propsPrefix()), keyPair.getAlias());

if (keyPair.getType() == SSL) {
props.putIfAbsent(String.format("%s.host", keyPair.propsPrefix()), DEFAULT_HOST_SCOPE);
props.putIfAbsent(String.format("%s.host", keyPair.propsPrefix()), ArgValue.SECURITY_WSS_HOST.getDefaultVal());
}


Expand Down
2 changes: 1 addition & 1 deletion src/qz/printer/action/PrintImage.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void parseData(JSONArray printData, PrintOptions options) throws JSONExce
case PLAIN:
// There's really no such thing as a 'PLAIN' image, assume it's a URL
case FILE:
bi = ImageIO.read(ConnectionUtilities.getInputStream(data.getString("data")));
bi = ImageIO.read(ConnectionUtilities.getInputStream(data.getString("data"), true));
break;
default:
bi = ImageIO.read(new ByteArrayInputStream(flavor.read(data.getString("data"))));
Expand Down
2 changes: 1 addition & 1 deletion src/qz/printer/action/PrintPDF.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public void parseData(JSONArray printData, PrintOptions options) throws JSONExce
case PLAIN:
// There's really no such thing as a 'PLAIN' PDF, assume it's a URL
case FILE:
doc = PDDocument.load(ConnectionUtilities.getInputStream(data.getString("data")));
doc = PDDocument.load(ConnectionUtilities.getInputStream(data.getString("data"), true));
break;
default:
doc = PDDocument.load(new ByteArrayInputStream(flavor.read(data.getString("data"))));
Expand Down
19 changes: 13 additions & 6 deletions src/qz/printer/action/PrintRaw.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

import com.ibm.icu.text.ArabicShapingException;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.ssl.Base64;
import org.apache.pdfbox.pdmodel.PDDocument;
import org.apache.pdfbox.pdmodel.common.PDRectangle;
import org.apache.pdfbox.rendering.PDFRenderer;
Expand Down Expand Up @@ -175,7 +174,7 @@ private ImageWrapper getImageWrapper(String data, JSONObject opt, PrintingUtilit
case PLAIN:
// There's really no such thing as a 'PLAIN' image, assume it's a URL
case FILE:
bi = ImageIO.read(ConnectionUtilities.getInputStream(data));
bi = ImageIO.read(ConnectionUtilities.getInputStream(data, true));
break;
default:
bi = ImageIO.read(new ByteArrayInputStream(seekConversion(flavor.read(data), rawOpts)));
Expand All @@ -191,7 +190,7 @@ private ImageWrapper getPdfWrapper(String data, JSONObject opt, PrintingUtilitie
case PLAIN:
// There's really no such thing as a 'PLAIN' PDF, assume it's a URL
case FILE:
doc = PDDocument.load(ConnectionUtilities.getInputStream(data));
doc = PDDocument.load(ConnectionUtilities.getInputStream(data, true));
break;
default:
doc = PDDocument.load(new ByteArrayInputStream(seekConversion(flavor.read(data), rawOpts)));
Expand Down Expand Up @@ -329,7 +328,7 @@ public void print(PrintOutput output, PrintOptions options) throws PrintExceptio
if (output.isSetHost()) {
printToHost(output.getHost(), output.getPort(), bab.getByteArray());
} else if (output.isSetFile()) {
printToFile(output.getFile(), bab.getByteArray());
printToFile(output.getFile(), bab.getByteArray(), true);
} else {
if (rawOpts.isForceRaw()) {
if(tempFiles == null) {
Expand All @@ -339,7 +338,7 @@ public void print(PrintOutput output, PrintOptions options) throws PrintExceptio
if(tempFiles.size() <= j) {
tempFile = File.createTempFile("qz_raw_", null);
tempFiles.add(j, tempFile);
printToFile(tempFile, bab.getByteArray());
printToFile(tempFile, bab.getByteArray(), false);
} else {
tempFile = tempFiles.get(j);
}
Expand Down Expand Up @@ -401,7 +400,15 @@ private void printToHost(String host, int port, byte[] cmds) throws IOException
*
* @param file File to be written
*/
private void printToFile(File file, byte[] cmds) throws IOException {
private void printToFile(File file, byte[] cmds, boolean locationRestricted) throws IOException {
if(file == null) throw new IOException("No file specified");

if(locationRestricted && !PrefsSearch.getBoolean(ArgValue.SECURITY_PRINT_TOFILE)) {
log.error("Printing to file '{}' is not permitted. Configure property '{}' to modify this behavior.",
file, ArgValue.SECURITY_PRINT_TOFILE.getMatch());
throw new IOException(String.format("Printing to file '%s' is not permitted", file));
}

log.debug("Printing to file: {}", file.getName());

//throws any exception and auto-closes stream
Expand Down
10 changes: 10 additions & 0 deletions src/qz/utils/ArgValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ public enum ArgValue {
"security.file.enabled"),
SECURITY_FILE_STRICT(PREFERENCES, "Enable/disable signing requirements for File Communications features", null, true,
"security.file.strict"),
SECURITY_DATA_PROTOCOLS(PREFERENCES, "URL protocols allowed for print, serial, hid, etc", null, "http,https",
"security.data.protocols"),
SECURITY_PRINT_TOFILE(PREFERENCES, "Enable/disable printing directly to file paths", null, false,
"security.print.tofile"),
SECURITY_WSS_SNISTRICT(PREFERENCES, "Enables strict http/websocket SNI checks", null, false,
"security.wss.snistrict"),
SECURITY_WSS_HTTPSONLY(PREFERENCES, "Disables insecure http/websocket ports (e.g. '8182')", null, false,
"security.wss.httpsonly"),
SECURITY_WSS_HOST(PREFERENCES, "Influences which physical adapter to bind to by setting the host parameter for http/websocket listening", null, "0.0.0.0",
"security.wss.host"),
LOG_DISABLE(PREFERENCES, "Disable/enable logging features", null, false,
"log.disable"),
LOG_ROTATE(PREFERENCES, "Number of log files to retain when the size fills up", null, 5,
Expand Down
43 changes: 41 additions & 2 deletions src/qz/utils/ConnectionUtilities.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@
import java.awt.*;
import java.io.IOException;
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLConnection;
import java.nio.file.Paths;
import java.security.cert.Certificate;
import java.security.cert.X509Certificate;
import java.util.HashMap;
Expand All @@ -39,9 +42,17 @@ public final class ConnectionUtilities {
*
* @param urlString an absolute URL giving location of resource to read.
*/
public static InputStream getInputStream(String urlString) throws IOException {
public static InputStream getInputStream(String urlString, boolean protocolRestricted) throws IOException {
try {
URLConnection urlConn = new URL(urlString).openConnection();
URL url = new URL(urlString);
if(protocolRestricted) {
String allowed = PrefsSearch.getString(ArgValue.SECURITY_DATA_PROTOCOLS);
if(!isAllowed(allowed, url)) {
log.error("URL '{}' is not a valid http or https location. Configure property '{}' to modify this behavior.", url, ArgValue.SECURITY_DATA_PROTOCOLS.getMatch());
throw new IOException(String.format("URL '%s' is not a valid [%s] location", url, allowed));
}
}
URLConnection urlConn = url.openConnection();
for( String key : getRequestProperties().keySet()) {
urlConn.setRequestProperty(key, requestProps.get(key));
}
Expand All @@ -54,6 +65,34 @@ public static InputStream getInputStream(String urlString) throws IOException {
}
}

private static boolean isAllowed(String allowed, URL url) {
if(url == null) return false;
String urlProtocol = url.getProtocol();
if(urlProtocol == null || urlProtocol.trim().isEmpty()) return false;
allowed = ArgValue.SECURITY_DATA_PROTOCOLS.getDefaultVal() +
(allowed == null || allowed.trim().isEmpty() ? "" : "," + allowed);
String[] protocols = allowed.split(",");
// Loop over http, https, etc
for(String protocol : protocols) {
if(urlProtocol.trim().equalsIgnoreCase(protocol.trim())) {
return true;
}
}
// Allow exception for file: demo/assets
if(urlProtocol.trim().toLowerCase(Locale.ENGLISH).equals("file")) {
try {
// Sanitize manipulative URLs
url = Paths.get(url.toURI()).normalize().toUri().toURL();
if (url.getPath().matches(".*/demo/assets/.*|.*/tray/assets/.*")) {
log.warn("Allowing printing from restricted protocol '{}:' for demo asset '{}'", urlProtocol, url);
return true;
}
}
catch(URISyntaxException | MalformedURLException ignore) {}
}
return false;
}

/**
* A blind SSL trust manager, for debugging SSL issues
*/
Expand Down
2 changes: 1 addition & 1 deletion src/qz/utils/FileUtilities.java
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ public static String readLocalFile(Path path) throws IOException {
}

public static byte[] readRawFile(String url) throws IOException {
return readFile(new DataInputStream(ConnectionUtilities.getInputStream(url)));
return readFile(new DataInputStream(ConnectionUtilities.getInputStream(url, true)));
}

private static byte[] readFile(DataInputStream in) throws IOException {
Expand Down
Loading