Skip to content

Commit

Permalink
Fix Palo Alto address and address-group inheritance (#4069)
Browse files Browse the repository at this point in the history
- fix logging-server collection
- fix service and service-group inheritance
  • Loading branch information
arifogel committed Jun 12, 2019
1 parent 942d180 commit 2814113
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@
import org.batfish.representation.palo_alto.SyslogServer;
import org.batfish.representation.palo_alto.VirtualRouter;
import org.batfish.representation.palo_alto.Vsys;
import org.batfish.representation.palo_alto.Vsys.NamespaceType;
import org.batfish.representation.palo_alto.Zone;
import org.batfish.vendor.StructureType;

Expand Down Expand Up @@ -846,8 +847,11 @@ public void enterSet_line_config_devices(Set_line_config_devicesContext ctx) {

@Override
public void enterS_policy_panorama(S_policy_panoramaContext ctx) {
_currentVsys =
_configuration.getVirtualSystems().computeIfAbsent(PANORAMA_VSYS_NAME, Vsys::new);
_currentVsys = _configuration.getPanorama();
if (_currentVsys == null) {
_currentVsys = new Vsys(PANORAMA_VSYS_NAME, NamespaceType.PANORAMA);
_configuration.setPanorama(_currentVsys);
}
}

@Override
Expand Down Expand Up @@ -1373,7 +1377,11 @@ public void exitSservgrp_members(Sservgrp_membersContext ctx) {

@Override
public void enterS_shared(S_sharedContext ctx) {
_currentVsys = _configuration.getVirtualSystems().computeIfAbsent(SHARED_VSYS_NAME, Vsys::new);
_currentVsys = _configuration.getShared();
if (_currentVsys == null) {
_currentVsys = new Vsys(SHARED_VSYS_NAME, NamespaceType.SHARED);
_configuration.setShared(_currentVsys);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Multiset;
import com.google.common.collect.Sets;
import com.google.common.collect.SortedMultiset;
Expand All @@ -31,6 +32,7 @@
import java.util.Map.Entry;
import java.util.NavigableMap;
import java.util.NavigableSet;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.SortedMap;
Expand Down Expand Up @@ -117,6 +119,10 @@ public final class PaloAltoConfiguration extends VendorConfiguration {

private String _ntpServerSecondary;

private @Nullable Vsys _panorama;

private @Nullable Vsys _shared;

private final SortedMap<String, Vsys> _sharedGateways;

private ConfigurationFormat _vendor;
Expand Down Expand Up @@ -289,10 +295,7 @@ public static String computeServiceGroupMemberAclName(

/** Convert vsys components to vendor independent model */
private void convertVirtualSystems() {
NavigableSet<String> loggingServers = new TreeSet<>();

for (Vsys vsys : _virtualSystems.values()) {
loggingServers.addAll(vsys.getSyslogServerAddresses());

// Create zone-specific outgoing ACLs.
for (Zone toZone : vsys.getZones().values()) {
Expand Down Expand Up @@ -326,7 +329,6 @@ private void convertVirtualSystems() {
}
}
}
_c.setLoggingServers(loggingServers);
}

/** Convert unique aspects of shared-gateways. */
Expand All @@ -342,9 +344,14 @@ private void convertSharedGateways() {

/** Convert structures common to all vsys-like namespaces */
private void convertNamespaces() {
Stream.concat(_sharedGateways.values().stream(), _virtualSystems.values().stream())
ImmutableSortedSet.Builder<String> loggingServers = ImmutableSortedSet.naturalOrder();
Streams.concat(
_sharedGateways.values().stream(),
_virtualSystems.values().stream(),
Stream.of(_panorama, _shared).filter(Objects::nonNull))
.forEach(
namespace -> {
loggingServers.addAll(namespace.getSyslogServerAddresses());
// convert address objects and groups to ip spaces
namespace
.getAddressObjects()
Expand Down Expand Up @@ -389,6 +396,7 @@ private void convertNamespaces() {
_c.getIpAccessLists().put(acl.getName(), acl);
}
});
_c.setLoggingServers(loggingServers.build());
}

/** Generates a cross-zone ACL from the two given zones in the same Vsys using the given rules. */
Expand Down Expand Up @@ -732,7 +740,7 @@ private List<Map.Entry<Rule, Vsys>> getAllRules(Vsys vsys) {
}

@Nullable
private static IpSpace ipSpaceFromRuleEndpoints(
private IpSpace ipSpaceFromRuleEndpoints(
Collection<RuleEndpoint> endpoints, Vsys vsys, Warnings w) {
return AclIpSpace.union(
endpoints.stream()
Expand Down Expand Up @@ -825,37 +833,51 @@ private IpAccessListLine toIpAccessListLine(Rule rule, Vsys vsys) {

/** Converts {@link RuleEndpoint} to {@code IpSpace} */
@Nonnull
private static IpSpace ruleEndpointToIpSpace(RuleEndpoint endpoint, Vsys vsys, Warnings w) {
@SuppressWarnings("fallthrough")
private IpSpace ruleEndpointToIpSpace(RuleEndpoint endpoint, Vsys vsys, Warnings w) {
String endpointValue = endpoint.getValue();
// Palo Alto allows object references that look like IP addresses, ranges, etc.
// Devices use objects over constants when possible, so, check to see if there is a matching
// group or object regardless of the type of endpoint we're expecting.
if (vsys.getAddressObjects().containsKey(endpointValue)) {
return vsys.getAddressObjects().get(endpointValue).getIpSpace();
} else if (vsys.getAddressGroups().containsKey(endpoint.getValue())) {
}
if (vsys.getAddressGroups().containsKey(endpoint.getValue())) {
return vsys.getAddressGroups()
.get(endpointValue)
.getIpSpace(vsys.getAddressObjects(), vsys.getAddressGroups());
} else {
// No named object found matching this endpoint, so parse the endpoint value as is
switch (endpoint.getType()) {
case Any:
return UniverseIpSpace.INSTANCE;
case IP_ADDRESS:
return Ip.parse(endpointValue).toIpSpace();
case IP_PREFIX:
return Prefix.parse(endpointValue).toIpSpace();
case IP_RANGE:
String[] ips = endpointValue.split("-");
return IpRange.range(Ip.parse(ips[0]), Ip.parse(ips[1]));
case REFERENCE:
// Undefined reference
w.redFlag("No matching address group/object found for RuleEndpoint: " + endpoint);
return EmptyIpSpace.INSTANCE;
default:
w.redFlag("Could not convert RuleEndpoint to IpSpace: " + endpoint);
return EmptyIpSpace.INSTANCE;
}
}
switch (vsys.getNamespaceType()) {
case LEAF:
if (_shared != null) {
return ruleEndpointToIpSpace(endpoint, _shared, w);
}
// fall-through
case SHARED:
if (_panorama != null) {
return ruleEndpointToIpSpace(endpoint, _panorama, w);
}
// fall-through
default:
// No named object found matching this endpoint, so parse the endpoint value as is
switch (endpoint.getType()) {
case Any:
return UniverseIpSpace.INSTANCE;
case IP_ADDRESS:
return Ip.parse(endpointValue).toIpSpace();
case IP_PREFIX:
return Prefix.parse(endpointValue).toIpSpace();
case IP_RANGE:
String[] ips = endpointValue.split("-");
return IpRange.range(Ip.parse(ips[0]), Ip.parse(ips[1]));
case REFERENCE:
// Undefined reference
w.redFlag("No matching address group/object found for RuleEndpoint: " + endpoint);
return EmptyIpSpace.INSTANCE;
default:
w.redFlag("Could not convert RuleEndpoint to IpSpace: " + endpoint);
return EmptyIpSpace.INSTANCE;
}
}
}

Expand Down Expand Up @@ -1207,4 +1229,20 @@ private void markAbstractStructureFromUnknownNamespace(
});
}
}

public @Nullable Vsys getPanorama() {
return _panorama;
}

public @Nullable Vsys getShared() {
return _shared;
}

public void setPanorama(@Nullable Vsys panorama) {
_panorama = panorama;
}

public void setShared(@Nullable Vsys shared) {
_shared = shared;
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
package org.batfish.representation.palo_alto;

import static org.batfish.representation.palo_alto.PaloAltoConfiguration.SHARED_VSYS_NAME;

import com.google.common.collect.ImmutableList;
import java.io.Serializable;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -31,14 +28,24 @@ public String getName() {
* Return the name of the vsys this reference is attached to, or return null if no match is found
*/
@Nullable
@SuppressWarnings("fallthrough")
String getVsysName(PaloAltoConfiguration pc, Vsys vsys) {
// Search for a matching member in the local then shared namespace, in that order
for (Vsys currentVsys : ImmutableList.of(vsys, pc.getVirtualSystems().get(SHARED_VSYS_NAME))) {
if (currentVsys.getServices().containsKey(_name)
|| currentVsys.getServiceGroups().containsKey(_name)) {
return currentVsys.getName();
}
if (vsys.getServices().containsKey(_name) || vsys.getServiceGroups().containsKey(_name)) {
return vsys.getName();
}
switch (vsys.getNamespaceType()) {
case LEAF:
if (pc.getShared() != null) {
return getVsysName(pc, pc.getShared());
}
// fall-through
case SHARED:
if (pc.getPanorama() != null) {
return getVsysName(pc, pc.getPanorama());
}
// fall-through
default:
return null;
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,20 @@
import java.util.SortedSet;
import java.util.TreeMap;
import java.util.TreeSet;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.ParametersAreNonnullByDefault;

@ParametersAreNonnullByDefault
public final class Vsys implements Serializable {

public enum NamespaceType {
/** vsys or shared-gateway */
LEAF,
PANORAMA,
SHARED
}

private static final long serialVersionUID = 1L;

private final SortedMap<String, AddressGroup> _addressGroups;
Expand Down Expand Up @@ -48,8 +57,20 @@ public final class Vsys implements Serializable {

private final SortedMap<String, Zone> _zones;

private final @Nonnull NamespaceType _namespaceType;

/**
* Construct a {@link Vsys} with provided {@code name} and namespace type {@link
* NamespaceType#LEAF}.
*/
public Vsys(String name) {
this(name, NamespaceType.LEAF);
}

/** Construct a {@link Vsys} with provided {@code name} and {@code namespaceType}. */
public Vsys(String name, NamespaceType namespaceType) {
_name = name;
_namespaceType = namespaceType;
_addressGroups = new TreeMap<>();
_addressObjects = new TreeMap<>();
_applications = new TreeMap<>();
Expand Down Expand Up @@ -169,4 +190,8 @@ public void setDisplayName(String displayName) {
public String toString() {
return MoreObjects.toStringHelper(Vsys.class).add("name", _name).toString();
}

public @Nonnull NamespaceType getNamespaceType() {
return _namespaceType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,6 @@ public void testAllowInterVsysNextVr() throws IOException {
assertBidirAccepted(hostname);
}

@Ignore
@Test
public void testMatchSharedAddress() throws IOException {
String hostname = "match-shared-address";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@
import org.junit.rules.ExpectedException;
import org.junit.rules.TemporaryFolder;

public class PaloAltoGrammarTest {
public final class PaloAltoGrammarTest {
private static final String TESTCONFIGS_PREFIX = "org/batfish/grammar/palo_alto/testconfigs/";

@Rule public TemporaryFolder _folder = new TemporaryFolder();
Expand Down

0 comments on commit 2814113

Please sign in to comment.