diff --git a/dspace-api/src/main/java/org/dspace/content/crosswalk/OREIngestionCrosswalk.java b/dspace-api/src/main/java/org/dspace/content/crosswalk/OREIngestionCrosswalk.java index 9e890a6046fa..d997ddf932d5 100644 --- a/dspace-api/src/main/java/org/dspace/content/crosswalk/OREIngestionCrosswalk.java +++ b/dspace-api/src/main/java/org/dspace/content/crosswalk/OREIngestionCrosswalk.java @@ -7,23 +7,17 @@ */ package org.dspace.content.crosswalk; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.net.ConnectException; -import java.net.URI; -import java.net.URISyntaxException; -import java.net.URL; import java.sql.SQLException; import java.text.NumberFormat; import java.util.Arrays; import java.util.Date; import java.util.HashSet; import java.util.List; -import java.util.Locale; -import java.util.Objects; import java.util.Set; +import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.logging.log4j.Logger; import org.dspace.authorize.AuthorizeException; import org.dspace.content.Bitstream; @@ -38,6 +32,12 @@ import org.dspace.content.service.ItemService; import org.dspace.core.Constants; import org.dspace.core.Context; +import org.dspace.harvest.ore.HarvestPolicyAware; +import org.dspace.harvest.ore.OreEgressPolicy; +import org.dspace.harvest.ore.OreResourceRejectedException; +import org.dspace.harvest.ore.OreUrlValidator; +import org.dspace.harvest.ore.RejectionReason; +import org.dspace.harvest.ore.SafeResourceFetcher; import org.dspace.services.ConfigurationService; import org.dspace.services.factory.DSpaceServicesFactory; import org.jdom2.Attribute; @@ -56,7 +56,7 @@ * @author Alexey Maslov */ public class OREIngestionCrosswalk - implements IngestionCrosswalk { + implements IngestionCrosswalk, HarvestPolicyAware { /** * log4j category */ @@ -84,6 +84,25 @@ public class OREIngestionCrosswalk protected ItemService itemService = ContentServiceFactory.getInstance().getItemService(); protected ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); + private final SafeResourceFetcher resourceFetcher = new SafeResourceFetcher(); + private OreEgressPolicy oreEgressPolicy; + + @Override + public void setOreEgressPolicy(OreEgressPolicy policy) { + this.oreEgressPolicy = policy; + } + + /** + * The packagers and the XSLT CLI reach this crosswalk with no harvest context, so they get the + * fail-closed policy rather than no policy at all. + */ + private OreEgressPolicy egressPolicy() { + if (oreEgressPolicy == null) { + oreEgressPolicy = OreEgressPolicy.strictest(configurationService); + } + return oreEgressPolicy; + } + @Override public void ingest(Context context, DSpaceObject dso, List metadata, boolean createMissingMetadataFields) @@ -135,6 +154,23 @@ public void ingest(Context context, DSpaceObject dso, Element root, boolean crea Filters.attribute(), null, ATOM_NS); entryId = xpathAltHref.evaluateFirst(doc).getValue(); + // Validate every aggregated resource URL up front, before any bitstream is created, so a URL rejected + // by the egress policy cannot leave the bitstreams of earlier resources orphaned in the assetstore + // when the record is rolled back. The fetch below still re-validates each URL (and every redirect hop) + // to guard against TOCTOU. + OreUrlValidator preValidator = new OreUrlValidator(); + for (Element resource : aggregatedResources) { + String href = resource.getAttributeValue("href"); + if (href == null) { + throw new CrosswalkException("Entry did not contain link to resource: " + entryId); + } + OreUrlValidator.Decision decision = + preValidator.validate(OreUrlValidator.parse(encodeForURL(href)), egressPolicy()); + if (!decision.isAllowed()) { + throw new OreResourceRejectedException(decision.getReason(), href, decision.getDetail()); + } + } + // Next for each resource, create a bitstream NumberFormat nf = NumberFormat.getInstance(); nf.setGroupingUsed(false); @@ -174,49 +210,36 @@ public void ingest(Context context, DSpaceObject dso, Element root, boolean crea targetBundle = targetBundles.get(0); } - URL ARurl = null; - InputStream in = null; - if (href != null) { - try { - // Make sure the url string escapes all the oddball characters - String processedURL = encodeForURL(href); - if (validResourceUri(entryId, processedURL)) { - // Generate a request for the aggregated resource - ARurl = new URL(processedURL); - in = ARurl.openStream(); - } else { - throw new FileNotFoundException("Failed to validate " + processedURL); - } - } catch (FileNotFoundException fe) { - log.error("The provided URI failed to return a resource: " + href); - } catch (ConnectException fe) { - log.error("The provided URI was invalid: " + href); - } - } else { - throw new CrosswalkException("Entry did not contain link to resource: " + entryId); + // Make sure the url string escapes all the oddball characters + String processedURL = encodeForURL(href); + Bitstream newBitstream; + // try-with-resources so the fetched stream - and the underlying HTTP connection/client it holds - + // is always closed, including when bitstream creation fails part way through. + // The trust anchor of the egress policy is the collection's oai_source, not entryId: entryId is + // read from this remote document and is therefore attacker-controlled. + try (InputStream in = resourceFetcher.fetch(OreUrlValidator.parse(processedURL), egressPolicy())) { + newBitstream = bitstreamService.create(context, targetBundle, in); + } catch (IOException ioe) { + // a transport failure - or the size cap, which the bitstore surfaces here wrapped in an + // IOException - has to drop this record only; escaping would stop the whole collection and, + // after removeAllBundles, commit the item without its files + throw transferFailed(href, ioe); } - // ingest and update - if (in != null) { - Bitstream newBitstream = bitstreamService.create(context, targetBundle, in); - - String bsName = resource.getAttributeValue("title"); - newBitstream.setName(context, bsName); + String bsName = resource.getAttributeValue("title"); + newBitstream.setName(context, bsName); - // Identify the format - String mimeString = resource.getAttributeValue("type"); - BitstreamFormat bsFormat = bitstreamFormatService.findByMIMEType(context, mimeString); - if (bsFormat == null) { - bsFormat = bitstreamFormatService.guessFormat(context, newBitstream); - } - newBitstream.setFormat(context, bsFormat); - bitstreamService.update(context, newBitstream); - - bundleService.addBitstream(context, targetBundle, newBitstream); - bundleService.update(context, targetBundle); - } else { - throw new CrosswalkException("Could not retrieve bitstream: " + entryId); + // Identify the format + String mimeString = resource.getAttributeValue("type"); + BitstreamFormat bsFormat = bitstreamFormatService.findByMIMEType(context, mimeString); + if (bsFormat == null) { + bsFormat = bitstreamFormatService.guessFormat(context, newBitstream); } + newBitstream.setFormat(context, bsFormat); + bitstreamService.update(context, newBitstream); + + bundleService.addBitstream(context, targetBundle, newBitstream); + bundleService.update(context, targetBundle); } log.info( @@ -263,60 +286,19 @@ private String encodeForURL(String sourceString) { } /** - * Validate a resource URI against the host and scheme of the remote OAI endpoint, or a configured - * list of allowed prefixes. - * This still implicitly "trusts" the remote OAI server, but will reject resource URIs with a totally - * different hostname to avoid downloading malicious resources from a compromised endpoint. - * Even if the URL prefix validation is disabled, schemes will still be enforced to http(s) so file:/// and - * other unwanted schemes cannot be used - * @param entryUrl the entryId of the parent ORE resource - * @param resourceUrl the resource URL of the aggregated ORE resource - * @return result of the validation + * Turn an I/O failure while fetching or storing a file into a rejection of this one record. + * + * @param href the resource that could not be transferred + * @param cause the failure, kept so the real reason still reaches the log + * @return the rejection to throw */ - private boolean validResourceUri(String entryUrl, String resourceUrl) { - try { - Set allowedSchemes = Set.of("http", "https"); - URI entryUri = new URI(entryUrl).normalize(); - URI resourceUri = new URI(resourceUrl).normalize(); - String scheme = resourceUri.getScheme(); - - if (scheme == null || - !allowedSchemes.contains(scheme.toLowerCase(Locale.ROOT))) { - log.warn("Illegal scheme requested for ORE resource: {}", resourceUri); - return false; - } - - if (configurationService.getBooleanProperty("oai.harvester.ore.file.validateUrlPrefix", false)) { - for (String allowedPrefix : configurationService - .getArrayProperty("oai.harvester.ore.file.allowedUrlPrefix")) { - URI allowedUri = new URI(allowedPrefix).normalize(); - // Return true on the first allowed prefix match - if (Objects.equals(resourceUri.getScheme(), allowedUri.getScheme()) - && Objects.equals(resourceUri.getHost().toLowerCase(Locale.ROOT), - allowedUri.getHost().toLowerCase(Locale.ROOT))) { - return true; - } - } - - // If no allowed prefixes were matched, we require scheme + host to match the remote OAI server - if (!Objects.equals(entryUri.getScheme(), resourceUri.getScheme())) { - log.warn("Illegal scheme requested for ORE resource: {}", resourceUri); - return false; - } - if (!Objects.equals( - entryUri.getHost().toLowerCase(Locale.ROOT), - resourceUri.getHost().toLowerCase(Locale.ROOT))) { - log.warn("Illegal host requested for ORE resource: {}", resourceUri); - return false; - } - } - - return true; - - } catch (URISyntaxException e) { - log.warn("Could not validate ORE resource URI: {}", resourceUrl); - return false; - } + private OreResourceRejectedException transferFailed(String href, IOException cause) { + // the cap is enforced while the body streams, so it arrives wrapped rather than as itself + boolean tooLarge = + ExceptionUtils.indexOfType(cause, SafeResourceFetcher.ResponseTooLargeException.class) >= 0; + return new OreResourceRejectedException( + tooLarge ? RejectionReason.RESPONSE_TOO_LARGE : RejectionReason.FETCH_FAILED, href, + tooLarge ? "size cap exceeded" : "transfer failed", cause); } } diff --git a/dspace-api/src/main/java/org/dspace/harvest/OAIHarvester.java b/dspace-api/src/main/java/org/dspace/harvest/OAIHarvester.java index 1b8d0002ce2c..5945fd307927 100644 --- a/dspace-api/src/main/java/org/dspace/harvest/OAIHarvester.java +++ b/dspace-api/src/main/java/org/dspace/harvest/OAIHarvester.java @@ -13,6 +13,8 @@ import java.io.PrintWriter; import java.io.StringWriter; import java.net.ConnectException; +import java.net.URI; +import java.net.URISyntaxException; import java.sql.SQLException; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -64,6 +66,10 @@ import org.dspace.handle.factory.HandleServiceFactory; import org.dspace.handle.service.HandleService; import org.dspace.harvest.factory.HarvestServiceFactory; +import org.dspace.harvest.ore.HarvestPolicyAware; +import org.dspace.harvest.ore.OreEgressPolicy; +import org.dspace.harvest.ore.OreResourceRejectedException; +import org.dspace.harvest.ore.RejectionReason; import org.dspace.harvest.service.HarvestedCollectionService; import org.dspace.harvest.service.HarvestedItemService; import org.dspace.services.ConfigurationService; @@ -121,6 +127,12 @@ public class OAIHarvester { Collection targetCollection; HarvestedCollection harvestRow; + // Number of records dropped because the ORE egress policy refused one of their files + private int rejectedRecords; + + // Number of records dropped because one of their files could not be downloaded at all + private int failedRecords; + // our context Context ourContext; @@ -454,12 +466,33 @@ public void runHarvest() throws SQLException, IOException, AuthorizeException { Date finishTime = new Date(); long timeTaken = finishTime.getTime() - startTime.getTime(); harvestRow.setHarvestStartTime(startTime); - harvestRow.setHarvestMessage("Harvest from " + oaiSource + " successful"); - harvestRow.setHarvestStatus(HarvestedCollection.STATUS_READY); - harvestRow.setLastHarvested(startTime); + int skippedRecords = rejectedRecords + failedRecords; + if (skippedRecords > 0) { + // deliberately generic: the rejected URLs and the addresses they resolved to belong in the log only + StringBuilder message = new StringBuilder("Harvest from ").append(oaiSource) + .append(" completed, but ").append(skippedRecords).append(" record(s) were skipped"); + if (rejectedRecords > 0) { + message.append("; ").append(rejectedRecords) + .append(" because a linked file was not allowed to be downloaded"); + } + if (failedRecords > 0) { + message.append("; ").append(failedRecords) + .append(" because a linked file could not be downloaded"); + } + harvestRow.setHarvestMessage(message.toString()); + harvestRow.setHarvestStatus(HarvestedCollection.STATUS_OAI_ERROR); + // lastHarvested deliberately not advanced: the skipped records fall outside the next incremental + // window otherwise and are never offered again; records already imported are skipped by the + // datestamp check in processRecord + } else { + harvestRow.setHarvestMessage("Harvest from " + oaiSource + " successful"); + harvestRow.setHarvestStatus(HarvestedCollection.STATUS_READY); + harvestRow.setLastHarvested(startTime); + } log.info( - "Harvest from " + oaiSource + " successful. The process took " + timeTaken + " milliseconds. Harvested " - + currentRecord + " items."); + "Harvest from " + oaiSource + (skippedRecords > 0 + ? " completed with " + skippedRecords + " skipped record(s)." : " successful.") + + " The process took " + timeTaken + " milliseconds. Harvested " + currentRecord + " items."); harvestedCollectionService.update(ourContext, harvestRow); ourContext.setMode(originalMode); @@ -476,6 +509,63 @@ private void reloadRequiredEntities() throws SQLException { harvestRow = ourContext.reloadEntity(harvestRow); } + /** + * Drop a record whose ORE ingest was refused by the egress policy. Everything this record wrote is still + * uncommitted, so rolling back restores the item exactly as it was before the ingest started. + * + * @param itemOaiID the OAI identifier of the record being dropped + * @param ore the rejection + * @throws SQLException if the rollback fails + */ + protected void rejectRecord(String itemOaiID, OreResourceRejectedException ore) throws SQLException { + // a remote server that would not serve the file is a transport problem, not a policy decision, and the + // administrator has to be told which of the two it was + if (ore.getReason() == RejectionReason.FETCH_FAILED) { + failedRecords++; + } else { + rejectedRecords++; + } + log.warn("Skipping record {} for collection {}: ORE file {} harvested from {} was rejected ({})", + itemOaiID, targetCollection.getID(), redactUrl(ore.getUrl()), harvestRow.getOaiSource(), + ore.getReason()); + ourContext.restoreAuthSystemState(); + ourContext.rollback(); + reloadRequiredEntities(); + } + + /** + * Strip the query string and any userinfo from a rejected URL before logging it: a rejected presigned + * S3/CDN URL can carry reusable credentials in those parts, which must not leak into the logs. + * + * @param url the rejected URL + * @return scheme, host, port and path only, or a placeholder when the URL cannot be parsed + */ + private static String redactUrl(String url) { + if (url == null) { + return ""; + } + try { + URI uri = new URI(url); + if (uri.getHost() != null) { + StringBuilder redacted = new StringBuilder(); + if (uri.getScheme() != null) { + redacted.append(uri.getScheme()).append("://"); + } + redacted.append(uri.getHost()); + if (uri.getPort() > 0) { + redacted.append(':').append(uri.getPort()); + } + if (uri.getRawPath() != null) { + redacted.append(uri.getRawPath()); + } + return redacted.toString(); + } + } catch (URISyntaxException e) { + // fall through to the placeholder + } + return ""; + } + /** * Process an individual PMH record, making (or updating) a corresponding DSpace Item. * @@ -532,6 +622,10 @@ protected void processRecord(Element record, String OREPrefix, final long curren if (harvestRow.getHarvestType() > 1) { oreREM = getMDrecord(harvestRow.getOaiSource(), itemOaiID, OREPrefix).get(0); ORExwalk = (IngestionCrosswalk) pluginService.getNamedPlugin(IngestionCrosswalk.class, this.ORESerialKey); + if (ORExwalk instanceof HarvestPolicyAware) { + ((HarvestPolicyAware) ORExwalk) + .setOreEgressPolicy(OreEgressPolicy.from(harvestRow, configurationService)); + } } // Ignore authorization @@ -570,7 +664,14 @@ protected void processRecord(Element record, String OREPrefix, final long curren if (harvestRow.getHarvestType() == 3) { log.info("Running ORE ingest on: " + item.getHandle()); itemService.removeAllBundles(ourContext, item); - ORExwalk.ingest(ourContext, item, oreREM, true); + try { + ORExwalk.ingest(ourContext, item, oreREM, true); + } catch (OreResourceRejectedException ore) { + // the metadata and the bundles are already gone at this point, so the whole record has to + // go back rather than leave the existing item stripped of its files + rejectRecord(itemOaiID, ore); + return; + } } } else { // NOTE: did not find, so we create (presumably, there will never be a case where an item already @@ -589,7 +690,12 @@ protected void processRecord(Element record, String OREPrefix, final long curren } if (harvestRow.getHarvestType() == 3) { - ORExwalk.ingest(ourContext, item, oreREM, true); + try { + ORExwalk.ingest(ourContext, item, oreREM, true); + } catch (OreResourceRejectedException ore) { + rejectRecord(itemOaiID, ore); + return; + } } // see if a handle can be extracted for the item diff --git a/dspace-api/src/main/java/org/dspace/harvest/ore/BlockedAddressPredicate.java b/dspace-api/src/main/java/org/dspace/harvest/ore/BlockedAddressPredicate.java new file mode 100644 index 000000000000..d78dbc58427f --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/harvest/ore/BlockedAddressPredicate.java @@ -0,0 +1,216 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.harvest.ore; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.util.Arrays; +import java.util.function.Predicate; + +/** + * Decides whether an address is one the server must never be talked into fetching from: loopback, private, + * link-local, CGNAT, reserved, documentation and cloud-metadata ranges. + *

+ * Matching is done on the raw address bytes, never on the textual form, and IPv6 addresses that embed an + * IPv4 address are unwrapped and re-checked recursively. {@code true} means BLOCKED. + */ +public class BlockedAddressPredicate implements Predicate { + + /** Ranges the java.net helpers below do not cover. */ + private static final Range[] BLOCKED_RANGES = { + // IPv4 + range(4, 8, "0.0.0.0/8 (this network)", 0), + range(4, 10, "100.64.0.0/10 (CGNAT)", 100, 64), + range(4, 24, "192.0.0.0/24 (IETF protocol assignments)", 192, 0, 0), + range(4, 24, "192.0.2.0/24 (TEST-NET-1)", 192, 0, 2), + range(4, 24, "198.51.100.0/24 (TEST-NET-2)", 198, 51, 100), + range(4, 24, "203.0.113.0/24 (TEST-NET-3)", 203, 0, 113), + range(4, 24, "192.88.99.0/24 (6to4 relay anycast)", 192, 88, 99), + range(4, 15, "198.18.0.0/15 (benchmarking)", 198, 18), + range(4, 4, "240.0.0.0/4 (reserved)", 240), + // IPv6 + range(16, 7, "fc00::/7 (unique local)", 0xfc), + range(16, 128, "::/128 (unspecified)", 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0), + range(16, 96, "::/96 (IPv4-compatible)", 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0), + range(16, 96, "::ffff:0:0:0/96 (IPv4-translated)", 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 0, 0), + range(16, 96, "64:ff9b::/96 (NAT64)", 0x00, 0x64, 0xff, 0x9b, 0, 0, 0, 0, 0, 0, 0, 0), + range(16, 64, "100::/64 (discard-only)", 0x01, 0x00, 0, 0, 0, 0, 0, 0), + range(16, 32, "2001::/32 (Teredo)", 0x20, 0x01, 0x00, 0x00), + range(16, 32, "2001:db8::/32 (documentation)", 0x20, 0x01, 0x0d, 0xb8), + range(16, 16, "2002::/16 (6to4)", 0x20, 0x02) + }; + + @Override + public boolean test(InetAddress address) { + return matchedRange(address) != null; + } + + /** + * Describe why an address is blocked, for logging. + * + * @param address the address to check + * @return a description such as "100.64.0.0/10 (CGNAT)", or null if the address is acceptable + */ + public String matchedRange(InetAddress address) { + if (address == null) { + return "unresolved address"; + } + + String category = wellKnownCategory(address); + if (category != null) { + return category; + } + + byte[] raw = address.getAddress(); + for (Range blocked : BLOCKED_RANGES) { + if (blocked.contains(raw)) { + return blocked.label; + } + } + + // an IPv6 address can carry an IPv4 address that has to be judged on its own merits + InetAddress embedded = embeddedIPv4(raw); + if (embedded != null) { + String inner = matchedRange(embedded); + if (inner != null) { + return inner + " embedded in IPv6"; + } + } + + return null; + } + + /** + * Compare the leading {@code bits} of two equally sized raw addresses. + * + * @param address the address bytes + * @param network the network bytes + * @param bits the prefix length + * @return true when the address falls inside the network + */ + static boolean prefixMatches(byte[] address, byte[] network, int bits) { + if (address.length != network.length || bits < 0 || bits > address.length * 8) { + return false; + } + int wholeBytes = bits / 8; + for (int i = 0; i < wholeBytes; i++) { + if (address[i] != network[i]) { + return false; + } + } + int remainder = bits % 8; + if (remainder == 0) { + return true; + } + int mask = (0xff << (8 - remainder)) & 0xff; + return (address[wholeBytes] & mask) == (network[wholeBytes] & mask); + } + + private static String wellKnownCategory(InetAddress address) { + if (address.isAnyLocalAddress()) { + return "wildcard address"; + } + if (address.isLoopbackAddress()) { + return "loopback address"; + } + if (address.isLinkLocalAddress()) { + return "link-local address"; + } + if (address.isSiteLocalAddress()) { + return "site-local address"; + } + if (address.isMulticastAddress()) { + return "multicast address"; + } + return null; + } + + /** + * Extract the IPv4 address tunnelled inside an IPv6 address, if there is one. Without this + * 2002:7f00:0001:: would reach 127.0.0.1. + */ + private static InetAddress embeddedIPv4(byte[] raw) { + if (raw.length != 16) { + return null; + } + + byte[] extracted = null; + if (isZero(raw, 10) && unsigned(raw[10]) == 0xff && unsigned(raw[11]) == 0xff) { + extracted = slice(raw, 12); // ::ffff:0:0/96, IPv4-mapped + } else if (isZero(raw, 8) && unsigned(raw[8]) == 0xff && unsigned(raw[9]) == 0xff + && raw[10] == 0 && raw[11] == 0) { + extracted = slice(raw, 12); // ::ffff:0:0:0/96, IPv4-translated + } else if (isZero(raw, 12)) { + extracted = slice(raw, 12); // ::/96, IPv4-compatible + } else if (unsigned(raw[0]) == 0x20 && unsigned(raw[1]) == 0x02) { + extracted = slice(raw, 2); // 2002::/16, 6to4 + } else if (unsigned(raw[0]) == 0x20 && unsigned(raw[1]) == 0x01 && raw[2] == 0 && raw[3] == 0) { + extracted = invert(slice(raw, 12)); // 2001::/32, Teredo stores it inverted + } else if (raw[0] == 0 && unsigned(raw[1]) == 0x64 && unsigned(raw[2]) == 0xff && unsigned(raw[3]) == 0x9b) { + extracted = slice(raw, 12); // 64:ff9b::/96, NAT64 + } + + if (extracted == null) { + return null; + } + try { + return InetAddress.getByAddress(extracted); + } catch (UnknownHostException e) { + return null; // unreachable: the array is always four bytes long + } + } + + private static boolean isZero(byte[] raw, int length) { + for (int i = 0; i < length; i++) { + if (raw[i] != 0) { + return false; + } + } + return true; + } + + private static byte[] slice(byte[] raw, int from) { + return Arrays.copyOfRange(raw, from, from + 4); + } + + private static byte[] invert(byte[] raw) { + for (int i = 0; i < raw.length; i++) { + raw[i] = (byte) ~raw[i]; + } + return raw; + } + + private static int unsigned(byte value) { + return value & 0xff; + } + + private static Range range(int addressLength, int bits, String label, int... network) { + byte[] bytes = new byte[addressLength]; + for (int i = 0; i < network.length; i++) { + bytes[i] = (byte) network[i]; + } + return new Range(bytes, bits, label); + } + + /** A network prefix held as raw bytes; only the leading {@code bits} are ever compared. */ + private static final class Range { + private final byte[] network; + private final int bits; + private final String label; + + private Range(byte[] network, int bits, String label) { + this.network = network; + this.bits = bits; + this.label = label; + } + + private boolean contains(byte[] address) { + return prefixMatches(address, network, bits); + } + } +} diff --git a/dspace-api/src/main/java/org/dspace/harvest/ore/HarvestPolicyAware.java b/dspace-api/src/main/java/org/dspace/harvest/ore/HarvestPolicyAware.java new file mode 100644 index 000000000000..d64c8b133097 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/harvest/ore/HarvestPolicyAware.java @@ -0,0 +1,21 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.harvest.ore; + +/** + * Implemented by crosswalks that fetch remote content and therefore need the egress rules of the collection + * being harvested. A crosswalk that is never given a policy must fall back to + * {@link OreEgressPolicy#strictest(org.dspace.services.ConfigurationService)}. + */ +public interface HarvestPolicyAware { + + /** + * @param policy the rules to apply to remote fetches during this harvest + */ + void setOreEgressPolicy(OreEgressPolicy policy); +} diff --git a/dspace-api/src/main/java/org/dspace/harvest/ore/OreEgressPolicy.java b/dspace-api/src/main/java/org/dspace/harvest/ore/OreEgressPolicy.java new file mode 100644 index 000000000000..32db1ccc4c28 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/harvest/ore/OreEgressPolicy.java @@ -0,0 +1,139 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.harvest.ore; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.commons.lang3.StringUtils; +import org.dspace.harvest.HarvestedCollection; +import org.dspace.services.ConfigurationService; + +/** + * Immutable set of rules applied when the ORE ingest crosswalk fetches an aggregated resource. + *

+ * An aggregated file is confined to the host of the collection's oai_source, or one of the configured + * allowed prefixes; internal, private and reserved addresses stay blocked in either case. + */ +public final class OreEgressPolicy { + + static final String CONFIG_PREFIX = "oai.harvester.ore.file."; + + private final URI anchor; + private final boolean blockInternalAddresses; + private final List allowedUrlPrefix; + private final List allowedInternalHosts; + private final int maxRedirects; + private final int connectTimeoutMs; + private final int readTimeoutMs; + private final long maxBytes; + + private OreEgressPolicy(URI anchor, ConfigurationService configurationService) { + this.anchor = anchor; + this.blockInternalAddresses = + configurationService.getBooleanProperty(CONFIG_PREFIX + "blockInternalAddresses", true); + this.allowedUrlPrefix = trimmedList(configurationService.getArrayProperty(CONFIG_PREFIX + "allowedUrlPrefix")); + this.allowedInternalHosts = + trimmedList(configurationService.getArrayProperty(CONFIG_PREFIX + "allowedInternalHosts")); + this.maxRedirects = configurationService.getIntProperty(CONFIG_PREFIX + "maxRedirects", 3); + this.connectTimeoutMs = configurationService.getIntProperty(CONFIG_PREFIX + "connectTimeout", 10000); + this.readTimeoutMs = configurationService.getIntProperty(CONFIG_PREFIX + "readTimeout", 30000); + this.maxBytes = configurationService.getLongProperty(CONFIG_PREFIX + "maxBytes", 2147483648L); + } + + /** + * Policy for callers that have no harvest context, such as the AIP/METS/SWORD packagers and the XSLT CLI: + * no anchor, so only an explicit allowedUrlPrefix can satisfy the host check. + * + * @param configurationService the DSpace configuration + * @return the fail-closed policy + */ + public static OreEgressPolicy strictest(ConfigurationService configurationService) { + return new OreEgressPolicy(null, configurationService); + } + + /** + * Policy for a harvested collection. The trust anchor is the administrator-supplied oai_source, never a + * value read out of the harvested document. + * + * @param harvestRow the collection being harvested, may be null + * @param configurationService the DSpace configuration + * @return the policy to apply to that collection's ORE ingest + */ + public static OreEgressPolicy from(HarvestedCollection harvestRow, ConfigurationService configurationService) { + if (harvestRow == null) { + return strictest(configurationService); + } + return new OreEgressPolicy(toUri(harvestRow.getOaiSource()), configurationService); + } + + /** + * @return the oai_source this collection harvests from, or null when there is no harvest context or the + * configured value is not a usable absolute URL + */ + public URI getAnchor() { + return anchor; + } + + public boolean isBlockInternalAddresses() { + return blockInternalAddresses; + } + + public List getAllowedUrlPrefix() { + return allowedUrlPrefix; + } + + public List getAllowedInternalHosts() { + return allowedInternalHosts; + } + + public int getMaxRedirects() { + return maxRedirects; + } + + public int getConnectTimeoutMs() { + return connectTimeoutMs; + } + + public int getReadTimeoutMs() { + return readTimeoutMs; + } + + /** + * @return the response size cap in bytes, or a negative value for unlimited + */ + public long getMaxBytes() { + return maxBytes; + } + + private static URI toUri(String value) { + if (StringUtils.isBlank(value)) { + return null; + } + try { + URI uri = new URI(value.trim()); + return uri.isAbsolute() && uri.getHost() != null ? uri : null; + } catch (URISyntaxException e) { + return null; + } + } + + private static List trimmedList(String[] values) { + if (values == null || values.length == 0) { + return Collections.emptyList(); + } + return Arrays.stream(values) + .filter(StringUtils::isNotBlank) + .map(String::trim) + .collect(Collectors.toUnmodifiableList()); + } +} diff --git a/dspace-api/src/main/java/org/dspace/harvest/ore/OreResourceRejectedException.java b/dspace-api/src/main/java/org/dspace/harvest/ore/OreResourceRejectedException.java new file mode 100644 index 000000000000..25e16e439e5e --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/harvest/ore/OreResourceRejectedException.java @@ -0,0 +1,51 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.harvest.ore; + +import org.dspace.content.crosswalk.CrosswalkException; + +/** + * Thrown when an ORE aggregated resource may not be fetched. It extends {@link CrosswalkException} so that + * it travels through the existing crosswalk signatures without changing them. + */ +public class OreResourceRejectedException extends CrosswalkException { + + private final RejectionReason reason; + private final String url; + + public OreResourceRejectedException(RejectionReason reason, String url, String detail) { + super(buildMessage(reason, url, detail)); + this.reason = reason; + this.url = url; + } + + public OreResourceRejectedException(RejectionReason reason, String url, String detail, Throwable cause) { + super(buildMessage(reason, url, detail), cause); + this.reason = reason; + this.url = url; + } + + public RejectionReason getReason() { + return reason; + } + + /** + * @return the URL that was refused; for a redirect chain this is the offending hop, not the original URL + */ + public String getUrl() { + return url; + } + + private static String buildMessage(RejectionReason reason, String url, String detail) { + StringBuilder message = new StringBuilder("ORE resource rejected [").append(reason).append("]: ").append(url); + if (detail != null && !detail.isEmpty()) { + message.append(" (").append(detail).append(')'); + } + return message.toString(); + } +} diff --git a/dspace-api/src/main/java/org/dspace/harvest/ore/OreUrlValidator.java b/dspace-api/src/main/java/org/dspace/harvest/ore/OreUrlValidator.java new file mode 100644 index 000000000000..07af92ee4ffd --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/harvest/ore/OreUrlValidator.java @@ -0,0 +1,242 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.harvest.ore; + +import java.net.InetAddress; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.UnknownHostException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Locale; + +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +/** + * Decides whether a single ORE aggregated resource URL may be fetched. Checks are applied in a fixed order + * and the first violation ends the check, so an unexpected input always fails closed. + */ +public class OreUrlValidator { + + private static final Logger log = LogManager.getLogger(); + + private static final String HTTP = "http"; + private static final String HTTPS = "https"; + + private final BlockedAddressPredicate blockedAddresses = new BlockedAddressPredicate(); + + /** + * Turn an href taken from an ORE document into a URI, refusing anything that is not usable. + * + * @param url the raw href + * @return the parsed URI + * @throws OreResourceRejectedException if the value is not an absolute, parseable URL + */ + public static URI parse(String url) throws OreResourceRejectedException { + try { + URI uri = new URI(url); + if (!uri.isAbsolute()) { + throw new OreResourceRejectedException(RejectionReason.SCHEME_NOT_ALLOWED, url, "not absolute"); + } + return uri; + } catch (URISyntaxException e) { + throw new OreResourceRejectedException(RejectionReason.MALFORMED_AUTHORITY, url, "unparseable URL", e); + } + } + + /** + * Validate a URL against the egress policy. + * + * @param uri the URL to fetch, or a single redirect hop of it + * @param policy the rules to apply + * @return the decision, carrying the resolved addresses when the URL is allowed + */ + public Decision validate(URI uri, OreEgressPolicy policy) { + if (uri == null) { + return Decision.reject(null, RejectionReason.MALFORMED_AUTHORITY, "no URL"); + } + + // 1. only http(s); this is what keeps file:, jar:, ftp: and friends out + String scheme = lower(uri.getScheme()); + if (!HTTP.equals(scheme) && !HTTPS.equals(scheme)) { + return Decision.reject(uri, RejectionReason.SCHEME_NOT_ALLOWED, "scheme " + uri.getScheme()); + } + + // 2. no credentials in the authority + String authority = uri.getRawAuthority(); + if (uri.getRawUserInfo() != null || (authority != null && authority.indexOf('@') >= 0)) { + return Decision.reject(uri, RejectionReason.USERINFO_PRESENT, "credentials in authority"); + } + + // 3. a host we can actually resolve; upstream dereferences this without checking + String host = uri.getHost(); + if (StringUtils.isBlank(host) || host.indexOf('%') >= 0) { + return Decision.reject(uri, RejectionReason.MALFORMED_AUTHORITY, "authority " + authority); + } + + // 4. host confinement: the resource must share scheme+host with the oai_source or an allowed prefix + if (!isHostConfined(uri, scheme, host, policy)) { + return Decision.reject(uri, RejectionReason.HOST_NOT_ALLOWED, "host " + host); + } + + // 5. resolve now, so the addresses we vet are the ones we connect to + List addresses; + try { + addresses = Arrays.asList(InetAddress.getAllByName(host)); + } catch (UnknownHostException e) { + return Decision.reject(uri, RejectionReason.HOST_UNRESOLVABLE, "host " + host); + } + if (addresses.isEmpty()) { + return Decision.reject(uri, RejectionReason.HOST_UNRESOLVABLE, "host " + host); + } + + // 6. every returned address must pass unless it is explicitly exempted + if (policy.isBlockInternalAddresses() && !isHostExempt(host, policy)) { + for (InetAddress address : addresses) { + String range = blockedAddresses.matchedRange(address); + if (range != null && !isAddressExempt(address, policy)) { + return Decision.reject(uri, RejectionReason.ADDRESS_BLOCKED, range); + } + } + } + + return Decision.allow(uri, addresses); + } + + /** + * The resource must share scheme and host with the collection's oai_source, or match a globally allowed + * prefix. The port is deliberately not compared, so same-host-different-port setups keep working. + */ + private boolean isHostConfined(URI uri, String scheme, String host, OreEgressPolicy policy) { + URI anchor = policy.getAnchor(); + if (anchor != null && host.equalsIgnoreCase(anchor.getHost()) && scheme.equals(lower(anchor.getScheme()))) { + return true; + } + for (String prefix : policy.getAllowedUrlPrefix()) { + if (matchesAllowedPrefix(uri, host, prefix)) { + return true; + } + } + return false; + } + + private boolean matchesAllowedPrefix(URI uri, String host, String prefix) { + if (!prefix.contains("://")) { + return host.equalsIgnoreCase(prefix); // bare hostname, the form documented in oai.cfg + } + URI allowed; + try { + allowed = new URI(prefix); + } catch (URISyntaxException e) { + log.warn("Ignoring unparseable {}allowedUrlPrefix entry: {}", OreEgressPolicy.CONFIG_PREFIX, prefix); + return false; + } + if (allowed.getHost() == null || !host.equalsIgnoreCase(allowed.getHost()) + || !lower(uri.getScheme()).equals(lower(allowed.getScheme()))) { + return false; + } + // the host was compared exactly above, so the remaining prefix test cannot be widened by a lookalike host + String allowedPath = StringUtils.defaultString(allowed.getRawPath()); + return allowedPath.isEmpty() || "/".equals(allowedPath) + || StringUtils.defaultString(uri.getRawPath()).startsWith(allowedPath); + } + + private boolean isHostExempt(String host, OreEgressPolicy policy) { + for (String entry : policy.getAllowedInternalHosts()) { + if (entry.indexOf('/') < 0 && entry.equalsIgnoreCase(host)) { + return true; + } + } + return false; + } + + private boolean isAddressExempt(InetAddress address, OreEgressPolicy policy) { + for (String entry : policy.getAllowedInternalHosts()) { + if (entry.indexOf('/') >= 0 && cidrContains(entry, address)) { + return true; + } + } + return false; + } + + private boolean cidrContains(String cidr, InetAddress address) { + int slash = cidr.lastIndexOf('/'); + try { + // the network part comes from configuration, not from the harvested document + InetAddress network = InetAddress.getByName(cidr.substring(0, slash).trim()); + int bits = Integer.parseInt(cidr.substring(slash + 1).trim()); + return BlockedAddressPredicate.prefixMatches(address.getAddress(), network.getAddress(), bits); + } catch (UnknownHostException | NumberFormatException e) { + log.warn("Ignoring unparseable {}allowedInternalHosts entry: {}", OreEgressPolicy.CONFIG_PREFIX, cidr); + return false; + } + } + + private static String lower(String value) { + return value == null ? "" : value.toLowerCase(Locale.ROOT); + } + + /** + * The outcome of validating one URL. + */ + public static final class Decision { + + private final boolean allowed; + private final RejectionReason reason; + private final URI uri; + private final List addresses; + private final String detail; + + private Decision(boolean allowed, RejectionReason reason, URI uri, List addresses, + String detail) { + this.allowed = allowed; + this.reason = reason; + this.uri = uri; + this.addresses = addresses; + this.detail = detail; + } + + static Decision allow(URI uri, List addresses) { + return new Decision(true, null, uri, Collections.unmodifiableList(addresses), null); + } + + static Decision reject(URI uri, RejectionReason reason, String detail) { + return new Decision(false, reason, uri, Collections.emptyList(), detail); + } + + public boolean isAllowed() { + return allowed; + } + + public RejectionReason getReason() { + return reason; + } + + public URI getUri() { + return uri; + } + + /** + * @return every address the host resolved to, empty when the URL was rejected + */ + public List getAddresses() { + return addresses; + } + + /** + * @return a short explanation for the log; may name the blocked range, so keep it out of user-visible + * messages + */ + public String getDetail() { + return detail; + } + } +} diff --git a/dspace-api/src/main/java/org/dspace/harvest/ore/RejectionReason.java b/dspace-api/src/main/java/org/dspace/harvest/ore/RejectionReason.java new file mode 100644 index 000000000000..d135253ac046 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/harvest/ore/RejectionReason.java @@ -0,0 +1,34 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.harvest.ore; + +/** + * Why an ORE aggregated resource was refused by the egress policy. + */ +public enum RejectionReason { + /** The URL scheme was neither http nor https. */ + SCHEME_NOT_ALLOWED, + /** The authority carried credentials. */ + USERINFO_PRESENT, + /** The authority could not be parsed into a usable host. */ + MALFORMED_AUTHORITY, + /** The host is not the one hosting the OAI source and is not explicitly allowed. */ + HOST_NOT_ALLOWED, + /** The host did not resolve to any address. */ + HOST_UNRESOLVABLE, + /** The host resolved to an internal, private or otherwise reserved address. */ + ADDRESS_BLOCKED, + /** The redirect chain exceeded the configured hop cap. */ + TOO_MANY_REDIRECTS, + /** A redirect moved the request from https to http. */ + REDIRECT_DOWNGRADE, + /** The response was larger than the configured byte cap. */ + RESPONSE_TOO_LARGE, + /** The remote server did not return a usable response. */ + FETCH_FAILED +} diff --git a/dspace-api/src/main/java/org/dspace/harvest/ore/SafeResourceFetcher.java b/dspace-api/src/main/java/org/dspace/harvest/ore/SafeResourceFetcher.java new file mode 100644 index 000000000000..93320b68e108 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/harvest/ore/SafeResourceFetcher.java @@ -0,0 +1,274 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.harvest.ore; + +import java.io.FilterInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.InetAddress; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.List; +import java.util.Locale; + +import org.apache.commons.lang3.StringUtils; +import org.apache.http.Header; +import org.apache.http.HttpEntity; +import org.apache.http.HttpHost; +import org.apache.http.HttpStatus; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.conn.ConnectTimeoutException; +import org.apache.http.conn.HttpHostConnectException; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.protocol.HTTP; +import org.dspace.app.client.DSpaceHttpClientFactory; + +/** + * Fetches an ORE aggregated resource under an {@link OreEgressPolicy}: redirects are followed by hand so + * that every hop is validated again with a fresh DNS lookup, the socket is pinned to the address that was + * validated, and the body is capped. + */ +public class SafeResourceFetcher { + + private static final String HTTPS = "https"; + + /** HttpClient 4.5 has no HttpStatus constant for 308. */ + private static final int SC_PERMANENT_REDIRECT = 308; + + private final OreUrlValidator validator = new OreUrlValidator(); + + /** + * Fetch a resource. + * + * @param url the resource to fetch + * @param policy the rules to apply, to this URL and to every redirect hop + * @return the response body; closing it releases the connection + * @throws OreResourceRejectedException if the URL, a redirect hop or the response is not acceptable + * @throws IOException if the transfer itself fails + */ + public InputStream fetch(URI url, OreEgressPolicy policy) throws OreResourceRejectedException, IOException { + CloseableHttpClient client = buildClient(policy); + boolean streamReturned = false; + try { + URI current = url; + int hops = 0; + while (true) { + OreUrlValidator.Decision decision = validator.validate(current, policy); + if (!decision.isAllowed()) { + throw new OreResourceRejectedException(decision.getReason(), current.toString(), + decision.getDetail()); + } + + CloseableHttpResponse response = execute(client, current, decision.getAddresses()); + boolean responseReturned = false; + try { + int status = response.getStatusLine().getStatusCode(); + if (isRedirect(status)) { + current = nextHop(current, response, policy, hops++); + continue; + } + InputStream body = openBody(current, response, policy); + responseReturned = true; + streamReturned = true; + return new CappedStream(body, response, client, policy.getMaxBytes()); + } finally { + if (!responseReturned) { + response.close(); + } + } + } + } finally { + if (!streamReturned) { + client.close(); + } + } + } + + private InputStream openBody(URI current, CloseableHttpResponse response, OreEgressPolicy policy) + throws OreResourceRejectedException, IOException { + int status = response.getStatusLine().getStatusCode(); + if (status != HttpStatus.SC_OK) { + throw new OreResourceRejectedException(RejectionReason.FETCH_FAILED, current.toString(), + "HTTP status " + status); + } + HttpEntity entity = response.getEntity(); + if (entity == null) { + throw new OreResourceRejectedException(RejectionReason.FETCH_FAILED, current.toString(), + "no response body"); + } + if (policy.getMaxBytes() >= 0 && entity.getContentLength() > policy.getMaxBytes()) { + throw new OreResourceRejectedException(RejectionReason.RESPONSE_TOO_LARGE, current.toString(), + "declared " + entity.getContentLength() + " bytes"); + } + return entity.getContent(); + } + + private CloseableHttpClient buildClient(OreEgressPolicy policy) { + RequestConfig requestConfig = RequestConfig.custom() + .setRedirectsEnabled(false) + .setConnectTimeout(policy.getConnectTimeoutMs()) + .setConnectionRequestTimeout(policy.getConnectTimeoutMs()) + .setSocketTimeout(policy.getReadTimeoutMs()) + .build(); + // compression is off so that a compressed body cannot expand past the byte cap; builder(true) keeps the + // configured egress proxy, which then owns the connect and therefore the address pinning + return DSpaceHttpClientFactory.getInstance().builder(true) + .setDefaultRequestConfig(requestConfig) + .disableContentCompression() + .disableAutomaticRetries() + .build(); + } + + /** + * Try the validated addresses in turn. The validator vetted every one of them, so failing over is + * security-neutral, and it keeps a round-robin DNS entry with one dead address working as it did before. + */ + private CloseableHttpResponse execute(CloseableHttpClient client, URI uri, List addresses) + throws IOException { + IOException lastFailure = null; + for (InetAddress address : addresses) { + try { + return client.execute(pinnedHost(uri, address), newRequest(uri)); + } catch (ConnectTimeoutException | HttpHostConnectException e) { + lastFailure = e; + } + } + // the list is never empty: the validator rejects a host that resolves to nothing + throw lastFailure != null ? lastFailure : new IOException("no address to connect to for " + uri.getHost()); + } + + private HttpGet newRequest(URI uri) { + HttpGet request = new HttpGet(requestTarget(uri)); + if (uri.getPort() <= 0) { + // pinning forces an explicit port onto the HttpHost, but the default port must stay out of Host: + // or presigned S3 and signed-CDN URLs fail their signature check + request.setHeader(HTTP.TARGET_HOST, uri.getHost()); + } + return request; + } + + /** + * Connect to the address that was validated, but keep the hostname so Host: and SNI still carry the name. + */ + private HttpHost pinnedHost(URI uri, InetAddress address) { + String scheme = lower(uri.getScheme()); + // the port has to be explicit: DefaultRoutePlanner rebuilds a portless HttpHost by name and would + // throw the pinned address away, leaving the connection to a second, unvalidated DNS lookup + int port = uri.getPort() > 0 ? uri.getPort() : (HTTPS.equals(scheme) ? 443 : 80); + return new HttpHost(address, uri.getHost(), port, scheme); + } + + private String requestTarget(URI uri) { + String path = StringUtils.defaultIfEmpty(uri.getRawPath(), "/"); + return uri.getRawQuery() == null ? path : path + "?" + uri.getRawQuery(); + } + + private URI nextHop(URI current, CloseableHttpResponse response, OreEgressPolicy policy, int hop) + throws OreResourceRejectedException { + if (hop >= policy.getMaxRedirects()) { + throw new OreResourceRejectedException(RejectionReason.TOO_MANY_REDIRECTS, current.toString(), + "more than " + policy.getMaxRedirects() + " hops"); + } + Header location = response.getFirstHeader("Location"); + if (location == null || StringUtils.isBlank(location.getValue())) { + throw new OreResourceRejectedException(RejectionReason.FETCH_FAILED, current.toString(), + "redirect without a Location header"); + } + URI next; + try { + next = current.resolve(new URI(location.getValue().trim())); + } catch (URISyntaxException | IllegalArgumentException e) { + throw new OreResourceRejectedException(RejectionReason.MALFORMED_AUTHORITY, current.toString(), + "unparseable Location header", e); + } + if (HTTPS.equals(lower(current.getScheme())) && !HTTPS.equals(lower(next.getScheme()))) { + throw new OreResourceRejectedException(RejectionReason.REDIRECT_DOWNGRADE, next.toString(), + "https redirected to " + next.getScheme()); + } + return next; + } + + private static boolean isRedirect(int status) { + return status == HttpStatus.SC_MOVED_PERMANENTLY || status == HttpStatus.SC_MOVED_TEMPORARILY + || status == HttpStatus.SC_SEE_OTHER || status == HttpStatus.SC_TEMPORARY_REDIRECT + || status == SC_PERMANENT_REDIRECT; + } + + private static String lower(String value) { + return value == null ? "" : value.toLowerCase(Locale.ROOT); + } + + /** + * Signals that the response outgrew the configured cap while it was being read. It has to be an + * {@link IOException} to be throwable from {@link InputStream#read()}. + */ + public static class ResponseTooLargeException extends IOException { + public ResponseTooLargeException(long maxBytes) { + super("ORE resource exceeded the configured maximum of " + maxBytes + " bytes"); + } + } + + /** + * Counts what is actually read rather than trusting Content-Length, and releases the connection and the + * client when it is closed. + */ + private static class CappedStream extends FilterInputStream { + + private final CloseableHttpResponse response; + private final CloseableHttpClient client; + private final long maxBytes; + private long consumed; + + CappedStream(InputStream in, CloseableHttpResponse response, CloseableHttpClient client, long maxBytes) { + super(in); + this.response = response; + this.client = client; + this.maxBytes = maxBytes; + } + + @Override + public int read() throws IOException { + int read = super.read(); + if (read >= 0) { + count(1); + } + return read; + } + + @Override + public int read(byte[] buffer, int offset, int length) throws IOException { + int read = super.read(buffer, offset, length); + if (read > 0) { + count(read); + } + return read; + } + + private void count(long bytes) throws IOException { + consumed += bytes; + if (maxBytes >= 0 && consumed > maxBytes) { + throw new ResponseTooLargeException(maxBytes); + } + } + + @Override + public void close() throws IOException { + try { + super.close(); + } finally { + try { + response.close(); + } finally { + client.close(); + } + } + } + } +} diff --git a/dspace-api/src/test/java/org/dspace/harvest/ore/BlockedAddressPredicateTest.java b/dspace-api/src/test/java/org/dspace/harvest/ore/BlockedAddressPredicateTest.java new file mode 100644 index 000000000000..85b06f94525a --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/harvest/ore/BlockedAddressPredicateTest.java @@ -0,0 +1,152 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.harvest.ore; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.net.InetAddress; +import java.net.UnknownHostException; + +import org.junit.Test; + +/** + * Unit tests for {@link BlockedAddressPredicate}. Every address here is an IP literal, so no test performs a + * DNS lookup. + */ +public class BlockedAddressPredicateTest { + + private final BlockedAddressPredicate predicate = new BlockedAddressPredicate(); + + @Test + public void blocksLoopbackAndWildcardAddresses() throws Exception { + assertBlocked("127.0.0.1"); + assertBlocked("127.99.1.2"); + assertBlocked("0.0.0.0"); + assertBlocked("0.1.2.3"); + assertBlocked("::1"); + assertBlocked("::"); + } + + @Test + public void blocksPrivateIPv4Addresses() throws Exception { + assertBlocked("10.0.0.5"); + assertBlocked("172.16.0.9"); + assertBlocked("192.168.1.1"); + } + + @Test + public void blocksCloudMetadataAddresses() throws Exception { + // AWS, GCP and Azure all publish their instance metadata here + assertBlocked(InetAddress.getByAddress(bytes(169, 254, 169, 254))); + // Alibaba puts it in CGNAT space, which none of the java.net helpers report + assertBlocked("100.100.100.200"); + assertBlocked("100.64.0.1"); + } + + @Test + public void blocksReservedIPv4Ranges() throws Exception { + assertBlocked("192.0.0.170"); + assertBlocked("192.0.2.1"); + assertBlocked("198.51.100.1"); + assertBlocked("203.0.113.1"); + assertBlocked("192.88.99.1"); + assertBlocked("198.18.0.1"); + assertBlocked("240.0.0.1"); + assertBlocked("255.255.255.255"); + } + + @Test + public void blocksMulticastAddresses() throws Exception { + assertBlocked("224.0.0.1"); + assertBlocked("ff02::1"); + } + + @Test + public void blocksUniqueLocalIPv6Addresses() throws Exception { + InetAddress uniqueLocal = InetAddress.getByName("fd12:3456::1"); + // the gap that matters most: the JDK does not consider fc00::/7 site-local + assertFalse(uniqueLocal.isSiteLocalAddress()); + assertBlocked(uniqueLocal); + assertBlocked("fc00::1"); + } + + @Test + public void blocksReservedIPv6Ranges() throws Exception { + assertBlocked("2001:db8::1"); + assertBlocked("100::1"); + } + + @Test + public void blocksIPv4AddressesTunnelledInsideIPv6() throws Exception { + assertBlocked("::ffff:127.0.0.1"); + assertBlocked("2002:7f00:0001::"); + assertBlocked("64:ff9b::7f00:1"); + assertBlocked("2001:0:4136:e378:8000:63bf:3fff:fdd2"); + // ::ffff:0:0:0/96 (RFC 6052 IPv4-translated) carries the IPv4 address two bytes further along + assertBlocked("::ffff:0:7f00:1"); + assertBlocked("::ffff:0:a9fe:a9fe"); + } + + @Test + public void allowsPublicAddresses() throws Exception { + assertAllowed("8.8.8.8"); + assertAllowed("93.184.216.34"); + assertAllowed(InetAddress.getByAddress(bytes(1, 1, 1, 1))); + assertAllowed("2606:4700:4700::1111"); + assertAllowed("2a00:1450:4001:80e::200e"); + } + + @Test + public void namesTheRangeThatMatched() throws Exception { + assertNull(predicate.matchedRange(InetAddress.getByName("8.8.8.8"))); + assertNotNull(predicate.matchedRange(InetAddress.getByName("100.100.100.200"))); + assertNotNull("an unresolved address must fail closed", predicate.matchedRange(null)); + } + + @Test + public void comparesOnlyTheLeadingBitsOfAPrefix() throws Exception { + byte[] network = InetAddress.getByName("10.0.0.0").getAddress(); + assertTrue(BlockedAddressPredicate.prefixMatches(address("10.255.3.4"), network, 8)); + assertFalse(BlockedAddressPredicate.prefixMatches(address("11.0.0.1"), network, 8)); + assertTrue(BlockedAddressPredicate.prefixMatches(address("10.0.0.1"), network, 0)); + // a prefix can never match across address families + assertFalse(BlockedAddressPredicate.prefixMatches(address("::1"), network, 8)); + assertFalse(BlockedAddressPredicate.prefixMatches(address("10.255.3.4"), network, 99)); + } + + private void assertBlocked(String literal) throws UnknownHostException { + assertBlocked(InetAddress.getByName(literal)); + } + + private void assertBlocked(InetAddress address) { + assertTrue(address.getHostAddress() + " must be blocked", predicate.test(address)); + } + + private void assertAllowed(String literal) throws UnknownHostException { + assertAllowed(InetAddress.getByName(literal)); + } + + private void assertAllowed(InetAddress address) { + assertFalse(address.getHostAddress() + " must be allowed", predicate.test(address)); + } + + private byte[] address(String literal) throws UnknownHostException { + return InetAddress.getByName(literal).getAddress(); + } + + private static byte[] bytes(int... values) { + byte[] raw = new byte[values.length]; + for (int i = 0; i < values.length; i++) { + raw[i] = (byte) values[i]; + } + return raw; + } +} diff --git a/dspace-api/src/test/java/org/dspace/harvest/ore/OreUrlValidatorTest.java b/dspace-api/src/test/java/org/dspace/harvest/ore/OreUrlValidatorTest.java new file mode 100644 index 000000000000..d19e2cc8835f --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/harvest/ore/OreUrlValidatorTest.java @@ -0,0 +1,208 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.harvest.ore; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.net.URI; +import java.util.HashMap; +import java.util.Map; + +import org.dspace.harvest.HarvestedCollection; +import org.dspace.services.ConfigurationService; +import org.junit.Test; + +/** + * Unit tests for {@link OreUrlValidator}. The policies are built by hand and every host is an IP literal, so + * no test needs Spring or a DNS lookup. + */ +public class OreUrlValidatorTest { + + private static final String PREFIX = "oai.harvester.ore.file."; + + /** A public host, standing in for the repository the collection harvests from. */ + private static final String OAI_SOURCE = "http://8.8.8.8/oai/request"; + + /** A different public host, standing in for the one an ORE record points at. */ + private static final String OTHER_HOST = "93.184.216.34"; + + private final OreUrlValidator validator = new OreUrlValidator(); + + private final Map config = new HashMap<>(); + + @Test + public void rejectsNonHttpSchemes() { + OreEgressPolicy policy = policy(OAI_SOURCE); + assertRejected(RejectionReason.SCHEME_NOT_ALLOWED, "file:///etc/passwd", policy); + assertRejected(RejectionReason.SCHEME_NOT_ALLOWED, "jar:file:///opt/dspace/lib/api.jar!/dspace.cfg", policy); + assertRejected(RejectionReason.SCHEME_NOT_ALLOWED, "ftp://8.8.8.8/private", policy); + assertRejected(RejectionReason.SCHEME_NOT_ALLOWED, "netdoc:///etc/passwd", policy); + } + + @Test + public void rejectsAnAuthorityWithoutAHost() { + // upstream dereferences this host without checking it and throws NPE + assertRejected(RejectionReason.MALFORMED_AUTHORITY, "http:///etc/passwd", policy(OAI_SOURCE)); + } + + @Test + public void rejectsCredentialsInTheAuthority() { + OreEgressPolicy policy = policy(OAI_SOURCE); + assertRejected(RejectionReason.USERINFO_PRESENT, "http://user:secret@8.8.8.8/x", policy); + // the "real" host of this one is the metadata service, not the one that reads like a hostname + assertRejected(RejectionReason.USERINFO_PRESENT, "http://8.8.8.8@169.254.169.254/latest/", policy); + } + + @Test + public void rejectsObfuscatedAddressForms() { + // the decimal and IPv6-embedded forms resolve to an internal address and must be caught there, so + // the policy is anchored on each host to let the address check be what rejects it + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://2130706433/", anchoredOn("2130706433")); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://[::ffff:7f00:1]/", anchoredOn("[::ffff:7f00:1]")); + // these never parse into a usable host in the first place + assertRejected(RejectionReason.MALFORMED_AUTHORITY, "http://127.1/", anchoredOn("127.1")); + assertRejected(RejectionReason.MALFORMED_AUTHORITY, "http://0x7f.0x0.0x0.0x1/", anchoredOn("0x7f.0x0.0x0.0x1")); + } + + @Test + public void blocksADifferentHost() { + assertRejected(RejectionReason.HOST_NOT_ALLOWED, "http://" + OTHER_HOST + "/file.pdf", policy(OAI_SOURCE)); + } + + @Test + public void allowsTheOaiSourceHost() { + OreEgressPolicy policy = policy(OAI_SOURCE); + assertAllowed("http://8.8.8.8/bitstream/123/1/file.pdf", policy); + // the port is deliberately not part of the comparison + assertAllowed("http://8.8.8.8:8080/bitstream/123/1/file.pdf", policy); + // the scheme is compared lower-cased + assertAllowed("HTTP://8.8.8.8/bitstream/123/1/file.pdf", policy); + } + + @Test + public void blocksASchemeMismatchWithTheOaiSource() { + assertRejected(RejectionReason.HOST_NOT_ALLOWED, "http://8.8.8.8/file.pdf", + policy("https://8.8.8.8/oai/request")); + } + + @Test + public void allowsAHostListedInAllowedUrlPrefix() { + config.put(PREFIX + "allowedUrlPrefix", new String[] {"http://" + OTHER_HOST + "/files"}); + OreEgressPolicy policy = policy(null); + assertAllowed("http://" + OTHER_HOST + "/files/a.pdf", policy); + assertRejected(RejectionReason.HOST_NOT_ALLOWED, "http://" + OTHER_HOST + "/elsewhere/a.pdf", policy); + assertRejected(RejectionReason.HOST_NOT_ALLOWED, "https://" + OTHER_HOST + "/files/a.pdf", policy); + + // the bare hostname form documented in oai.cfg + config.put(PREFIX + "allowedUrlPrefix", new String[] {OTHER_HOST}); + assertAllowed("https://" + OTHER_HOST + "/anything", policy(null)); + } + + @Test + public void blocksInternalAddresses() { + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://169.254.169.254/latest/meta-data/iam/", + anchoredOn("169.254.169.254")); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://100.100.100.200/latest/meta-data/", + anchoredOn("100.100.100.200")); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://127.0.0.1:8983/solr/statistics/select?q=*:*", + anchoredOn("127.0.0.1")); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://10.1.2.3/internal", anchoredOn("10.1.2.3")); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://[fd12:3456::1]/internal", anchoredOn("[fd12:3456::1]")); + } + + @Test + public void allowsAnInternalHostOnlyWhenItIsListed() { + String solr = "http://127.0.0.1:8983/solr/statistics/select"; + assertRejected(RejectionReason.ADDRESS_BLOCKED, solr, anchoredOn("127.0.0.1")); + + config.put(PREFIX + "allowedInternalHosts", new String[] {"127.0.0.1"}); + assertAllowed(solr, anchoredOn("127.0.0.1")); + + // CIDR entries are matched against the resolved address instead of the host name + config.put(PREFIX + "allowedInternalHosts", new String[] {"10.0.0.0/8"}); + assertAllowed("http://10.1.2.3/internal", anchoredOn("10.1.2.3")); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://192.168.1.1/internal", anchoredOn("192.168.1.1")); + } + + @Test + public void strictestPolicyRejectsAnExternalHost() { + assertRejected(RejectionReason.HOST_NOT_ALLOWED, "http://" + OTHER_HOST + "/file.pdf", + OreEgressPolicy.strictest(configuration())); + // a null harvest row must land on the same fail-closed policy + assertRejected(RejectionReason.HOST_NOT_ALLOWED, "http://" + OTHER_HOST + "/file.pdf", + OreEgressPolicy.from(null, configuration())); + } + + @Test + public void rejectsAMissingUrl() { + OreUrlValidator.Decision decision = validator.validate(null, policy(OAI_SOURCE)); + assertFalse(decision.isAllowed()); + assertEquals(RejectionReason.MALFORMED_AUTHORITY, decision.getReason()); + } + + @Test + public void parseRejectsRelativeAndUnparseableUrls() throws Exception { + assertEquals(URI.create("http://8.8.8.8/x"), OreUrlValidator.parse("http://8.8.8.8/x")); + + OreResourceRejectedException relative = + assertThrows(OreResourceRejectedException.class, () -> OreUrlValidator.parse("/bitstream/1")); + assertEquals(RejectionReason.SCHEME_NOT_ALLOWED, relative.getReason()); + + OreResourceRejectedException unparseable = + assertThrows(OreResourceRejectedException.class, () -> OreUrlValidator.parse("http://8.8.8.8/a b")); + assertEquals(RejectionReason.MALFORMED_AUTHORITY, unparseable.getReason()); + } + + private void assertRejected(RejectionReason expected, String url, OreEgressPolicy policy) { + OreUrlValidator.Decision decision = validator.validate(URI.create(url), policy); + assertFalse(url + " must be rejected", decision.isAllowed()); + assertEquals(url, expected, decision.getReason()); + } + + private void assertAllowed(String url, OreEgressPolicy policy) { + OreUrlValidator.Decision decision = validator.validate(URI.create(url), policy); + assertTrue(url + " must be allowed, was rejected as " + decision.getReason(), decision.isAllowed()); + assertFalse("an allowed URL must carry the addresses it was validated against", + decision.getAddresses().isEmpty()); + } + + /** A policy confined to the given oai_source. */ + private OreEgressPolicy policy(String oaiSource) { + HarvestedCollection harvestRow = mock(HarvestedCollection.class); + when(harvestRow.getOaiSource()).thenReturn(oaiSource); + return OreEgressPolicy.from(harvestRow, configuration()); + } + + /** A policy whose oai_source confines to exactly {@code host}, so the address checks are what a test exercises. */ + private OreEgressPolicy anchoredOn(String host) { + return policy("http://" + host + "/oai/request"); + } + + /** + * Answers with whatever default the caller passes in, so the tests run against the shipped defaults and + * only the entries a test puts in {@link #config} differ. + */ + private ConfigurationService configuration() { + return mock(ConfigurationService.class, invocation -> { + Object[] arguments = invocation.getArguments(); + if (arguments.length == 0) { + return null; + } + Object value = config.get(arguments[0]); + if (value != null) { + return value; + } + return arguments.length > 1 ? arguments[1] : null; + }); + } +} diff --git a/dspace-api/src/test/java/org/dspace/harvest/ore/SafeResourceFetcherTest.java b/dspace-api/src/test/java/org/dspace/harvest/ore/SafeResourceFetcherTest.java new file mode 100644 index 000000000000..e494995d2762 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/harvest/ore/SafeResourceFetcherTest.java @@ -0,0 +1,275 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.harvest.ore; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; + +import java.io.IOException; +import java.io.InputStream; +import java.net.InetAddress; +import java.net.URI; +import java.net.UnknownHostException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; +import org.apache.http.config.Registry; +import org.apache.http.config.RegistryBuilder; +import org.apache.http.conn.DnsResolver; +import org.apache.http.conn.socket.ConnectionSocketFactory; +import org.apache.http.conn.socket.PlainConnectionSocketFactory; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.dspace.app.client.DSpaceHttpClientFactory; +import org.dspace.services.ConfigurationService; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.MockedStatic; + +/** + * Unit tests for {@link SafeResourceFetcher}, driven against a {@link MockWebServer}. + */ +public class SafeResourceFetcherTest { + + private static final String PREFIX = "oai.harvester.ore.file."; + + private final SafeResourceFetcher fetcher = new SafeResourceFetcher(); + + private final Map config = new HashMap<>(); + + private MockWebServer server; + + private MockedStatic httpClientFactory; + + @Before + public void setUp() throws IOException { + // the host confinement is always on, so the test hosts have to be allowed explicitly for the address + // and redirect checks to be what the tests actually exercise + config.put(PREFIX + "allowedUrlPrefix", new String[] {"127.0.0.1", "169.254.169.254"}); + // MockWebServer answers on 127.0.0.1, which the predicate blocks, so the tests exempt it explicitly + config.put(PREFIX + "allowedInternalHosts", new String[] {"127.0.0.1"}); + server = new MockWebServer(); + server.start(InetAddress.getByName("127.0.0.1"), 0); + httpClientFactory = mockStatic(DSpaceHttpClientFactory.class); + httpClientFactory.when(() -> DSpaceHttpClientFactory.getInstance()).thenReturn(new PlaintextClientFactory()); + } + + @After + public void tearDown() throws IOException { + httpClientFactory.close(); + server.shutdown(); + } + + @Test + public void returnsTheBodyOfAnAllowedUrl() throws Exception { + server.enqueue(new MockResponse().setBody("aggregated bitstream")); + try (InputStream body = fetcher.fetch(url("/bitstreams/1"), policy())) { + assertEquals("aggregated bitstream", new String(body.readAllBytes(), UTF_8)); + } + RecordedRequest request = server.takeRequest(); + assertEquals("/bitstreams/1", request.getPath()); + } + + @Test + public void rejectsTheServerAddressWhenItIsNotExempted() { + config.remove(PREFIX + "allowedInternalHosts"); + OreResourceRejectedException rejected = + assertThrows(OreResourceRejectedException.class, () -> fetcher.fetch(url("/bitstreams/1"), policy())); + assertEquals(RejectionReason.ADDRESS_BLOCKED, rejected.getReason()); + assertEquals(0, server.getRequestCount()); + } + + @Test + public void rejectsARedirectToTheMetadataService() { + String metadata = "http://169.254.169.254/latest/meta-data/iam/security-credentials/"; + server.enqueue(new MockResponse().setResponseCode(302).addHeader("Location", metadata)); + OreResourceRejectedException rejected = + assertThrows(OreResourceRejectedException.class, () -> fetcher.fetch(url("/ore"), policy())); + assertEquals(RejectionReason.ADDRESS_BLOCKED, rejected.getReason()); + assertEquals(metadata, rejected.getUrl()); + // the hop is judged before it is fetched, so nothing was ever sent to the metadata service + assertEquals(1, server.getRequestCount()); + } + + @Test + public void rejectsARedirectChainOverTheCap() { + config.put(PREFIX + "maxRedirects", 2); + for (int hop = 0; hop < 3; hop++) { + server.enqueue(new MockResponse().setResponseCode(302).addHeader("Location", "/hop" + hop)); + } + OreResourceRejectedException rejected = + assertThrows(OreResourceRejectedException.class, () -> fetcher.fetch(url("/ore"), policy())); + assertEquals(RejectionReason.TOO_MANY_REDIRECTS, rejected.getReason()); + assertEquals(3, server.getRequestCount()); + } + + @Test + public void refusesAnHttpsToHttpDowngrade() { + String downgraded = "http://127.0.0.1:" + server.getPort() + "/plain"; + server.enqueue(new MockResponse().setResponseCode(301).addHeader("Location", downgraded)); + URI secure = URI.create("https://127.0.0.1:" + server.getPort() + "/secure"); + OreResourceRejectedException rejected = + assertThrows(OreResourceRejectedException.class, () -> fetcher.fetch(secure, policy())); + assertEquals(RejectionReason.REDIRECT_DOWNGRADE, rejected.getReason()); + assertEquals(downgraded, rejected.getUrl()); + assertEquals(1, server.getRequestCount()); + } + + @Test + public void rejectsADeclaredLengthOverTheCap() { + config.put(PREFIX + "maxBytes", 16L); + server.enqueue(new MockResponse().setBody(body(4096))); + OreResourceRejectedException rejected = + assertThrows(OreResourceRejectedException.class, () -> fetcher.fetch(url("/big"), policy())); + assertEquals(RejectionReason.RESPONSE_TOO_LARGE, rejected.getReason()); + } + + @Test + public void rejectsABodyThatOutgrowsTheCapWhileStreaming() throws Exception { + config.put(PREFIX + "maxBytes", 16L); + server.enqueue(new MockResponse().setChunkedBody(body(4096), 512)); + try (InputStream body = fetcher.fetch(url("/big"), policy())) { + assertThrows(SafeResourceFetcher.ResponseTooLargeException.class, () -> body.readAllBytes()); + } + } + + @Test + public void rejectsABodyThatLiesAboutItsLength() throws Exception { + config.put(PREFIX + "maxBytes", 16L); + // chunked framing wins over the header, so the declared length never gets the chance to be trusted + server.enqueue(new MockResponse().setChunkedBody(body(4096), 512).setHeader("Content-Length", "4")); + try (InputStream body = fetcher.fetch(url("/liar"), policy())) { + assertThrows(SafeResourceFetcher.ResponseTooLargeException.class, () -> body.readAllBytes()); + } + } + + @Test + public void pinsTheConnectionToTheValidatedAddressAndDoesNotReResolve() throws Exception { + // the validator vets the host once; the connection must then target that exact address rather than + // look the host up a second time, which is what defeats DNS rebinding between check and connect + RecordingDnsResolver dns = new RecordingDnsResolver(); + httpClientFactory.when(() -> DSpaceHttpClientFactory.getInstance()) + .thenReturn(new PinningProbeClientFactory(dns)); + server.enqueue(new MockResponse().setBody("pinned")); + + try (InputStream body = fetcher.fetch(url("/pin"), policy())) { + assertEquals("pinned", new String(body.readAllBytes(), UTF_8)); + } + + // a regression such as dropping the explicit port in pinnedHost makes DefaultRoutePlanner rebuild the + // target by name, which sends the connect back through the resolver; a pinned address never does + assertTrue("the connection must not re-resolve the pinned host, but it looked up " + dns.resolvedHosts(), + dns.resolvedHosts().isEmpty()); + } + + private URI url(String path) { + return URI.create("http://127.0.0.1:" + server.getPort() + path); + } + + private static String body(int length) { + return "x".repeat(length); + } + + /** + * No harvest context, so the address checks have to hold on their own; the test hosts are permitted + * through allowedUrlPrefix in {@link #setUp()}. + */ + private OreEgressPolicy policy() { + return OreEgressPolicy.strictest(configuration()); + } + + /** + * Answers with whatever default the caller passes in, so the tests run against the shipped defaults and + * only the entries a test puts in {@link #config} differ. + */ + private ConfigurationService configuration() { + return mock(ConfigurationService.class, invocation -> { + Object[] arguments = invocation.getArguments(); + if (arguments.length == 0) { + return null; + } + Object value = config.get(arguments[0]); + if (value != null) { + return value; + } + return arguments.length > 1 ? arguments[1] : null; + }); + } + + /** + * Stands in for the Spring-managed factory. It also serves https over a plaintext socket, so the redirect + * rules can be exercised against MockWebServer without a test certificate authority. + */ + private static class PlaintextClientFactory extends DSpaceHttpClientFactory { + + @Override + public HttpClientBuilder builder(boolean setProxy) { + Registry plaintext = RegistryBuilder.create() + .register("http", PlainConnectionSocketFactory.getSocketFactory()) + .register("https", PlainConnectionSocketFactory.getSocketFactory()) + .build(); + return HttpClients.custom().setConnectionManager(new PoolingHttpClientConnectionManager(plaintext)); + } + } + + /** + * Like {@link PlaintextClientFactory}, but wires a resolver into the connection manager that records every + * host the client asks it to look up, so a test can prove the connect bypassed it. + */ + private static class PinningProbeClientFactory extends DSpaceHttpClientFactory { + + private final DnsResolver dnsResolver; + + PinningProbeClientFactory(DnsResolver dnsResolver) { + this.dnsResolver = dnsResolver; + } + + @Override + public HttpClientBuilder builder(boolean setProxy) { + Registry plaintext = RegistryBuilder.create() + .register("http", PlainConnectionSocketFactory.getSocketFactory()) + .register("https", PlainConnectionSocketFactory.getSocketFactory()) + .build(); + PoolingHttpClientConnectionManager manager = + new PoolingHttpClientConnectionManager(plaintext, null, null, dnsResolver, -1, TimeUnit.MILLISECONDS); + return HttpClients.custom().setConnectionManager(manager); + } + } + + /** + * Records the hosts it is asked to resolve. It still answers with loopback so that, if a regression does + * route a connect through here, the test fails on the recorded lookup rather than on a connection error. + */ + private static class RecordingDnsResolver implements DnsResolver { + + private final List hosts = Collections.synchronizedList(new ArrayList<>()); + + @Override + public InetAddress[] resolve(String host) throws UnknownHostException { + hosts.add(host); + return new InetAddress[] {InetAddress.getByName("127.0.0.1")}; + } + + List resolvedHosts() { + return hosts; + } + } +} diff --git a/dspace/config/modules/oai.cfg b/dspace/config/modules/oai.cfg index 10409cd10e82..ac7ab2ffb011 100644 --- a/dspace/config/modules/oai.cfg +++ b/dspace/config/modules/oai.cfg @@ -153,9 +153,41 @@ oai.harvester.unknownSchema = fail # this config parameter, a new handle will be minted instead. Default value: 123456789. #oai.harvester.rejectedHandlePrefix = 123456789, myTestHandle -# If ingesting files with ORE, only files with URLs that match the base URL of the remote -# OAI endpoint's domain name are accepted, or a list of other URL prefixes defined below -#oai.harvester.ore.file.validateUrlPrefix = true -# Prefixes that are allowed globally (for any endpoint) are below +# If ingesting files with ORE (harvest type "metadata and bitstreams"), each aggregated file URL +# must share scheme+host with the collection's remote OAI endpoint (its oai_source), or match one +# of the allowedUrlPrefix values below. This host confinement is always enforced and http(s) is the +# only permitted scheme. A prefix is either a bare hostname (any path on that host over any scheme) +# or a scheme://host/path prefix. Repeatable. Default: empty. +# Redirects are re-confined at every hop, so a file URL that redirects to a different host (a CDN or a +# presigned S3 bucket, say) is blocked unless that host is also listed here. #oai.harvester.ore.file.allowedUrlPrefix = dspace.myinstitution.edu -#oai.harvester.ore.file.allowedUrlPrefix = files.myinstitution.edu +#oai.harvester.ore.file.allowedUrlPrefix = https://files.myinstitution.edu/bitstreams +# The METS/SWORD packagers and the XSLT CLI have no collection and therefore no oai_source, so +# allowedUrlPrefix above is the ONLY way to let them fetch ORE files, and it ships empty. This bites +# only for a package that declares OTHERMDTYPE="ore"; DSpace's own AIP disseminator does not emit one. + +# Block URLs resolving to loopback, private, link-local, CGNAT, reserved or cloud-metadata +# addresses. Checked at every redirect hop. Default: true. +# Setting this to false restores the server-side request forgery this check exists to prevent: a +# remote OAI source can then make DSpace fetch any internal host and republish the response. +#oai.harvester.ore.file.blockInternalAddresses = true + +# Hostnames or CIDRs exempted from the check above - the only supported way to harvest files +# from an internal repository. Repeatable. Default: empty. +#oai.harvester.ore.file.allowedInternalHosts = repository.internal.myinstitution.edu +#oai.harvester.ore.file.allowedInternalHosts = 10.1.2.0/24 + +# NOTE: with http.proxy.host configured (dspace.cfg) these ORE fetches go through the proxy, which +# resolves the hostname itself and connects on our behalf. The address checks are then advisory +# only - they describe what DSpace resolved, not what was connected to - so the proxy has to +# enforce its own egress rules. + +# Maximum number of redirect hops; each hop is re-validated in full. Default: 3. +#oai.harvester.ore.file.maxRedirects = 3 + +# Connect and read timeouts for fetching an ORE file, in milliseconds. Defaults: 10000 / 30000. +#oai.harvester.ore.file.connectTimeout = 10000 +#oai.harvester.ore.file.readTimeout = 30000 + +# Maximum size of a single fetched ORE file in bytes; -1 means unlimited. Default: 2 GiB. +#oai.harvester.ore.file.maxBytes = 2147483648