diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 94c9133..70eae45 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,5 +1,5 @@ default: - image: gradle:8.4-jdk11 + image: gradle:8.6-jdk11 # Explicit version of the Mergerequests-Pipelines workflow, with the main branch # added. diff --git a/build.gradle b/build.gradle index e13254a..a93eb97 100644 --- a/build.gradle +++ b/build.gradle @@ -53,7 +53,7 @@ dependencies { implementation 'org.springdoc:springdoc-openapi-ui:1.7.0' runtimeOnly 'io.micrometer:micrometer-registry-prometheus' - implementation 'org.postgresql:postgresql:42.6.0' + implementation 'org.postgresql:postgresql:42.7.2' runtimeOnly 'org.springframework.boot:spring-boot-starter-tomcat' implementation 'com.google.code.gson:gson:2.10.1' diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar index 7f93135..d64cd49 100644 Binary files a/gradle/wrapper/gradle-wrapper.jar and b/gradle/wrapper/gradle-wrapper.jar differ diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 3fa8f86..a80b22c 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.6-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME diff --git a/gradlew.bat b/gradlew.bat index 6689b85..7101f8e 100644 --- a/gradlew.bat +++ b/gradlew.bat @@ -43,11 +43,11 @@ set JAVA_EXE=java.exe %JAVA_EXE% -version >NUL 2>&1 if %ERRORLEVEL% equ 0 goto execute -echo. -echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH. -echo. -echo Please set the JAVA_HOME variable in your environment to match the -echo location of your Java installation. +echo. 1>&2 +echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH. 1>&2 +echo. 1>&2 +echo Please set the JAVA_HOME variable in your environment to match the 1>&2 +echo location of your Java installation. 1>&2 goto fail @@ -57,11 +57,11 @@ set JAVA_EXE=%JAVA_HOME%/bin/java.exe if exist "%JAVA_EXE%" goto execute -echo. -echo ERROR: JAVA_HOME is set to an invalid directory: %JAVA_HOME% -echo. -echo Please set the JAVA_HOME variable in your environment to match the -echo location of your Java installation. +echo. 1>&2 +echo ERROR: JAVA_HOME is set to an invalid directory: %JAVA_HOME% 1>&2 +echo. 1>&2 +echo Please set the JAVA_HOME variable in your environment to match the 1>&2 +echo location of your Java installation. 1>&2 goto fail diff --git a/src/main/java/net/ripe/rpki/rest/service/CaRoaConfigurationService.java b/src/main/java/net/ripe/rpki/rest/service/CaRoaConfigurationService.java index 1ef737d..e798be7 100644 --- a/src/main/java/net/ripe/rpki/rest/service/CaRoaConfigurationService.java +++ b/src/main/java/net/ripe/rpki/rest/service/CaRoaConfigurationService.java @@ -185,13 +185,20 @@ public ResponseEntity stageRoaChanges(@PathVariable("caName") final CaName ca final Set currentRoutes = new HashSet<>(currentRoaConfiguration.toAllowedRoutes()); final Set futureRoutes = new HashSet<>(futureRoas.size()); + IpResourceSet affectedRanges; try { + // This will fill up futureRoutes as well affectedRanges = buildAffectedRanges(futureRoas, futureRoutes, currentRoutes); } catch (IllegalArgumentException e) { return ResponseEntity.status(BAD_REQUEST).body(Map.of(ERROR, e.getMessage())); } + Optional validationError = Roas.validateRoaUpdate(futureRoutes); + if (validationError.isPresent()) { + return ResponseEntity.status(BAD_REQUEST).body(Map.of(ERROR, validationError.get())); + } + final List bgpAnnouncementChanges = getBgpAnnouncementChanges( ca, currentRoutes, futureRoutes, bgpAnnouncements, affectedRanges); @@ -210,26 +217,9 @@ private IpResourceSet buildAffectedRanges(List futureRoas, Set> futureRoaMap = Utils.makeROAMap(futureRoas); - Optional e = Utils.validateUniqueROAs("Error in future ROAs", futureRoaMap); - if (e.isPresent()) { - throw new IllegalArgumentException(e.get()); - } - for (AllowedRoute route : currentRoutes) { if (!futureRoutes.contains(route)) affectedRanges.add(route.getPrefix()); - - final AnnouncedRoute key = new AnnouncedRoute(route.getAsn(), route.getPrefix()); - if (futureRoaMap.containsKey(key)) { - futureRoaMap.get(key).forEach(futureMaxLength -> { - if (!Objects.equals(futureMaxLength, route.getMaximumLength())) { - // the same ASN+prefix and different max length - throw new IllegalArgumentException(Utils.getSameROAErrorMessage(route, key, futureMaxLength)); - } - }); - } } return affectedRanges; } @@ -299,9 +289,10 @@ public ResponseEntity publishROAs(@PathVariable("caName") final CaName caName final String ifMatch = StringUtils.defaultIfEmpty(ifMatchHeader, publishSet.getIfMatch()); try { - Utils.validateNoIdenticalROAs( - roaViewService.getRoaConfiguration(ca.getId()), - publishSet.getAdded(), publishSet.getDeleted()) + var currentRoas = new HashSet<>(roaViewService.getRoaConfiguration(ca.getId()).toAllowedRoutes()); + var futureRoas = Roas.applyDiff(currentRoas, Roas.toDiff(publishSet)); + + Roas.validateRoaUpdate(futureRoas) .ifPresent(rc -> { throw new IllegalArgumentException(rc); }); diff --git a/src/main/java/net/ripe/rpki/rest/service/Roas.java b/src/main/java/net/ripe/rpki/rest/service/Roas.java new file mode 100644 index 0000000..c072732 --- /dev/null +++ b/src/main/java/net/ripe/rpki/rest/service/Roas.java @@ -0,0 +1,66 @@ +package net.ripe.rpki.rest.service; + +import com.google.common.collect.Streams; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; +import lombok.Value; +import net.ripe.ipresource.Asn; +import net.ripe.ipresource.IpRange; +import net.ripe.rpki.commons.validation.roa.AllowedRoute; +import net.ripe.rpki.commons.validation.roa.AnnouncedRoute; +import net.ripe.rpki.rest.pojo.PublishSet; +import net.ripe.rpki.rest.pojo.ROA; + +import java.util.*; +import java.util.stream.Collectors; + +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public class Roas { + + public static Optional validateUniqueROAs(String prefix, Map> newOnes) { + for (var e : newOnes.entrySet()) { + var maxLengths = e.getValue(); + if (maxLengths.size() > 1) { + var sorted = maxLengths.stream().sorted().collect(Collectors.toList()); + return Optional.of(String.format("%s: there are more than one pair (%s, %s), max lengths: %s", + prefix, e.getKey().getOriginAsn(), e.getKey().getPrefix(), sorted)); + } + } + return Optional.empty(); + } + + public static RoaDiff toDiff(PublishSet publishSet) { + return new RoaDiff( + publishSet.getAdded().stream().map(Roas::toAllowedRoute).collect(Collectors.toSet()), + publishSet.getDeleted().stream().map(Roas::toAllowedRoute).collect(Collectors.toSet()) + ); + } + + private static AllowedRoute toAllowedRoute(ROA roa) { + var roaIpRange = IpRange.parse(roa.getPrefix()); + var maxLength = roa.getMaxLength() != null ? roa.getMaxLength() : roaIpRange.getPrefixLength(); + return new AllowedRoute(Asn.parse(roa.getAsn()), roaIpRange, maxLength); + } + + public static Set applyDiff(Set currentRoas, RoaDiff diff) { + var futureRoas = new HashSet<>(currentRoas); + diff.getDeleted().forEach(futureRoas::remove); + futureRoas.addAll(diff.getAdded()); + return futureRoas; + } + + public static Optional validateRoaUpdate(Set futureRoutes) { + var futureMap = futureRoutes.stream().collect(Collectors.toMap( + r -> new AnnouncedRoute(r.getAsn(), r.getPrefix()), + r -> Collections.singletonList(r.getMaximumLength()), + (a, b) -> Streams.concat(a.stream(), b.stream()).collect(Collectors.toList()))); + + return validateUniqueROAs("Error in future ROAs", futureMap); + } + + @Value + public static class RoaDiff { + Set added; + Set deleted; + } +} diff --git a/src/main/java/net/ripe/rpki/rest/service/Utils.java b/src/main/java/net/ripe/rpki/rest/service/Utils.java index e76a55e..3093b9a 100644 --- a/src/main/java/net/ripe/rpki/rest/service/Utils.java +++ b/src/main/java/net/ripe/rpki/rest/service/Utils.java @@ -20,8 +20,6 @@ import net.ripe.rpki.rest.pojo.ROA; import net.ripe.rpki.server.api.dto.BgpRisEntry; import net.ripe.rpki.server.api.dto.RoaAlertConfigurationData; -import net.ripe.rpki.server.api.dto.RoaConfigurationData; -import net.ripe.rpki.server.api.dto.RoaConfigurationPrefixData; import net.ripe.rpki.server.api.services.read.RoaAlertConfigurationViewService; import org.springframework.http.ResponseEntity; @@ -161,103 +159,4 @@ protected static ResponseEntity badRequestError(Exception e) { protected static ResponseEntity badRequestError(String errorMessage) { return ResponseEntity.badRequest().body(Map.of("error", errorMessage)); } - - static Optional validateNoIdenticalROAs(RoaConfigurationData roaConfigurationData, List newRoas, List roasToDelete) { - final List existingROAs = roaConfigurationData.getPrefixes().stream() - .map(p -> new ExistingROA(p.getAsn(), p.getPrefix(), p.getNullableMaxLength())) - .collect(Collectors.toList()); - return validateNoIdenticalROAs(existingROAs, newRoas, roasToDelete); - } - - /** - * Validate that there are not existing ROAs having the same AS and Prefix - * but different Max Length fields. - * - * @return Optional error text for the first validation error that was found. - */ - static Optional validateNoIdenticalROAs(List existingROAs, List newRoas, List roasToDelete) { - - final Map> newOnes = makeROAMap(newRoas); - final Map> deletedOnes = makeROAMap(roasToDelete); - - Optional e = validateUniqueROAs("Error in new ROAs", newOnes); - if (e.isPresent()) return e; - - e = validateUniqueROAs("Error in deleted ROAs", deletedOnes); - if (e.isPresent()) return e; - - final Map> newOnesUnique = uniqueEntries(newOnes); - final Map> deletedOnesUnique = uniqueEntries(deletedOnes); - - for (var existingRoa : existingROAs) { - var key = new AnnouncedRoute(existingRoa.getAsn(), existingRoa.getPrefix()); - if (deletedOnesUnique.containsKey(key)) { - var maxLengthToDelete = deletedOnesUnique.get(key).orElse(null); - if (Objects.equals(existingRoa.getMaximumLength(), maxLengthToDelete)) { - newOnesUnique.remove(key); - } - } - } - - for (var existingRoa : existingROAs) { - var key = new AnnouncedRoute(existingRoa.getAsn(), existingRoa.getPrefix()); - if (newOnesUnique.containsKey(key)) { - final Integer newMaxLength = newOnesUnique.get(key).orElse(null); - if (!Objects.equals(existingRoa.getMaximumLength(), newMaxLength)) { - // we are not going to delete existing one - return Optional.of(getSameROAErrorMessage(existingRoa, key, newMaxLength)); - } - } - } - return Optional.empty(); - } - - public static Map> uniqueEntries(Map> m) { - var newM = new HashMap>(m.size()); - m.forEach((k, list) -> newM.put(k, Optional.ofNullable(list.get(0)))); - return newM; - } - - public static Optional validateUniqueROAs(String prefix, Map> newOnes) { - for (var e : newOnes.entrySet()) { - if (e.getValue().size() > 1) { - return Optional.of(String.format("%s: there are more than one pair (%s, %s), max lengths: %s", - prefix, e.getKey().getOriginAsn(), e.getKey().getPrefix(), e.getValue())); - } - } - return Optional.empty(); - } - - public static String getSameROAErrorMessage(Object existingRoa, AnnouncedRoute key, Integer newMaxLength) { - return String.format( - "There is an overlap in ROAs: existing %s has the same (ASN, prefix) as added %s", - existingRoa, - new ROA(key.getOriginAsn().toString(), key.getPrefix().toString(), newMaxLength)); - } - - public static Map> makeROAMap(List newRoas) { - return newRoas.stream().map(r -> { - AnnouncedRoute announcedRoute = new AnnouncedRoute(Asn.parse(r.getAsn()), IpRange.parse(r.getPrefix())); - return Pair.of(announcedRoute, Collections.singletonList(r.getMaxLength())); - }).collect(Collectors.toMap(Pair::getLeft, Pair::getRight, - (a, b) -> Streams.concat(a.stream(), b.stream()) - .collect(Collectors.toList()))); - } - - @Value - public static class ExistingROA { - Asn asn; - IpRange prefix; - Integer maximumLength; - - @Override - public String toString() { - return "ROA{" + - "asn=" + asn + - ", prefix=" + prefix + - ", maximumLength=" + maximumLength + - '}'; - } - } - } diff --git a/src/test/java/net/ripe/rpki/rest/service/CaRoaConfigurationServiceTest.java b/src/test/java/net/ripe/rpki/rest/service/CaRoaConfigurationServiceTest.java index 90bfcd4..51d083c 100644 --- a/src/test/java/net/ripe/rpki/rest/service/CaRoaConfigurationServiceTest.java +++ b/src/test/java/net/ripe/rpki/rest/service/CaRoaConfigurationServiceTest.java @@ -30,11 +30,7 @@ import javax.security.auth.x500.X500Principal; import javax.ws.rs.core.HttpHeaders; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; +import java.util.*; import static net.ripe.rpki.rest.service.AbstractCaRestService.API_URL_PREFIX; import static org.assertj.core.api.Assertions.assertThat; @@ -115,7 +111,26 @@ public void shouldPostROAtoStageNoRealChange() throws Exception { } @Test - public void shouldPreventFromStaginBreakingChanges() throws Exception { + public void shouldNotRejectSameAsnPrefixAndDifferentMaxLengthWhenItsAReplacements() throws Exception { + when(roaViewService.getRoaConfiguration(CA_ID)).thenReturn(new RoaConfigurationData(List.of( + new RoaConfigurationPrefixData(new Asn(10), IpRange.parse("193.0.24.0/21"), 21)))); + + ImmutableResourceSet ipResourceSet = ImmutableResourceSet.parse("127.0.0.1, ::1"); + when(certificateAuthorityData.getResources()).thenReturn(ipResourceSet); + + Map> bgpRisEntries = new HashMap<>(); + bgpRisEntries.put(false, Collections.singletonList(new BgpRisEntry(new Asn(10), IpRange.parse("193.0.24.0/21"), 21))); + when(bgpRisEntryViewService.findMostSpecificContainedAndNotContained(ipResourceSet)).thenReturn(bgpRisEntries); + + // try to replace one maxLength with the other + mockMvc.perform(Rest.post(API_URL_PREFIX + "/123/roas/stage") + .content("[{\"asn\" : \"AS10\", \"prefix\" : \"193.0.24.0/21\", \"maximalLength\": 22 }]")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.length()").value("1")); + } + + @Test + public void shouldRejectSameAsnPrefixAndDifferentMaxLengthWhenItsExtraROA() throws Exception { when(roaViewService.getRoaConfiguration(CA_ID)).thenReturn(new RoaConfigurationData(Collections.singletonList( new RoaConfigurationPrefixData(new Asn(10), IpRange.parse(TESTNET_1), 32)))); @@ -131,12 +146,13 @@ public void shouldPreventFromStaginBreakingChanges() throws Exception { when(bgpRisEntryViewService.findMostSpecificContainedAndNotContained(ipResourceSet)).thenReturn(bgpRisEntries); mockMvc.perform(Rest.post(API_URL_PREFIX + "/123/roas/stage") - .content("[{\"asn\" : \"AS10\", \"prefix\" : \"" + TESTNET_1 + "\", \"maximalLength\" : \"24\"}]")) + .content("[" + + "{\"asn\" : \"AS10\", \"prefix\" : \"" + TESTNET_1 + "\", \"maximalLength\" : \"24\"}," + + "{\"asn\" : \"AS10\", \"prefix\" : \"" + TESTNET_1 + "\", \"maximalLength\" : \"25\"}]" + )) .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.error") - .value("There is an overlap in ROAs: existing " + - "AllowedRoute[asn=AS10,maximumLength=32,prefix=192.0.2.0/24] has the same (ASN, prefix) as added " + - "ROA{asn='AS10', prefix='192.0.2.0/24', maximalLength=24}")); + .value("Error in future ROAs: there are more than one pair (AS10, 192.0.2.0/24), max lengths: [24, 25]")); } /** diff --git a/src/test/java/net/ripe/rpki/rest/service/RoasTest.java b/src/test/java/net/ripe/rpki/rest/service/RoasTest.java new file mode 100644 index 0000000..d2c7701 --- /dev/null +++ b/src/test/java/net/ripe/rpki/rest/service/RoasTest.java @@ -0,0 +1,60 @@ +package net.ripe.rpki.rest.service; + +import net.ripe.ipresource.Asn; +import net.ripe.ipresource.IpRange; +import net.ripe.rpki.commons.validation.roa.AllowedRoute; +import org.junit.Test; + +import java.util.Collections; +import java.util.Optional; +import java.util.Set; + +import static org.junit.Assert.assertEquals; + +public class RoasTest { + + @Test + public void testValidateApplyDiff() { + assertEquals( + Set.of(new AllowedRoute(new Asn(11L), IpRange.parse("192.0.2.0/24"), 24)), + Roas.applyDiff( + Set.of(new AllowedRoute(new Asn(10L), IpRange.parse("192.0.2.0/24"), 24)), + new Roas.RoaDiff( + Set.of(new AllowedRoute(new Asn(11L), IpRange.parse("192.0.2.0/24"), 24)), + Set.of(new AllowedRoute(new Asn(10L), IpRange.parse("192.0.2.0/24"), 24)) + ) + )); + } + + @Test + public void testValidateRoaUpdate() { + assertEquals(Optional.empty(), Roas.validateRoaUpdate(Collections.emptySet())); + + // No changes + assertEquals(Optional.empty(), + Roas.validateRoaUpdate(Set.of(new AllowedRoute(new Asn(10L), IpRange.parse("192.0.2.0/24"), 24)))); + + // Add a couple of ROAs + assertEquals(Optional.empty(), + Roas.validateRoaUpdate( + Set.of( + new AllowedRoute(new Asn(10L), IpRange.parse("192.0.2.0/24"), 24), + new AllowedRoute(new Asn(11L), IpRange.parse("193.0.2.0/24"), 24), + new AllowedRoute(new Asn(12L), IpRange.parse("194.0.2.0/24"), 24) + ))); + + // Delete a couple of ROAs + assertEquals(Optional.empty(), + Roas.validateRoaUpdate( + Set.of(new AllowedRoute(new Asn(10L), IpRange.parse("192.0.2.0/24"), 24)))); + + // Try to add maxLength duplicates + assertEquals(Optional.of("Error in future ROAs: there are more than one pair (AS10, 192.0.2.0/24), max lengths: [24, 25]"), + Roas.validateRoaUpdate( + Set.of( + new AllowedRoute(new Asn(10L), IpRange.parse("192.0.2.0/24"), 24), + new AllowedRoute(new Asn(10L), IpRange.parse("192.0.2.0/24"), 25) + ))); + } + +} \ No newline at end of file diff --git a/src/test/java/net/ripe/rpki/rest/service/UtilsTest.java b/src/test/java/net/ripe/rpki/rest/service/UtilsTest.java index 6e49df5..7e2c3cb 100644 --- a/src/test/java/net/ripe/rpki/rest/service/UtilsTest.java +++ b/src/test/java/net/ripe/rpki/rest/service/UtilsTest.java @@ -1,17 +1,14 @@ package net.ripe.rpki.rest.service; -import net.ripe.ipresource.Asn; import net.ripe.ipresource.IpRange; import net.ripe.rpki.rest.pojo.ROA; import org.junit.Test; -import java.util.Collections; -import java.util.List; -import java.util.Optional; - -import static net.ripe.rpki.rest.service.Utils.*; +import static net.ripe.rpki.rest.service.Utils.errorsInUserInputRoas; +import static net.ripe.rpki.rest.service.Utils.maxLengthIsValid; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.*; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; public class UtilsTest { @Test @@ -54,51 +51,4 @@ public void shouldRejectTooLongMaxLengthV6() { assertFalse(maxLengthIsValid(IpRange.parse("2001:DB8::/48"), 133)); } - @Test - public void shouldValidateSameROAUpdate() { - assertEquals(Optional.empty(), - validateNoIdenticalROAs(Collections.emptyList(), Collections.emptyList(), Collections.emptyList())); - - assertEquals(Optional.empty(), - validateNoIdenticalROAs( - List.of(new ExistingROA(new Asn(10L), IpRange.parse("192.0.2.0/24"), null)), - Collections.emptyList(), - Collections.emptyList())); - - assertEquals(Optional.of( - "There is an overlap in ROAs: existing ROA{asn=AS10, prefix=192.0.1.0/24, maximumLength=28} " + - "has the same (ASN, prefix) as added ROA{asn='AS10', prefix='192.0.1.0/24', maximalLength=27}"), - validateNoIdenticalROAs( - List.of(new ExistingROA(new Asn(10L), IpRange.parse("192.0.1.0/24"), 28)), - List.of(new ROA("AS10", "192.0.1.0/24", 27)), - Collections.emptyList())); - - assertEquals(Optional.of( - "There is an overlap in ROAs: existing ROA{asn=AS10, prefix=192.0.1.0/24, maximumLength=28} " + - "has the same (ASN, prefix) as added ROA{asn='AS10', prefix='192.0.1.0/24', maximalLength=null}"), - validateNoIdenticalROAs( - List.of(new ExistingROA(new Asn(10L), IpRange.parse("192.0.1.0/24"), 28)), - List.of(new ROA("AS10", "192.0.1.0/24", null)), - Collections.emptyList())); - - assertEquals(Optional.empty(), - validateNoIdenticalROAs( - List.of(new ExistingROA(new Asn(10L), IpRange.parse("192.0.1.0/24"), 28)), - List.of(new ROA("AS10", "192.0.1.0/24", 27)), - List.of(new ROA("AS10", "192.0.1.0/24", 28)) - )); - - assertEquals(Optional.empty(), - validateNoIdenticalROAs( - List.of(new ExistingROA(new Asn(10L), IpRange.parse("192.0.2.0/24"), null)), - List.of(new ROA("AS10", "192.0.2.0/24", 27)), - List.of(new ROA("AS10", "192.0.2.0/24", null)) - )); - - assertEquals(Optional.empty(), - validateNoIdenticalROAs( - List.of(new ExistingROA(new Asn(10L), IpRange.parse("192.0.2.0/24"), 27)), - List.of(new ROA("AS10", "192.0.2.0/24", null)), - List.of(new ROA("AS10", "192.0.2.0/24", 27)))); - } }