More AAA app refactoring

- cleaned up exception handling a little - reformatting of the code to be easier to read and group similar code together - fixed some style problems found by IntelliJ Change-Id: I932ac48d82b598b1f59c906477fb5e4deded413f

More AAA app refactoring
- cleaned up exception handling a little - reformatting of the code to be easier to read and group similar code together - fixed some style problems found by IntelliJ Change-Id: I932ac48d82b598b1f59c906477fb5e4deded413f
Ray Milkey · Gerrit Code Review
Commit 1c0ea1a12e6fa62368405eda821c2a26b3aaddd8 1c0ea1a1 1 parent 66a8554a
Showing 1 changed file with 49 additions and 39 deletions
apps/aaa/src/main/java/org/onosproject/aaa/AAA.java
--- a/apps/aaa/src/main/java/org/onosproject/aaa/AAA.java
View file @1c0ea1a
+++ b/apps/aaa/src/main/java/org/onosproject/aaa/AAA.java
View file @1c0ea1a
@@ -79,62 +79,85 @@ import static org.slf4j.LoggerFactory.getLogger;
 public class AAA {
     // RADIUS server IP address
     private static final String DEFAULT_RADIUS_IP = "192.168.1.10";
+
     // NAS IP address
     private static final String DEFAULT_NAS_IP = "192.168.1.11";
+
     // RADIUS uplink port
     private static final int DEFAULT_RADIUS_UPLINK = 2;
+
     // RADIUS server shared secret
     private static final String DEFAULT_RADIUS_SECRET = "ONOSecret";
+
     // RADIUS MAC address
     private static final String RADIUS_MAC_ADDRESS = "00:00:00:00:01:10";
+
     // NAS MAC address
     private static final String NAS_MAC_ADDRESS = "00:00:00:00:10:01";
+
     // Radius Switch Id
     private static final String DEFAULT_RADIUS_SWITCH = "of:90e2ba82f97791e9";
+
     // Radius Port Number
     private static final String DEFAULT_RADIUS_PORT = "129";
+
     // for verbose output
     private final Logger log = getLogger(getClass());
+
     // a list of our dependencies :
     // to register with ONOS as an application - described next
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected CoreService coreService;
+
     // to receive Packet-in events that we'll respond to
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected PacketService packetService;
+
     // end host information
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected HostService hostService;
+
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected VoltTenantService voltTenantService;
+
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected ComponentConfigService cfgService;
+
     // Parsed RADIUS server IP address
     protected InetAddress parsedRadiusIpAddress;
     // Parsed NAS IP address
     protected InetAddress parsedNasIpAddress;
-    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+
-    protected ComponentConfigService cfgService;
     // our application-specific event handler
     private ReactivePacketProcessor processor = new ReactivePacketProcessor();
+
     // our unique identifier
     private ApplicationId appId;
+
     @Property(name = "radiusIpAddress", value = DEFAULT_RADIUS_IP,
             label = "RADIUS IP Address")
     private String radiusIpAddress = DEFAULT_RADIUS_IP;
+
     @Property(name = "nasIpAddress", value = DEFAULT_NAS_IP,
             label = "NAS IP Address")
     private String nasIpAddress = DEFAULT_NAS_IP;
+
     @Property(name = "radiusMacAddress", value = RADIUS_MAC_ADDRESS,
             label = "RADIUS MAC Address")
     private String radiusMacAddress = RADIUS_MAC_ADDRESS;
+
     @Property(name = "nasMacAddress", value = NAS_MAC_ADDRESS,
             label = "NAS MAC Address")
     private String nasMacAddress = NAS_MAC_ADDRESS;
+
     @Property(name = "radiusSecret", value = DEFAULT_RADIUS_SECRET,
             label = "RADIUS shared secret")
     private String radiusSecret = DEFAULT_RADIUS_SECRET;
+
     @Property(name = "radiusSwitchId", value = DEFAULT_RADIUS_SWITCH,
             label = "Radius switch")
     private String radiusSwitch = DEFAULT_RADIUS_SWITCH;
+
     @Property(name = "radiusPortNumber", value = DEFAULT_RADIUS_PORT,
             label = "Radius port")
     private String radiusPort = DEFAULT_RADIUS_PORT;
@@ -181,14 +204,14 @@ public class AAA {
             parsedRadiusIpAddress = InetAddress.getByName(s);
             radiusIpAddress = Strings.isNullOrEmpty(s) ? DEFAULT_RADIUS_IP : s;
         } catch (UnknownHostException e) {
-            log.error("Invalid RADIUS IP address specification: {}", s);
+            log.error("Invalid RADIUS IP address specification: {}", s, e);
         }
         try {
             s = Tools.get(properties, "nasIpAddress");
             parsedNasIpAddress = InetAddress.getByName(s);
             nasIpAddress = Strings.isNullOrEmpty(s) ? DEFAULT_NAS_IP : s;
         } catch (UnknownHostException e) {
-            log.error("Invalid NAS IP address specification: {}", s);
+            log.error("Invalid NAS IP address specification: {}", s, e);
         }
         s = Tools.get(properties, "radiusMacAddress");
@@ -284,6 +307,7 @@ public class AAA {
             if (ethPkt == null) {
                 return;
             }
+            try {
                 // identify if incoming packet comes from supplicant (EAP) or RADIUS
                 switch (EthType.EtherType.lookup(ethPkt.getEtherType())) {
                     case EAPOL:
@@ -299,17 +323,17 @@ public class AAA {
                             byte[] datagram = udpPacket.getPayload().serialize();
                             RADIUS radiusPacket;
-                        try {
                             radiusPacket = RADIUS.deserializer().deserialize(datagram, 0, datagram.length);
-                        } catch (DeserializationException e) {
-                            log.warn("Unable to deserialize RADIUS packet:", e);
-                            return;
-                        }
                             handleRadiusPacket(radiusPacket);
                         }
+
                         break;
                     default:
-                    return;
+                        log.trace("Skipping Ethernet packet type {}",
+                                  EthType.EtherType.lookup(ethPkt.getEtherType()));
+                }
+            } catch (DeserializationException | StateMachineException e) {
+                log.warn("Unable to process RADIUS packet:", e);
             }
         }
@@ -319,7 +343,7 @@ public class AAA {
          *
          * @param inPacket Ethernet packet coming from the supplicant
          */
-        private void handleSupplicantPacket(InboundPacket inPacket) {
+        private void handleSupplicantPacket(InboundPacket inPacket) throws StateMachineException {
             Ethernet ethPkt = inPacket.parsed();
             // Where does it come from?
             MacAddress srcMAC = ethPkt.getSourceMAC();
@@ -337,7 +361,6 @@ public class AAA {
             switch (eapol.getEapolType()) {
                 case EAPOL.EAPOL_START:
-                    try {
                     stateMachine.start();
                     stateMachine.setSupplicantConnectpoint(inPacket.receivedFrom());
@@ -350,26 +373,27 @@ public class AAA {
                     stateMachine.setVlanId(ethPkt.getVlanID());
                     this.sendPacketToSupplicant(eth, stateMachine.supplicantConnectpoint());
-                    } catch (StateMachineException e) {
-                        e.printStackTrace();
-                    }
                     break;
                 case EAPOL.EAPOL_PACKET:
+                    RADIUS radiusPayload;
                     //check if this is a Response/Identify or  a Response/TLS
                     EAP eapPacket = (EAP) eapol.getPayload();
                     byte dataType = eapPacket.getDataType();
                     switch (dataType) {
+
                     case EAP.ATTR_IDENTITY:
-                            try {
                         //request id access to RADIUS
-                                RADIUS radiusPayload = new RADIUS(RADIUS.RADIUS_CODE_ACCESS_REQUEST,
+                        stateMachine.setUsername(eapPacket.getData());
+
+                        radiusPayload =
+                                new RADIUS(RADIUS.RADIUS_CODE_ACCESS_REQUEST,
                                            eapPacket.getIdentifier());
                         radiusPayload.setIdentifier(stateMachine.identifier());
                         radiusPayload.setAttribute(RADIUSAttribute.RADIUS_ATTR_USERNAME,
                                                    eapPacket.getData());
-                                stateMachine.setUsername(eapPacket.getData());
+
                         radiusPayload.setAttribute(RADIUSAttribute.RADIUS_ATTR_NAS_IP,
                                                    AAA.this.parsedNasIpAddress.getAddress());
@@ -382,16 +406,13 @@ public class AAA {
                         //change the state to "PENDING"
                         stateMachine.requestAccess();
-                            } catch (StateMachineException e) {
-                                e.printStackTrace();
-                            }
                         break;
                         case EAP.ATTR_MD5:
                             //verify if the EAP identifier corresponds to the challenge identifier from the client state
                             //machine.
                             if (eapPacket.getIdentifier() == stateMachine.challengeIdentifier()) {
                                 //send the RADIUS challenge response
-                                RADIUS radiusPayload = new RADIUS(RADIUS.RADIUS_CODE_ACCESS_REQUEST,
+                                radiusPayload = new RADIUS(RADIUS.RADIUS_CODE_ACCESS_REQUEST,
                                                            eapPacket.getIdentifier());
                                 radiusPayload.setIdentifier(stateMachine.challengeIdentifier());
                                 radiusPayload.setAttribute(RADIUSAttribute.RADIUS_ATTR_USERNAME,
@@ -408,9 +429,8 @@ public class AAA {
                             }
                             break;
                         case EAP.ATTR_TLS:
-                            try {
                             //request id access to RADIUS
-                                RADIUS radiusPayload = new RADIUS(RADIUS.RADIUS_CODE_ACCESS_REQUEST,
+                            radiusPayload = new RADIUS(RADIUS.RADIUS_CODE_ACCESS_REQUEST,
                                                        eapPacket.getIdentifier());
                             radiusPayload.setIdentifier(stateMachine.identifier());
                             radiusPayload.setAttribute(RADIUSAttribute.RADIUS_ATTR_USERNAME,
@@ -429,16 +449,14 @@ public class AAA {
                             sendRadiusMessage(radiusPayload);
                             // TODO: this gets called on every fragment, should only be called at TLS-Start
                             stateMachine.requestAccess();
-                            } catch (StateMachineException e) {
+
-                                e.printStackTrace();
-                            }
                             break;
                         default:
                             return;
                     }
                     break;
                 default:
-                    return;
+                    log.trace("Skipping EAPOL message {}", eapol.getEapolType());
             }
         }
@@ -447,15 +465,15 @@ public class AAA {
          *
          * @param radiusPacket RADIUS packet coming from the RADIUS server.
          */
-        private void handleRadiusPacket(RADIUS radiusPacket) {
+        private void handleRadiusPacket(RADIUS radiusPacket) throws StateMachineException {
             StateMachine stateMachine = StateMachine.lookupStateMachineById(radiusPacket.getIdentifier());
             if (stateMachine == null) {
                 log.error("Invalid session identifier, exiting...");
                 return;
             }
-            EAP eapPayload = new EAP();
+            EAP eapPayload;
-            Ethernet eth = null;
+            Ethernet eth;
             switch (radiusPacket.getCode()) {
                 case RADIUS.RADIUS_CODE_ACCESS_CHALLENGE:
                     byte[] challengeState = radiusPacket.getAttribute(RADIUSAttribute.RADIUS_ATTR_STATE).getValue();
@@ -467,7 +485,6 @@ public class AAA {
                     this.sendPacketToSupplicant(eth, stateMachine.supplicantConnectpoint());
                     break;
                 case RADIUS.RADIUS_CODE_ACCESS_ACCEPT:
-                    try {
                     //send an EAPOL - Success to the supplicant.
                     byte[] eapMessage =
                             radiusPacket.getAttribute(RADIUSAttribute.RADIUS_ATTR_EAP_MESSAGE).getValue();
@@ -479,16 +496,9 @@ public class AAA {
                     this.sendPacketToSupplicant(eth, stateMachine.supplicantConnectpoint());
                     stateMachine.authorizeAccess();
-                    } catch (StateMachineException e) {
-                        e.printStackTrace();
-                    }
                     break;
                 case RADIUS.RADIUS_CODE_ACCESS_REJECT:
-                    try {
                     stateMachine.denyAccess();
-                    } catch (StateMachineException e) {
-                        e.printStackTrace();
-                    }
                     break;
                 default:
                     log.warn("Unknown RADIUS message received with code: {}", radiusPacket.getCode());