Skip to content
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

SRI support #235

Merged
merged 6 commits into from
Jun 30, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ public void setType(final String type) {
this.setAttribute("type", type);
}

public String getIntegrity() {
return this.getAttribute("integrity");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: check indent

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, make the function private, unless public is necessary.

}

// TODO can go in Urls util class.
private boolean isWellFormedURL() {
final HTMLDocumentImpl doc = (HTMLDocumentImpl) this.getOwnerDocument();
Expand Down Expand Up @@ -319,7 +323,8 @@ private void processLink() {
if (uacontext.isExternalCSSEnabled()) {
try {
final String href = this.getHref();
final StyleSheet jSheet = CSSUtilities.jParse(this, href, doc, doc.getBaseURI(), false);
final String integrity = this.getIntegrity();
final StyleSheet jSheet = CSSUtilities.jParse(this, href, integrity, doc, doc.getBaseURI(), false);
if (this.styleSheet != null) {
this.styleSheet.setJStyleSheet(jSheet);
} else {
Expand All @@ -336,7 +341,7 @@ private void processLink() {
+ "] does not appear to be a valid URI.");
dispatchEvent("error");
} catch (final IOException err) {
this.warn("Unable to parse CSS. URI=[" + this.getHref() + "].", err);
this.warn("Exception while fetching CSS. URI=[" + this.getHref() + "].", err);
dispatchEvent("error");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ public void setType(final String type) {
this.setAttribute("type", type);
}

public String getIntegrity() {
return this.getAttribute("integrity");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: check indent

also check visibility (private / public)


private static final String[] jsTypes = {
"application/ecmascript",
"application/javascript",
Expand Down Expand Up @@ -176,7 +180,8 @@ protected final void processScript() {
// Code might have restrictions on accessing
// items from elsewhere.
try {
request.open("GET", scriptURI, false);
final String integrity = this.getIntegrity();
request.open("GET", scriptURI, false, integrity);
request.send(null, new Request(scriptURL, RequestKind.JavaScript));
} catch (final java.io.IOException thrown) {
logger.log(Level.WARNING, "processScript()", thrown);
Expand All @@ -185,9 +190,10 @@ protected final void processScript() {
return null;
});
final int status = request.getStatus();
if ((status != 200) && (status != 0)) {
if ((status != 200)) {
this.warn("Script at [" + scriptURI + "] failed to load; HTTP status: " + status + ".");
dispatchEvent("error");
doc.markJobsFinished(1, false);
return;
}
text = request.getResponseText();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public void open(final String method, final String url, final boolean asyncFlag,
throws java.io.IOException {
final String adjustedMethod = checkAndAdjustMethod(method);
try {
request.open(adjustedMethod, this.getFullURL(url), asyncFlag, userName, password);
request.open(adjustedMethod, this.getFullURL(url), asyncFlag, userName, password, null);
} catch (final MalformedURLException mfe) {
throw ScriptRuntime.typeError("url malformed");
}
Expand Down
15 changes: 10 additions & 5 deletions src/HTML_Renderer/org/lobobrowser/html/style/CSSUtilities.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ public static StyleSheet jParseStyleSheet(final org.w3c.dom.Node ownerNode, fina
return jParseCSS2(ownerNode, baseURI, stylesheetStr, bcontext);
}

public static StyleSheet jParse(final org.w3c.dom.Node ownerNode, final String href, final HTMLDocumentImpl doc, final String baseUri,
final boolean considerDoubleSlashComments) throws IOException {
public static StyleSheet jParse(final org.w3c.dom.Node ownerNode, final String href, final String integrity, final HTMLDocumentImpl doc, final String baseUri, final boolean considerDoubleSlashComments) throws IOException {
final UserAgentContext bcontext = doc.getUserAgentContext();
final NetworkRequest request = bcontext.createHttpRequest();
final URL baseURL = new URL(baseUri);
Expand All @@ -116,7 +115,7 @@ public static StyleSheet jParse(final org.w3c.dom.Node ownerNode, final String h
// Perform a synchronous request
final IOException ioException = SecurityUtil.doPrivileged(() -> {
try {
request.open("GET", cssURI, false);
request.open("GET", cssURI, false, integrity);
request.send(null, new Request(cssURL, RequestKind.CSS));
return null;
} catch (final java.io.IOException thrown) {
Expand All @@ -128,9 +127,15 @@ public static StyleSheet jParse(final org.w3c.dom.Node ownerNode, final String h
throw ioException;
}
final int status = request.getStatus();
if (status != 200 && status != 0) {
throw new IOException("Unable to parse CSS. URI=[" + cssURI + "]. Response status was " + status + ".");
if (status != 200) {
throw new IOException("Unable to fetch CSS. URI=[" + cssURI + "]. Response status was " + status + ".");
}
/*
if ((status != 200) && (status != 0)) {
logger.warning("Unable to parse CSS. URI=[" + cssURI + "]. Response status was " + status + ".");
return getEmptyStyleSheet();
}
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove the commented code


final String text = request.getResponseText();
if ((text != null) && !"".equals(text)) {
Expand Down
131 changes: 131 additions & 0 deletions src/Platform_Core/org/lobobrowser/context/AlgorithmDigest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package org.lobobrowser.context;

import java.security.MessageDigest;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Base64;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

public class AlgorithmDigest implements Comparable<AlgorithmDigest> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check visibility of class: package private / public

Also make it final.


private final String algorithm;
private final String digest;
private final Integer strength;
private final static Map<String, Integer> str = createMap();

public AlgorithmDigest(String algorithm, String digest, Integer strength) {
super();
this.algorithm = algorithm;
this.digest = digest;
this.strength = strength;
}

public static boolean validate(final byte[] input, final String integrity) {

final List<AlgorithmDigest> algDigests = parseMetadata(integrity);
System.out.println("Strongest algorithms: " + algDigests);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nuke all debug prints in this file


System.out.println("The value of the integrity attribute is: " + integrity);

if (algDigests == null)
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code style: surround with braces.

For a classic example of the problem that braces protect against, see: https://nakedsecurity.sophos.com/2014/02/24/anatomy-of-a-goto-fail-apples-ssl-bug-explained-plus-an-unofficial-patch/


final boolean matchFound = algDigests.stream().anyMatch((algDigest) -> {
final String encodedResult = algDigest.getHash(input);
return encodedResult.equals(algDigest.digest);
});

return matchFound;

}

public static List<AlgorithmDigest> parseMetadata(final String integrity) {

if (integrity == null || integrity.length() == 0) {
return null;
}

System.out.println(integrity);
final String[] tokens = integrity.split("\\s+");

final List<AlgorithmDigest> hashes = getHashes(tokens);

if (hashes.isEmpty())
return null;

return strongestAlgDigests(hashes);

}

private static List<AlgorithmDigest> getHashes(final String[] tokens) {
final List<AlgorithmDigest> hashes = new ArrayList<AlgorithmDigest>();

for (int i = 0; i < tokens.length; i++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try converting to enhanced for loop. In eclipse Ctrl 1 should suggest it automatically.

// TODO: check token syntax against
// https://www.w3.org/TR/SRI/#the-integrity-attribute
final int hyphen = tokens[i].indexOf("-");
if (hyphen < 0) {
continue;
}

final String alg = tokens[i].substring(0, hyphen);
if (str.containsKey(alg)) {
final AlgorithmDigest ag = new AlgorithmDigest(alg, tokens[i].substring(hyphen + 1), str.get(alg));
hashes.add(ag);

}

}
return hashes;
}

public static List<AlgorithmDigest> strongestAlgDigests(final List<AlgorithmDigest> hashes) {
Collections.sort(hashes, (a, b) -> a.compareTo(b));

System.out.println("Hashes: " + Arrays.toString(hashes.toArray()));
final Integer strongest = hashes.get(0).strength;

final List<AlgorithmDigest> result = hashes.stream().filter((h) -> h.strength == strongest)
.collect(Collectors.toList());

System.out.println("result: " + Arrays.toString(result.toArray()));
return result;
}

public String getHash(final byte[] input) {
final String alg = algorithm.substring(0, 3) + "-" + algorithm.substring(3);
System.out.println("Algorithm used is: " + alg);
try {
final MessageDigest md = MessageDigest.getInstance(alg); // returns an object that implements alg
md.update(input); // updates digest using input array of bytes
final byte[] digestedBytes = md.digest(); // completes hash, returns array of bytes
return Base64.getEncoder().encodeToString(digestedBytes); // converts array of bytes into base64-encoded string
} catch (final Exception e) {
System.out.println("Exception: " + e.getMessage());
}
return "";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to return null on failure, instead of empty string.

}

private static Map<String, Integer> createMap() {
final Map<String, Integer> stren = new HashMap<String, Integer>();
stren.put("sha256", 1);
stren.put("sha384", 2);
stren.put("sha512", 3);
return stren;
}

@Override
public int compareTo(final AlgorithmDigest o) {
return o.strength - strength;
}

@Override
public String toString() {
return "AlgorithmDigest [algorithm=" + algorithm + ", digest=" + digest + ", strength=" + strength + "]";
}

}
38 changes: 30 additions & 8 deletions src/Platform_Core/org/lobobrowser/context/NetworkRequestImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.logging.Level;
import java.util.logging.Logger;


import org.eclipse.jdt.annotation.NonNull;
import org.lobobrowser.clientlet.ClientletAccess;
import org.lobobrowser.clientlet.ClientletContext;
Expand Down Expand Up @@ -155,37 +156,39 @@ public String getResponseHeader(final String headerName) {
}

public void open(final String method, final String url) throws IOException {
this.open(method, url, true);
this.open(method, url, true, null);
}

public void open(final String method, final @NonNull URL url) {
this.open(method, url, true, null, null);
this.open(method, url, true, null, null, null);
}

public void open(final String method, final @NonNull URL url, final boolean asyncFlag) {
this.open(method, url, asyncFlag, null, null);
this.open(method, url, asyncFlag, null, null, null);
}

public void open(final String method, final String url, final boolean asyncFlag) throws IOException {
public void open(final String method, final String url, final boolean asyncFlag, final String integrity) throws IOException {
final URL urlObj = Urls.createURL(null, url);
this.open(method, urlObj, asyncFlag, null, null);
this.open(method, urlObj, asyncFlag, null, null, integrity);
}

public void open(final String method, final @NonNull URL url, final boolean asyncFlag, final String userName) {
this.open(method, url, asyncFlag, userName, null);
this.open(method, url, asyncFlag, userName, null, null);
}

private boolean isAsynchronous = false;
private String requestMethod;
private URL requestURL;
private String integrity;

// private String requestUserName;
// private String requestPassword;

public void open(final String method, final @NonNull URL url, final boolean asyncFlag, final String userName, final String password) {
public void open(final String method, final @NonNull URL url, final boolean asyncFlag, final String userName, final String password, final String integrity) {
this.isAsynchronous = asyncFlag;
this.requestMethod = method;
this.requestURL = url;
this.integrity = integrity;
// this.requestUserName = userName;
// this.requestPassword = password;
this.changeReadyState(NetworkRequest.STATE_LOADING);
Expand Down Expand Up @@ -235,10 +238,12 @@ private void changeReadyState(final int newState) {
}

private void setResponse(final ClientletResponse response) {

final Runnable runnable = () -> {
if (response.isFromCache()) {
final Object cachedResponse = response.getTransientCachedObject();
if (cachedResponse instanceof CacheableResponse) {
System.out.println("Cached response for " + response.getResponseURL());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove debug print

// It can be of a different type.
final CacheableResponse cr = (CacheableResponse) cachedResponse;
this.changeReadyState(NetworkRequest.STATE_LOADING);
Expand All @@ -251,7 +256,7 @@ private void setResponse(final ClientletResponse response) {
}
try {
this.changeReadyState(NetworkRequest.STATE_LOADING);
final LocalResponse newResponse = new LocalResponse(response);
LocalResponse newResponse = new LocalResponse(response); //was final before
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it can be converted back to final.

this.localResponse = newResponse;
this.changeReadyState(NetworkRequest.STATE_LOADED);
final int cl = response.getContentLength();
Expand Down Expand Up @@ -284,6 +289,8 @@ private void setResponse(final ClientletResponse response) {
}
}
newResponse.writeBytes(buffer, 0, numRead);


if (firstTime) {
firstTime = false;
this.changeReadyState(NetworkRequest.STATE_INTERACTIVE);
Expand All @@ -294,7 +301,18 @@ private void setResponse(final ClientletResponse response) {
threadContext.setProgressEvent(prevProgress);
}
}

// here goes integrity
final boolean valid = AlgorithmDigest.validate(newResponse.getBuffer().toByteArray(), integrity);

if (valid == false) {
this.localResponse = null;
}

//TODO: CORS support

newResponse.setComplete(true);

// The following should return non-null if the response is complete.
final CacheableResponse cacheable = newResponse.getCacheableResponse();
if (cacheable != null) {
Expand Down Expand Up @@ -520,6 +538,10 @@ public void writeBytes(final byte[] bytes, final int offset, final int length) t
out.write(bytes, offset, length);
}

private ByteArrayOutputStream getBuffer() {
return this.cacheable.buffer;
}

public void setComplete(final boolean complete) {
this.cacheable.complete = complete;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ public int getResponseCode() throws IOException {
if (this.connection instanceof HttpURLConnection) {
return ((HttpURLConnection) this.connection).getResponseCode();
} else {
return 0;
return 200;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public interface NetworkRequest {
* @param asyncFlag
* Whether the request should be asynchronous.
*/
public void open(String method, String url, boolean asyncFlag) throws java.io.IOException;
public void open(String method, String url, boolean asyncFlag, String integrity) throws java.io.IOException;

/**
* Opens a request.
Expand All @@ -183,8 +183,8 @@ public interface NetworkRequest {
* The destination URL.
* @param asyncFlag
* Whether the request should be asynchronous.
* @param userName
* The HTTP authentication user name.
* @param integrity
* Integrity value to be checked.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also looks like a spurious change.

*/
public void open(String method, @NonNull URL url, boolean asyncFlag, String userName) throws java.io.IOException;

Expand All @@ -202,7 +202,7 @@ public interface NetworkRequest {
* @param password
* The HTTP authentication password.
*/
public void open(String method, @NonNull URL url, boolean asyncFlag, String userName, String password) throws java.io.IOException;
public void open(String method, @NonNull URL url, boolean asyncFlag, String userName, String password, String integrity) throws java.io.IOException;

/**
* Sends POST content if any.
Expand Down