diff --git a/dspace-api/src/main/java/org/dspace/app/harvest/Harvest.java b/dspace-api/src/main/java/org/dspace/app/harvest/Harvest.java index f2630572e36..d5212ee6720 100644 --- a/dspace-api/src/main/java/org/dspace/app/harvest/Harvest.java +++ b/dspace-api/src/main/java/org/dspace/app/harvest/Harvest.java @@ -54,6 +54,8 @@ public class Harvest extends DSpaceRunnable { private String oaiSetID = null; private String metadataKey = null; private int harvestType = 0; + // null means "-x was not given"; as in the REST contract, that leaves an existing setting untouched + private Boolean allowExternalUrls; protected Context context; @@ -116,6 +118,9 @@ public void setup() throws ParseException { if (commandLine.hasOption('m')) { metadataKey = commandLine.getOptionValue('m'); } + if (commandLine.hasOption('x')) { + allowExternalUrls = Boolean.TRUE; + } } /** @@ -146,7 +151,7 @@ public void internalRun() throws Exception { handler.logInfo("PING OAI server: Harvest -g -a oai_source -i oai_set_id"); handler.logInfo( "SETUP a collection for harvesting: Harvest -s -c collection -t harvest_type -a oai_source -i " + - "oai_set_id -m metadata_format"); + "oai_set_id -m metadata_format [-x]"); handler.logInfo("RUN harvest once: Harvest -r -e eperson -c collection"); handler.logInfo("START harvest scheduler: Harvest -S"); handler.logInfo("RESET all harvest status: Harvest -R"); @@ -227,7 +232,8 @@ public void internalRun() throws Exception { "A metadata key (commonly the prefix) must be specified for this collection"); } - configureCollection(context, collection, harvestType, oaiSource, oaiSetID, metadataKey); + configureCollection(context, collection, harvestType, oaiSource, oaiSetID, metadataKey, + allowExternalUrls); } else if ("ping".equals(command)) { if (oaiSource == null || oaiSetID == null) { handler.logError( @@ -287,7 +293,7 @@ private Collection resolveCollection(Context context, String collectionID) { private void configureCollection(Context context, String collectionID, int type, String oaiSource, String oaiSetId, - String mdConfigId) { + String mdConfigId, Boolean allowExternalUrls) { handler.logInfo("Running: configure collection"); Collection collection = resolveCollection(context, collectionID); @@ -301,6 +307,9 @@ private void configureCollection(Context context, String collectionID, int type, context.turnOffAuthorisationSystem(); hc.setHarvestParams(type, oaiSource, oaiSetId, mdConfigId); + if (allowExternalUrls != null) { + hc.setAllowExternalUrls(allowExternalUrls); + } hc.setHarvestStatus(HarvestedCollection.STATUS_READY); harvestedCollectionService.update(context, hc); context.restoreAuthSystemState(); diff --git a/dspace-api/src/main/java/org/dspace/app/harvest/HarvestScriptConfiguration.java b/dspace-api/src/main/java/org/dspace/app/harvest/HarvestScriptConfiguration.java index ff83c3ecb22..8186a22e894 100644 --- a/dspace-api/src/main/java/org/dspace/app/harvest/HarvestScriptConfiguration.java +++ b/dspace-api/src/main/java/org/dspace/app/harvest/HarvestScriptConfiguration.java @@ -48,6 +48,9 @@ public Options getOptions() { options.addOption("m", "metadata_format", true, "the name of the desired metadata format for harvesting, resolved to namespace and " + "crosswalk in dspace.cfg"); + options.addOption("x", "allow-external-urls", false, + "allow ORE ingest to fetch files from hosts other than the OAI-PMH server; " + + "internal and private addresses remain blocked"); options.addOption("h", "help", false, "help"); 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 2372615e6dc..77cccc1f2c0 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,11 +7,8 @@ */ package org.dspace.content.crosswalk; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.net.ConnectException; -import java.net.URL; import java.sql.SQLException; import java.text.NumberFormat; import java.time.Instant; @@ -20,6 +17,7 @@ import java.util.List; 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; @@ -34,6 +32,13 @@ 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.factory.DSpaceServicesFactory; import org.jdom2.Attribute; import org.jdom2.Document; import org.jdom2.Element; @@ -50,7 +55,7 @@ * @author Alexey Maslov */ public class OREIngestionCrosswalk - implements IngestionCrosswalk { + implements IngestionCrosswalk, HarvestPolicyAware { /** * log4j category */ @@ -77,6 +82,25 @@ public class OREIngestionCrosswalk protected BundleService bundleService = ContentServiceFactory.getInstance().getBundleService(); protected ItemService itemService = ContentServiceFactory.getInstance().getItemService(); + 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(DSpaceServicesFactory.getInstance().getConfigurationService()); + } + return oreEgressPolicy; + } @Override public void ingest(Context context, DSpaceObject dso, List metadata, boolean createMissingMetadataFields) @@ -126,7 +150,8 @@ public void ingest(Context context, DSpaceObject dso, Element root, boolean crea XPathFactory.instance() .compile("/atom:entry/atom:link[@rel='alternate']/@href", Filters.attribute(), null, ATOM_NS); - entryId = xpathAltHref.evaluateFirst(doc).getValue(); + Attribute entryIdAttribute = xpathAltHref.evaluateFirst(doc); + entryId = entryIdAttribute == null ? null : entryIdAttribute.getValue(); // Next for each resource, create a bitstream NumberFormat nf = NumberFormat.getInstance(); @@ -146,8 +171,11 @@ public void ingest(Context context, DSpaceObject dso, Element root, boolean crea Filters.element(), null, ATOM_NS, ORE_ATOM, RDF_NS); desc = xpathDesc.evaluateFirst(doc); - if (desc != null && desc.getChild("type", RDF_NS).getAttributeValue("resource", RDF_NS) - .equals(DS_NS.getURI() + "DSpaceBitstream")) { + // the harvested document need not carry an , so neither may be dereferenced + Element descType = desc == null ? null : desc.getChild("type", RDF_NS); + String descTypeResource = descType == null ? null : descType.getAttributeValue("resource", RDF_NS); + + if ((DS_NS.getURI() + "DSpaceBitstream").equals(descTypeResource)) { bundleName = desc.getChildText("description", DCTERMS_NS); log.debug("Setting bundle name to: " + bundleName); } else { @@ -167,27 +195,35 @@ 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); - // Generate a request for the aggregated resource - ARurl = new URL(processedURL); - in = ARurl.openStream(); - } 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); + // 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. + in = resourceFetcher.fetch(OreUrlValidator.parse(processedURL), egressPolicy()); + } catch (IOException ioe) { + // a transport failure has to drop this record only; escaping here would stop the whole + // collection, and after removeAllBundles it would commit the item without its files + throw transferFailed(href, ioe); } } else { - throw new CrosswalkException("Entry did not contain link to resource: " + entryId); + // entryId is absent when the record carries no rel="alternate" link, so name it only if we have it + throw new CrosswalkException(entryId == null ? "Entry did not contain link to resource" + : "Entry did not contain link to resource: " + entryId); } // ingest and update if (in != null) { - Bitstream newBitstream = bitstreamService.create(context, targetBundle, in); + Bitstream newBitstream; + try { + newBitstream = bitstreamService.create(context, targetBundle, in); + } catch (IOException ioe) { + // the body is only read here, and the bitstore wraps everything it catches in a plain + // IOException, so both the size cap and a mid-transfer failure surface at this point + throw transferFailed(href, ioe); + } String bsName = resource.getAttributeValue("title"); newBitstream.setName(context, bsName); @@ -214,6 +250,22 @@ public void ingest(Context context, DSpaceObject dso, Element root, boolean crea } + /** + * 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 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); + } + /** * Helper method to escape all characters that are not part of the canon set * diff --git a/dspace-api/src/main/java/org/dspace/harvest/HarvestedCollection.java b/dspace-api/src/main/java/org/dspace/harvest/HarvestedCollection.java index 40e4dfe8345..fd8b3de537a 100644 --- a/dspace-api/src/main/java/org/dspace/harvest/HarvestedCollection.java +++ b/dspace-api/src/main/java/org/dspace/harvest/HarvestedCollection.java @@ -65,6 +65,10 @@ public class HarvestedCollection implements ReloadableEntity { @Column(name = "last_harvested", columnDefinition = "timestamp with time zone") private Instant lastHarvested; + // Widens the hosts an ORE ingest may fetch files from; internal addresses stay blocked either way. + @Column(name = "allow_external_urls") + private boolean allowExternalUrls; + @Transient public static final int TYPE_NONE = 0; @Transient @@ -165,6 +169,15 @@ public void setHarvestStartTime(Instant date) { this.harvestStartTime = date; } + /** + * Allows ORE ingest to fetch files from hosts other than the one hosting the OAI source. + * + * @param allowExternalUrls true to skip the host confinement check + */ + public void setAllowExternalUrls(boolean allowExternalUrls) { + this.allowExternalUrls = allowExternalUrls; + } + /* Getting for the appropriate harvesting-related columns */ public Collection getCollection() { @@ -206,4 +219,8 @@ public Instant getHarvestDate() { public Instant getHarvestStartTime() { return harvestStartTime; } + + public boolean isAllowExternalUrls() { + return allowExternalUrls; + } } 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 34770f45f9f..49c8cd2274b 100644 --- a/dspace-api/src/main/java/org/dspace/harvest/OAIHarvester.java +++ b/dspace-api/src/main/java/org/dspace/harvest/OAIHarvester.java @@ -62,6 +62,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; @@ -128,6 +132,12 @@ public class OAIHarvester { private Namespace metadataNS; private String metadataKey; + // 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; + // DOMbuilder class for the DOM -> JDOM conversions private static final DOMBuilder db = new DOMBuilder(); // The point at which this thread should terminate itself @@ -445,12 +455,33 @@ public void runHarvest() throws SQLException, IOException, AuthorizeException { Instant finishTime = Instant.now(); long timeTaken = finishTime.toEpochMilli() - startTime.toEpochMilli(); harvestRow.setHarvestStartTime(startTime); - harvestRow.setHarvestMessage("Harvest from " + oaiSource + " successful"); - harvestRow.setHarvestStatus(HarvestedCollection.STATUS_READY); - harvestRow.setLastHarvested(startTime); + if (rejectedRecords + failedRecords > 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(rejectedRecords + failedRecords).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); + } + int skippedRecords = rejectedRecords + failedRecords; 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); @@ -523,6 +554,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 @@ -561,7 +596,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 @@ -580,7 +622,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 @@ -662,6 +709,29 @@ protected void processRecord(Element record, String OREPrefix, final long curren ourContext.restoreAuthSystemState(); } + /** + * 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(), ore.getUrl(), harvestRow.getOaiSource(), ore.getReason()); + ourContext.restoreAuthSystemState(); + ourContext.rollback(); + reloadRequiredEntities(); + } + /** * Scan an item's metadata, looking for the value "identifier.*". If it meets the parameters that identify it as 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 00000000000..d78dbc58427 --- /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 00000000000..d64c8b13309 --- /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 00000000000..890c868fc03 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/harvest/ore/OreEgressPolicy.java @@ -0,0 +1,146 @@ +/** + * 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. + *

+ * Only {@link #isAllowExternalUrls()} is per collection; it widens the set of permitted public hosts and + * nothing else. Internal, private and reserved addresses stay blocked in both states. + */ +public final class OreEgressPolicy { + + static final String CONFIG_PREFIX = "oai.harvester.ore.file."; + + private final boolean allowExternalUrls; + 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(boolean allowExternalUrls, URI anchor, ConfigurationService configurationService) { + this.allowExternalUrls = allowExternalUrls; + 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 and the flag off, 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(false, 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(harvestRow.isAllowExternalUrls(), toUri(harvestRow.getOaiSource()), + configurationService); + } + + public boolean isAllowExternalUrls() { + return allowExternalUrls; + } + + /** + * @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 00000000000..25e16e439e5 --- /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 00000000000..a28d3584403 --- /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 only step the per-collection flag governs + if (!policy.isAllowExternalUrls() && !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, whatever the flag says + 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 00000000000..d135253ac04 --- /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 00000000000..93320b68e10 --- /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/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V9.1_2026.07.31__harvested_collection_allow_external_urls.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V9.1_2026.07.31__harvested_collection_allow_external_urls.sql new file mode 100644 index 00000000000..bedf3725a78 --- /dev/null +++ b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V9.1_2026.07.31__harvested_collection_allow_external_urls.sql @@ -0,0 +1,17 @@ +-- +-- 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/ +-- + +----------------------------------------------------------------------------------- +-- Alter harvested_collection table +----------------------------------------------------------------------------------- + +ALTER TABLE harvested_collection ADD COLUMN allow_external_urls BOOLEAN DEFAULT FALSE NOT NULL; + +-- Existing "metadata and bitstreams" harvests already fetch files cross-host; keep them working. +-- Internal/private addresses are blocked regardless of this flag. +UPDATE harvested_collection SET allow_external_urls = TRUE WHERE harvest_type = 3; diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V9.1_2026.07.31__harvested_collection_allow_external_urls.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V9.1_2026.07.31__harvested_collection_allow_external_urls.sql new file mode 100644 index 00000000000..bedf3725a78 --- /dev/null +++ b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V9.1_2026.07.31__harvested_collection_allow_external_urls.sql @@ -0,0 +1,17 @@ +-- +-- 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/ +-- + +----------------------------------------------------------------------------------- +-- Alter harvested_collection table +----------------------------------------------------------------------------------- + +ALTER TABLE harvested_collection ADD COLUMN allow_external_urls BOOLEAN DEFAULT FALSE NOT NULL; + +-- Existing "metadata and bitstreams" harvests already fetch files cross-host; keep them working. +-- Internal/private addresses are blocked regardless of this flag. +UPDATE harvested_collection SET allow_external_urls = TRUE WHERE harvest_type = 3; 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 00000000000..85b06f94525 --- /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 00000000000..44606e89108 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/harvest/ore/OreUrlValidatorTest.java @@ -0,0 +1,205 @@ +/** + * 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(true, 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(true, OAI_SOURCE)); + } + + @Test + public void rejectsCredentialsInTheAuthority() { + OreEgressPolicy policy = policy(true, 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() { + OreEgressPolicy policy = policy(true, OAI_SOURCE); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://2130706433/", policy); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://[::ffff:7f00:1]/", policy); + assertRejected(RejectionReason.MALFORMED_AUTHORITY, "http://127.1/", policy); + assertRejected(RejectionReason.MALFORMED_AUTHORITY, "http://0x7f.0x0.0x0.0x1/", policy); + } + + @Test + public void blocksADifferentHostWhenExternalUrlsAreOff() { + assertRejected(RejectionReason.HOST_NOT_ALLOWED, "http://" + OTHER_HOST + "/file.pdf", + policy(false, OAI_SOURCE)); + } + + @Test + public void allowsADifferentHostWhenExternalUrlsAreOn() { + assertAllowed("http://" + OTHER_HOST + "/file.pdf", policy(true, OAI_SOURCE)); + } + + @Test + public void allowsTheOaiSourceHostWhenExternalUrlsAreOff() { + OreEgressPolicy policy = policy(false, 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(false, "https://8.8.8.8/oai/request")); + } + + @Test + public void allowsAHostListedInAllowedUrlPrefix() { + config.put(PREFIX + "allowedUrlPrefix", new String[] {"http://" + OTHER_HOST + "/files"}); + OreEgressPolicy policy = policy(false, 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(false, null)); + } + + @Test + public void blocksInternalAddressesEvenWhenExternalUrlsAreOn() { + OreEgressPolicy policy = policy(true, OAI_SOURCE); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://169.254.169.254/latest/meta-data/iam/", policy); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://100.100.100.200/latest/meta-data/", policy); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://127.0.0.1:8983/solr/statistics/select?q=*:*", policy); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://10.1.2.3/internal", policy); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://[fd12:3456::1]/internal", policy); + } + + @Test + public void allowsAnInternalHostOnlyWhenItIsListed() { + String solr = "http://127.0.0.1:8983/solr/statistics/select"; + assertRejected(RejectionReason.ADDRESS_BLOCKED, solr, policy(true, OAI_SOURCE)); + + config.put(PREFIX + "allowedInternalHosts", new String[] {"127.0.0.1"}); + assertAllowed(solr, policy(true, OAI_SOURCE)); + + // 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", policy(true, OAI_SOURCE)); + assertRejected(RejectionReason.ADDRESS_BLOCKED, "http://192.168.1.1/internal", policy(true, OAI_SOURCE)); + } + + @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(true, 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()); + } + + private OreEgressPolicy policy(boolean allowExternalUrls, String oaiSource) { + HarvestedCollection harvestRow = mock(HarvestedCollection.class); + when(harvestRow.isAllowExternalUrls()).thenReturn(allowExternalUrls); + when(harvestRow.getOaiSource()).thenReturn(oaiSource); + return OreEgressPolicy.from(harvestRow, 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; + }); + } +} 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 00000000000..3cc468c20f5 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/harvest/ore/SafeResourceFetcherTest.java @@ -0,0 +1,207 @@ +/** + * 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.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.io.InputStream; +import java.net.InetAddress; +import java.net.URI; +import java.util.HashMap; +import java.util.Map; + +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.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.harvest.HarvestedCollection; +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 { + // 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()); + } + } + + 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); + } + + /** + * External URLs are allowed, which is the state in which the address checks have to hold on their own. + */ + private OreEgressPolicy policy() { + HarvestedCollection harvestRow = mock(HarvestedCollection.class); + when(harvestRow.isAllowExternalUrls()).thenReturn(true); + when(harvestRow.getOaiSource()).thenReturn(null); + return OreEgressPolicy.from(harvestRow, 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)); + } + } +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/HarvestedCollectionConverter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/HarvestedCollectionConverter.java index b77f783a3ed..bc447d3d650 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/HarvestedCollectionConverter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/HarvestedCollectionConverter.java @@ -53,8 +53,11 @@ public HarvestedCollectionRest convert(HarvestedCollection obj, Projection proje harvestedCollectionRest.setHarvestMessage(obj.getHarvestMessage()); harvestedCollectionRest.setHarvestStartTime(obj.getHarvestStartTime()); harvestedCollectionRest.setLastHarvested(obj.getHarvestDate()); + harvestedCollectionRest.setAllowExternalUrls(obj.isAllowExternalUrls()); } else { harvestedCollectionRest.setHarvestType(HarvestTypeEnum.NONE); + // Not harvesting (yet): report the secure default rather than null, so the UI can bind a checkbox. + harvestedCollectionRest.setAllowExternalUrls(Boolean.FALSE); } return harvestedCollectionRest; diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/HarvestedCollectionRest.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/HarvestedCollectionRest.java index 1eb8210874e..3ffd8c1fb38 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/HarvestedCollectionRest.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/HarvestedCollectionRest.java @@ -48,6 +48,10 @@ public class HarvestedCollectionRest extends BaseObjectRest { @JsonProperty("last_harvested") private Instant lastHarvested; + // Nullable on purpose: absent in a PUT body means "leave the stored value unchanged". + @JsonProperty("allow_external_urls") + private Boolean allowExternalUrls; + private HarvesterMetadataRest metadata_configs; private CollectionRest collectionRest; @@ -162,6 +166,14 @@ public void setLastHarvested(Instant lastHarvested) { this.lastHarvested = lastHarvested; } + public Boolean getAllowExternalUrls() { + return allowExternalUrls; + } + + public void setAllowExternalUrls(Boolean allowExternalUrls) { + this.allowExternalUrls = allowExternalUrls; + } + @LinkRest(name = "harvestermetadata") @JsonIgnore public HarvesterMetadataRest getMetadataConfigs() { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/HarvestedCollectionRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/HarvestedCollectionRestRepository.java index 3855d24452d..0215fb8a530 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/HarvestedCollectionRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/HarvestedCollectionRestRepository.java @@ -138,11 +138,16 @@ private void updateCollectionHarvestSettings(Context context, HarvestedCollectio String oaiSource = harvestedCollectionRest.getOaiSource(); String oaiSetId = harvestedCollectionRest.getOaiSetId(); String metadataConfigId = harvestedCollectionRest.getMetadataConfigId(); + Boolean allowExternalUrls = harvestedCollectionRest.getAllowExternalUrls(); harvestedCollection.setHarvestType(harvestType); harvestedCollection.setOaiSource(oaiSource); harvestedCollection.setOaiSetId(oaiSetId); harvestedCollection.setHarvestMetadataConfig(metadataConfigId); + // Absent from the request body means "leave unchanged", so only apply an explicit value. + if (allowExternalUrls != null) { + harvestedCollection.setAllowExternalUrls(allowExternalUrls); + } harvestedCollectionService.update(context, harvestedCollection); } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionHarvestSettingsControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionHarvestSettingsControllerIT.java index e7479786e7b..d6051f248b3 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionHarvestSettingsControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionHarvestSettingsControllerIT.java @@ -10,6 +10,7 @@ import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -99,11 +100,28 @@ public void SetUp() throws SQLException, AuthorizeException { */ public JSONObject createHarvestSettingsJson(String harvestType, String oaiSource, String oaiSetId, String metadataConfigId) { + return createHarvestSettingsJson(harvestType, oaiSource, oaiSetId, metadataConfigId, null); + } + + /** + * Function to create a JSONObject containing the harvest settings + * @param harvestType The harvest type + * @param oaiSource The OAI source + * @param oaiSetId The OAI set id + * @param metadataConfigId The metadata config id + * @param allowExternalUrls Whether external URLs are allowed; null omits the key from the body + * @return A JSONObject containing the given harvest settings + */ + public JSONObject createHarvestSettingsJson(String harvestType, String oaiSource, String oaiSetId, + String metadataConfigId, Boolean allowExternalUrls) { JSONObject json = new JSONObject(); json.put("harvest_type", harvestType); json.put("oai_source", oaiSource); json.put("oai_set_id", oaiSetId); json.put("metadata_config_id", metadataConfigId); + if (allowExternalUrls != null) { + json.put("allow_external_urls", allowExternalUrls.booleanValue()); + } return json; } @@ -137,6 +155,7 @@ public void GetCollectionHarvestSettings() throws Exception { .andExpect(jsonPath("$.harvest_status", is("READY"))) .andExpect(jsonPath("$.harvest_start_time", is(nullValue()))) .andExpect(jsonPath("$.last_harvested", is(nullValue()))) + .andExpect(jsonPath("$.allow_external_urls", is(false))) .andExpect(jsonPath("$._links.self.href", endsWith("api/core/collections/" + collection.getID() + "/harvester"))) .andExpect(jsonPath("$._embedded.harvestermetadata", Matchers.allOf( @@ -219,6 +238,7 @@ public void getCollectionHarvestSettingsIfNotSet() throws Exception { .andExpect(jsonPath("$.harvest_status", is(nullValue()))) .andExpect(jsonPath("$.harvest_start_time", is(nullValue()))) .andExpect(jsonPath("$.last_harvested", is(nullValue()))) + .andExpect(jsonPath("$.allow_external_urls", is(false))) .andExpect(jsonPath("$._links.self.href", endsWith("api/core/collections/" + collectionNoHarvestSettings.getID() + "/harvester"))) .andExpect(jsonPath("$._embedded.harvestermetadata", Matchers.allOf( @@ -250,6 +270,93 @@ public void PutWorksWithStandardSettings() throws Exception { assertTrue(harvestedCollection.getHarvestMetadataConfig().equals(json.getString("metadata_config_id"))); } + @Test + public void PutAllowExternalUrlsTrueIsStoredAndReturned() throws Exception { + String token = getAuthToken(admin.getEmail(), password); + + JSONObject json = createHarvestSettingsJson("METADATA_ONLY", "https://dspace.org/oai/request", + "col_1721.1_114174", "dc", true); + + getClient(token).perform( + put("/api/core/collections/" + collection.getID() + "/harvester") + .contentType("application/json") + .content(json.toString())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.allow_external_urls", is(true))); + + getClient(token).perform( + get("/api/core/collections/" + collection.getID() + "/harvester")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.allow_external_urls", is(true))); + + assertTrue(harvestedCollectionService.find(context, collection).isAllowExternalUrls()); + } + + @Test + public void PutWithoutAllowExternalUrlsLeavesStoredValueUnchanged() throws Exception { + String token = getAuthToken(admin.getEmail(), password); + + // Seed the flag through the endpoint, there is no HarvestedCollectionBuilder. + JSONObject withFlag = createHarvestSettingsJson("METADATA_ONLY", "https://dspace.org/oai/request", + "col_1721.1_114174", "dc", true); + + getClient(token).perform( + put("/api/core/collections/" + collection.getID() + "/harvester") + .contentType("application/json") + .content(withFlag.toString())) + .andExpect(status().isOk()); + + // A body without the key must not silently reset the flag. + JSONObject withoutFlag = createHarvestSettingsJson("METADATA_ONLY", "https://dspace.org/oai/request", + "col_1721.1_114174", "dc"); + + getClient(token).perform( + put("/api/core/collections/" + collection.getID() + "/harvester") + .contentType("application/json") + .content(withoutFlag.toString())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.allow_external_urls", is(true))); + + getClient(token).perform( + get("/api/core/collections/" + collection.getID() + "/harvester")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.allow_external_urls", is(true))); + + assertTrue(harvestedCollectionService.find(context, collection).isAllowExternalUrls()); + } + + @Test + public void PutAllowExternalUrlsFalseIsStored() throws Exception { + String token = getAuthToken(admin.getEmail(), password); + + JSONObject flagOn = createHarvestSettingsJson("METADATA_ONLY", "https://dspace.org/oai/request", + "col_1721.1_114174", "dc", true); + + getClient(token).perform( + put("/api/core/collections/" + collection.getID() + "/harvester") + .contentType("application/json") + .content(flagOn.toString())) + .andExpect(status().isOk()); + + // An explicit false must overwrite, unlike an absent key. + JSONObject flagOff = createHarvestSettingsJson("METADATA_ONLY", "https://dspace.org/oai/request", + "col_1721.1_114174", "dc", false); + + getClient(token).perform( + put("/api/core/collections/" + collection.getID() + "/harvester") + .contentType("application/json") + .content(flagOff.toString())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.allow_external_urls", is(false))); + + getClient(token).perform( + get("/api/core/collections/" + collection.getID() + "/harvester")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.allow_external_urls", is(false))); + + assertFalse(harvestedCollectionService.find(context, collection).isAllowExternalUrls()); + } + @Test public void PutUnProcessableEntityIfIncorrectSettings() throws Exception { String token = getAuthToken(admin.getEmail(), password); diff --git a/dspace/config/modules/oai.cfg b/dspace/config/modules/oai.cfg index 2ac7e79d193..0922452ce00 100644 --- a/dspace/config/modules/oai.cfg +++ b/dspace/config/modules/oai.cfg @@ -132,3 +132,45 @@ oai.harvester.unknownSchema = fail # when attempting to find the handle of harvested items. If there is a match with # 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 +#oai.harvester.ore.file.allowedUrlPrefix = dspace.myinstitution.edu +#oai.harvester.ore.file.allowedUrlPrefix = files.myinstitution.edu + +# validateUrlPrefix above is superseded by the per-collection "allow external URLs" flag +# (harvested_collection.allow_external_urls) and is no longer read. +# Flag OFF (default): an ORE file URL must share scheme+host with the collection's oai_source, +# or match one of the allowedUrlPrefix values above. +# Flag ON: that host check is skipped. Everything below applies in BOTH states. +# The METS/SWORD packagers and the XSLT CLI have no collection and therefore no flag, 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. +#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