diff --git a/whois-api/src/main/java/net/ripe/db/whois/api/rest/search/RpkiRoaMessageGenerator.java b/whois-api/src/main/java/net/ripe/db/whois/api/rest/search/RpkiRoaMessageGenerator.java index 579ca612e9..b79ad4ccae 100644 --- a/whois-api/src/main/java/net/ripe/db/whois/api/rest/search/RpkiRoaMessageGenerator.java +++ b/whois-api/src/main/java/net/ripe/db/whois/api/rest/search/RpkiRoaMessageGenerator.java @@ -13,6 +13,7 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Component; +import javax.annotation.Nullable; import java.util.Map; @@ -47,6 +48,7 @@ private boolean canProceed(final Parameters parameters){ return isEnabled && (parameters.getRoaCheck()!= null && parameters.getRoaCheck()); } + @Nullable private RpslMessage validateRoa(final RpslObject rpslObject){ final Map.Entry invalidRpkiRoa = whoisRoaChecker.validateAndGetInvalidRoa(rpslObject); @@ -57,7 +59,8 @@ private RpslMessage validateRoa(final RpslObject rpslObject){ return switch (invalidRpkiRoa.getValue()) { case INVALID_ORIGIN -> new RpslMessage(QueryMessages.roaRouteOriginConflicts(rpslObject.getType().getName(), invalidRpkiRoa.getKey().getMaxLength(), invalidRpkiRoa.getKey().getAsn())); case INVALID_PREFIX_LENGTH -> new RpslMessage(QueryMessages.roaRoutePrefixLengthConflicts(rpslObject.getType().getName(), invalidRpkiRoa.getKey().getMaxLength(), invalidRpkiRoa.getKey().getAsn())); - default -> new RpslMessage(QueryMessages.roaRouteConflicts(rpslObject.getType().getName(), invalidRpkiRoa.getKey().getMaxLength(), invalidRpkiRoa.getKey().getAsn())); + case INVALID_PREFIX_AND_ORIGIN -> new RpslMessage(QueryMessages.roaRouteConflicts(rpslObject.getType().getName(), invalidRpkiRoa.getKey().getMaxLength(), invalidRpkiRoa.getKey().getAsn())); + default -> null; }; } } diff --git a/whois-api/src/test/java/net/ripe/db/whois/api/rest/WhoisSearchServiceTestIntegration.java b/whois-api/src/test/java/net/ripe/db/whois/api/rest/WhoisSearchServiceTestIntegration.java index 364fd209b8..4cdb241c56 100644 --- a/whois-api/src/test/java/net/ripe/db/whois/api/rest/WhoisSearchServiceTestIntegration.java +++ b/whois-api/src/test/java/net/ripe/db/whois/api/rest/WhoisSearchServiceTestIntegration.java @@ -2007,6 +2007,27 @@ public void search_route_roa_origin_and_prefix_length_mismatch_validation_enable "either remove this route: object or update or delete the ROA.\n")); } + @Test + public void search_route_one_invalid_one_valid_roas_makes_it_valid() { + databaseHelper.addObject(RpslObject.parse("" + + "route: 92.38.45.0/24\n" + + "descr: Ripe test allocation\n" + + "origin: AS61979\n" + + "admin-c: TP1-TEST\n" + + "mnt-by: OWNER-MNT\n" + + "mnt-lower: OWNER-MNT\n" + + "source: TEST-NONAUTH\n")); + ipTreeUpdater.rebuild(); + + final WhoisResources whoisResources = RestTest.target(getPort(), "whois/search.xml?query-string=92.38.45.0/24AS61979&roa-check=true&flags=no-referenced") + .request() + .get(WhoisResources.class); + + assertThat(whoisResources.getWhoisObjects(), hasSize(1)); + + assertThat(whoisResources.getWhoisObjects().get(0).getObjectMessages().getMessages(), hasSize(0)); + } + @Test public void search_route6_roa_mismatch_less_specific_as_xml_strings() { databaseHelper.addObject(RpslObject.parse("" + diff --git a/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/RpkiRoaChecker.java b/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/AbstractRpkiRoaChecker.java similarity index 94% rename from whois-commons/src/main/java/net/ripe/db/whois/common/rpki/RpkiRoaChecker.java rename to whois-commons/src/main/java/net/ripe/db/whois/common/rpki/AbstractRpkiRoaChecker.java index 7cd0afedaa..addbedc8d4 100644 --- a/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/RpkiRoaChecker.java +++ b/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/AbstractRpkiRoaChecker.java @@ -17,11 +17,11 @@ import static net.ripe.db.whois.common.rpki.ValidationStatus.VALID; import static net.ripe.db.whois.common.rpsl.ObjectType.ROUTE; -public abstract class RpkiRoaChecker { +public abstract class AbstractRpkiRoaChecker { private final RpkiService rpkiService; - public RpkiRoaChecker(final RpkiService rpkiService) { + public AbstractRpkiRoaChecker(final RpkiService rpkiService) { this.rpkiService = rpkiService; } diff --git a/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/ValidationStatus.java b/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/ValidationStatus.java index d31bce5543..663929ae9e 100644 --- a/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/ValidationStatus.java +++ b/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/ValidationStatus.java @@ -6,5 +6,6 @@ public enum ValidationStatus { VALID, INVALID, INVALID_ORIGIN, - INVALID_PREFIX_LENGTH + INVALID_PREFIX_LENGTH, + INVALID_PREFIX_AND_ORIGIN } diff --git a/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/WhoisRoaChecker.java b/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/WhoisRoaChecker.java index 7d0db9f75e..e8bf89255b 100644 --- a/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/WhoisRoaChecker.java +++ b/whois-commons/src/main/java/net/ripe/db/whois/common/rpki/WhoisRoaChecker.java @@ -11,45 +11,63 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.Predicate; +import java.util.stream.Stream; -import static net.ripe.db.whois.common.rpki.ValidationStatus.INVALID; import static net.ripe.db.whois.common.rpki.ValidationStatus.INVALID_ORIGIN; +import static net.ripe.db.whois.common.rpki.ValidationStatus.INVALID_PREFIX_AND_ORIGIN; import static net.ripe.db.whois.common.rpki.ValidationStatus.INVALID_PREFIX_LENGTH; +import static net.ripe.db.whois.common.rpki.ValidationStatus.NOT_FOUND; import static net.ripe.db.whois.common.rpki.ValidationStatus.VALID; @Component -public class WhoisRoaChecker extends RpkiRoaChecker { +public class WhoisRoaChecker extends AbstractRpkiRoaChecker { public WhoisRoaChecker(final RpkiService rpkiService) { super(rpkiService); } public Map.Entry validateAndGetInvalidRoa(final RpslObject route){ - final Optional> roaStatusMap = validateRoas(route) + /* This method prioritize VALID roas over INVALID roas. So in case of overlap the VALID ROA will se used. + This is a common behaviour related to roas */ + final Map roasStatus = validateRoas(route); + final Optional> roaStatusMap = roasStatus .entrySet() .stream() - .filter(entry -> INVALID.equals(entry.getValue()) || INVALID_PREFIX_LENGTH.equals(entry.getValue()) || INVALID_ORIGIN.equals(entry.getValue())) - .findFirst(); + .filter(getValidOrNotFoundRoas()).findFirst() + .or(() -> getInvalidRoas(roasStatus).findFirst()); if (roaStatusMap.isEmpty()){ return null; } return roaStatusMap.get(); } + @Override protected ValidationStatus validate(final RpslObject route, final Roa roa, final IpInterval prefix) { final List invalidStatus = Lists.newArrayList(); - final long nonAuthAsn = Asn.parse(route.getValueForAttribute(AttributeType.ORIGIN).toString()).asBigInteger().longValue(); + final long routeAsn = Asn.parse(route.getValueForAttribute(AttributeType.ORIGIN).toString()).asBigInteger().longValue(); if (prefix.getPrefixLength() > roa.getMaxLength()){ invalidStatus.add(INVALID_PREFIX_LENGTH); } - if (nonAuthAsn != 0 && nonAuthAsn != roa.getAsn()){ + if (routeAsn == 0 || routeAsn != roa.getAsn()){ invalidStatus.add(INVALID_ORIGIN); } if (invalidStatus.isEmpty()){ return VALID; } - return invalidStatus.size() == 1 ? invalidStatus.get(0) : INVALID; + return invalidStatus.size() == 1 ? invalidStatus.get(0) : INVALID_PREFIX_AND_ORIGIN; + } + + private Predicate> getValidOrNotFoundRoas() { + return entry -> NOT_FOUND.equals(entry.getValue()) || VALID.equals(entry.getValue()); + } + + private Stream> getInvalidRoas(Map roasStatus) { + return roasStatus.entrySet() + .stream() + .filter(other -> INVALID_PREFIX_AND_ORIGIN.equals(other.getValue()) || INVALID_PREFIX_LENGTH.equals(other.getValue()) || INVALID_ORIGIN.equals(other.getValue())) + .sorted((o1, o2) -> o1.getKey().getPrefix().compareTo(o2.getKey().getPrefix())); } } diff --git a/whois-commons/src/test/java/net/ripe/db/whois/common/rpki/WhoisRoaCheckerTest.java b/whois-commons/src/test/java/net/ripe/db/whois/common/rpki/WhoisRoaCheckerTest.java new file mode 100644 index 0000000000..aec24e7855 --- /dev/null +++ b/whois-commons/src/test/java/net/ripe/db/whois/common/rpki/WhoisRoaCheckerTest.java @@ -0,0 +1,131 @@ +package net.ripe.db.whois.common.rpki; + +import com.google.common.collect.Sets; +import net.ripe.db.whois.common.rpsl.RpslObject; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Map; + +import static net.ripe.db.whois.common.rpki.ValidationStatus.VALID; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +public class WhoisRoaCheckerTest { + + @Mock + RpkiService rpkiService; + + @InjectMocks + WhoisRoaChecker whoisRoaChecker; + + @Test + public void exact_match_valid() { + when(rpkiService.findRoas(eq("10.0.0.0/8"))).thenReturn(Sets.newHashSet(new Roa("AS1", 8, "10.0.0.0/8", "ripe"))); + final Map.Entry result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse( + "route: 10.0.0.0/8\n" + + "origin: AS1" + )); + assertThat(result.getValue(), is(ValidationStatus.VALID)); + } + + @Test + public void different_origin_invalid() { + when(rpkiService.findRoas(eq("10.0.0.0/8"))).thenReturn(Sets.newHashSet(new Roa("AS1", 8, "10.0.0.0/8", "ripe"))); + final Map.Entry result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse( + "route: 10.0.0.0/8\n" + + "origin: AS2" + )); + assertThat(result.getValue(), is(ValidationStatus.INVALID_ORIGIN)); + } + + @Test + public void asn0_invalid() { + when(rpkiService.findRoas(eq("10.0.0.0/8"))).thenReturn(Sets.newHashSet(new Roa("AS1", 8, "10.0.0.0/8", "ripe"))); + final Map.Entry result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse( + "route: 10.0.0.0/8\n" + + "origin: AS0" + )); + assertThat(result.getValue(), is(ValidationStatus.INVALID_ORIGIN)); + } + + @Test + public void more_specific_roa_prefix_lower_length_invalid() { + when(rpkiService.findRoas(eq("10.0.0.0/16"))).thenReturn(Sets.newHashSet(new Roa("AS1", 8, "10.0.0.0/8", "ripe"))); + final Map.Entry result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse( + "route: 10.0.0.0/16\n" + + "origin: AS1" + )); + assertThat(result.getValue(), is(ValidationStatus.INVALID_PREFIX_LENGTH)); + } + + @Test + public void more_specific_roa_allowed_by_same_length_valid() { + when(rpkiService.findRoas(eq("10.0.0.0/16"))).thenReturn(Sets.newHashSet(new Roa("AS1", 16, "10.0.0.0/8", "ripe"))); + final Map.Entry result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse( + "route: 10.0.0.0/16\n" + + "origin: AS1" + )); + assertThat(result.getValue(), is(VALID)); + } + + @Test + public void more_specific_roa_allowed_by_max_length_valid() { + when(rpkiService.findRoas(eq("10.0.0.0/16"))).thenReturn(Sets.newHashSet(new Roa("AS1", 24, "10.0.0.0/8", "ripe"))); + final Map.Entry result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse( + "route: 10.0.0.0/16\n" + + "origin: AS1" + )); + assertThat(result.getValue(), is(VALID)); + } + + @Test + public void more_specific_roa_prefix_and_different_origin_invalid() { + when(rpkiService.findRoas(eq("10.0.0.0/16"))).thenReturn(Sets.newHashSet(new Roa("AS1", 8, "10.0.0.0/8", "ripe"))); + final Map.Entry result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse( + "route: 10.0.0.0/16\n" + + "origin: AS2" + )); + assertThat(result.getValue(), is(ValidationStatus.INVALID_PREFIX_AND_ORIGIN)); + } + + @Test + public void less_specific_roa_prefix_valid() { + when(rpkiService.findRoas(eq("10.0.0.0/8"))).thenReturn(Sets.newHashSet(new Roa("AS1", 8, "10.0.0.0/16", "ripe"))); + final Map.Entry result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse( + "route: 10.0.0.0/8\n" + + "origin: AS1" + )); + assertThat(result.getValue(), is(ValidationStatus.VALID)); + } + + @Test + public void overlap_valid_invalid_roa_then_valid() { + final Roa nonMatchingRoa = new Roa("AS44546", 24, "92.38.0.0/17", "ripe"); + final Roa matchingRoa = new Roa("AS61979", 24, "92.38.44.0/22", "ripe"); + when(rpkiService.findRoas(eq("92.38.45.0/24"))).thenReturn(Sets.newHashSet(nonMatchingRoa, matchingRoa)); + final Map.Entry result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse( + "route: 92.38.45.0/24\n" + + "origin: AS61979" + )); + assertThat(result.getValue(), is(ValidationStatus.VALID)); + } + + @Test + public void overlap_two_invalid_roas_then_invalid() { + final Roa nonMatchingRoa = new Roa("AS44546", 24, "92.38.0.0/17", "ripe"); + final Roa matchingRoa = new Roa("AS61979", 8, "92.38.44.0/22", "ripe"); + when(rpkiService.findRoas(eq("92.38.45.0/24"))).thenReturn(Sets.newHashSet(nonMatchingRoa, matchingRoa)); + final Map.Entry result = whoisRoaChecker.validateAndGetInvalidRoa(RpslObject.parse( + "route: 92.38.45.0/24\n" + + "origin: AS61979" + )); + assertThat(result.getValue(), is(ValidationStatus.INVALID_ORIGIN)); + } +} diff --git a/whois-commons/src/test/resources/rpki/roas.json b/whois-commons/src/test/resources/rpki/roas.json index f235fabdbd..b7abff3e38 100644 --- a/whois-commons/src/test/resources/rpki/roas.json +++ b/whois-commons/src/test/resources/rpki/roas.json @@ -26,6 +26,8 @@ { "asn": "AS1964", "prefix": "206.48.0.0/16", "maxLength": 16, "ta": "arin" }, { "asn": "AS6505", "prefix": "193.4.0.0/16", "maxLength": 16, "ta": "arin" }, { "asn": "AS7505", "prefix": "200.4.0.0/23", "maxLength": 23, "ta": "arin" }, - { "asn": "AS6505", "prefix": "176.223.250.0/23", "maxLength": 23, "ta": "ripe" } + { "asn": "AS6505", "prefix": "176.223.250.0/23", "maxLength": 23, "ta": "ripe" }, + { "asn": "AS44546", "prefix": "92.38.0.0/17", "maxLength": 24, "ta": "arin" }, + { "asn": "AS61979", "prefix": "92.38.44.0/22", "maxLength": 24, "ta": "ripe" } ] } \ No newline at end of file