Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-client/pull/122#discussion_r127611971
--- Diff:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java
---
@@ -0,0 +1,352 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.guacamole.auth.radius;
+
+import com.google.inject.Inject;
+import java.io.File;
+import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.security.NoSuchAlgorithmException;
+import org.apache.guacamole.GuacamoleException;
+import org.apache.guacamole.GuacamoleUnsupportedException;
+import org.apache.guacamole.environment.LocalEnvironment;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import net.jradius.client.RadiusClient;
+import net.jradius.exception.RadiusException;
+import net.jradius.packet.RadiusPacket;
+import net.jradius.packet.AccessRequest;
+import net.jradius.dictionary.*;
+import net.jradius.packet.attribute.AttributeList;
+import net.jradius.packet.attribute.RadiusAttribute;
+import net.jradius.client.auth.EAPTLSAuthenticator;
+import net.jradius.client.auth.EAPTTLSAuthenticator;
+import net.jradius.client.auth.RadiusAuthenticator;
+import net.jradius.client.auth.PEAPAuthenticator;
+import net.jradius.packet.attribute.AttributeFactory;
+import net.jradius.packet.AccessChallenge;
+import net.jradius.packet.RadiusResponse;
+
+/**
+ * Service for creating and managing connections to RADIUS servers.
+ */
+public class RadiusConnectionService {
+
+ /**
+ * Logger for this class.
+ */
+ private final Logger logger =
LoggerFactory.getLogger(RadiusConnectionService.class);
+
+ /**
+ * Service for retrieving RADIUS server configuration information.
+ */
+ @Inject
+ private ConfigurationService confService;
+
+
+ /**
+ * The RADIUS client;
+ */
+ private RadiusClient radiusClient;
+
+ /**
+ * Creates a new instance of RadiusClient, configured with parameters
+ * from guacamole.properties.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while parsing guacamole.properties, or if the
+ * configuration of RadiusClient fails.
+ */
+ private void createRadiusConnection() {
+
+ // Create the RADIUS client with the configuration parameters
+ try {
+ radiusClient = new
RadiusClient(InetAddress.getByName(confService.getRadiusServer()),
+
confService.getRadiusSharedSecret(),
+
confService.getRadiusAuthPort(),
+
confService.getRadiusAcctPort(),
+
confService.getRadiusTimeout());
+ }
+ catch (GuacamoleException e) {
+ logger.error("Unable to initialize RADIUS client: {}",
e.getMessage());
+ logger.debug("Failed to init RADIUS client.", e);
+ }
+ catch (UnknownHostException e) {
+ logger.error("Unable to resolve host: {}", e.getMessage());
+ logger.debug("Failed to resolve host.", e);
+ }
+ catch (IOException e) {
+ logger.error("Unable to communicate with host: {}",
e.getMessage());
+ logger.debug("Failed to communicate with host.", e);
+ }
+
+ }
+
+ /**
+ * Creates a new instance of RadiusAuthentictor, configured with
+ * parameters specified within guacamole.properties.
+ *
+ * @return
+ * A new RadiusAuthenticator instance which has been configured
+ * with parameters from guacamole.properties, or null if
+ * configuration fails.
+ */
+ private RadiusAuthenticator setupRadiusAuthenticator() throws
GuacamoleException {
+
+ // If we don't have a radiusClient object, yet, don't go any
further.
+ if (radiusClient == null) {
+ logger.error("RADIUS client hasn't been set up, yet.");
+ logger.debug("We can't run this method until the RADIUS client
has been set up.");
+ return null;
+ }
+
+ // Pull configuration parameters from guacamole.properties
+ LocalEnvironment guacEnv = new LocalEnvironment();
+ String guacHome = guacEnv.getGuacamoleHome().getAbsolutePath();
+ String caFile = confService.getRadiusCAFile();
+ String caPassword = confService.getRadiusCAPassword();
+ String keyFile = confService.getRadiusKeyFile();
+ String keyPassword = confService.getRadiusKeyPassword();
+ String innerProtocol = confService.getRadiusEAPTTLSInnerProtocol();
+
+ RadiusAuthenticator radAuth =
radiusClient.getAuthProtocol(confService.getRadiusAuthProtocol());
+ if (radAuth == null)
+ throw new GuacamoleException("Could not get a valid
RadiusAuthenticator for specified protocol: " +
confService.getRadiusAuthProtocol());
+
+ // If we're using any of the TLS protocols, we need to configure
them
+ if (radAuth instanceof PEAPAuthenticator ||
+ radAuth instanceof EAPTLSAuthenticator ||
+ radAuth instanceof EAPTTLSAuthenticator) {
+
+ if (caFile != null) {
+ ((EAPTLSAuthenticator)radAuth).setCaFile((new
File(guacHome, caFile)).toString());
+
((EAPTLSAuthenticator)radAuth).setCaFileType(confService.getRadiusCAType());
+ if (caPassword != null)
+
((EAPTLSAuthenticator)radAuth).setCaPassword(caPassword);
+ }
+
+ if (keyPassword != null)
+ ((EAPTLSAuthenticator)radAuth).setKeyPassword(keyPassword);
+
+ ((EAPTLSAuthenticator)radAuth).setKeyFile((new File(guacHome,
keyFile)).toString());
+
((EAPTLSAuthenticator)radAuth).setKeyFileType(confService.getRadiusKeyType());
+
((EAPTLSAuthenticator)radAuth).setTrustAll(confService.getRadiusTrustAll());
+ }
+
+ // If we're using EAP-TTLS, we need to define tunneled protocol
+ if (radAuth instanceof EAPTTLSAuthenticator) {
+ if (innerProtocol == null)
+ throw new GuacamoleException("Trying to use EAP-TTLS, but
no inner protocol specified.");
+
+
((EAPTTLSAuthenticator)radAuth).setInnerProtocol(innerProtocol);
+ }
+
+ return radAuth;
+
+ }
+
+ /**
+ * Authenticate to the RADIUS server and return the response from the
+ * server.
+ *
+ * @param username
+ * The username for authentication.
+ * @param password
+ * The password for authentication.
+ *
+ * @return
+ * A RadiusPacket with the response of the server.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while talking to the server.
+ */
+ public RadiusPacket authenticate(String username, String password)
+ throws GuacamoleException {
+
+ // If a username hasn't been provided, stop
+ if (username == null || username.isEmpty()) {
+ logger.warn("Anonymous access not allowed with RADIUS
client.");
+ return null;
+ }
+
+ // If a password hasn't been provided, stop
+ if (password == null || password.isEmpty()) {
+ logger.warn("Password required for RADIUS authentication.");
+ return null;
+ }
+
+ // Create the connection and load the attribute dictionary
+ createRadiusConnection();
+
AttributeFactory.loadAttributeDictionary("net.jradius.dictionary.AttributeDictionaryImpl");
+
+ // If the client is null, we return null - something has gone wrong
+ if (radiusClient == null)
+ return null;
+
+ RadiusAuthenticator radAuth = setupRadiusAuthenticator();
+
+ if (radAuth == null)
+ throw new GuacamoleException("Unknown RADIUS authentication
protocol.");
+
+ // Set up attributes, create the access request, and send the
packet
+ try {
+ AttributeList radAttrs = new AttributeList();
+ radAttrs.add(new Attr_UserName(username));
+ radAttrs.add(new Attr_UserPassword(password));
+ radAttrs.add(new Attr_CleartextPassword(password));
+
+ AccessRequest radAcc = new AccessRequest(radiusClient);
+
+ // EAP-TTLS tunnels protected attributes inside the TLS layer
+ if (radAuth instanceof EAPTTLSAuthenticator) {
+ radAuth.setUsername(new Attr_UserName(username));
+
((EAPTTLSAuthenticator)radAuth).setTunneledAttributes(radAttrs);
+ }
+ else
+ radAcc.addAttributes(radAttrs);
+
+ radAuth.setupRequest(radiusClient, radAcc);
+ radAuth.processRequest(radAcc);
+ RadiusResponse reply = radiusClient.sendReceive(radAcc,
confService.getRadiusRetries());
+
+ // We receive a Challenge not asking for user input, so
silently process the challenge
+ while((reply instanceof AccessChallenge) &&
(reply.findAttribute(Attr_ReplyMessage.TYPE) == null)) {
+ radAuth.processChallenge(radAcc, reply);
+ reply = radiusClient.sendReceive(radAcc,
confService.getRadiusRetries());
+ }
+ return reply;
+ }
+
+ catch (RadiusException e) {
+ logger.error("Unable to complete authentication.",
e.getMessage());
+ logger.debug("Authentication with RADIUS failed.", e);
+ return null;
+ }
+
+ catch (NoSuchAlgorithmException e) {
+ logger.error("No such RADIUS algorithm: {}", e.getMessage());
+ logger.debug("Unknown RADIUS algorithm.", e);
+ return null;
+ }
+ }
+
+ /**
+ * Authenticate to the RADIUS server using existing state and a
response
+ *
+ * @param username
+ * The username for the authentication
+ * @param state
+ * The previous state of the RADIUS connection
+ * @param response
+ * The response to the RADIUS challenge
+ *
+ * @return
+ * A RadiusPacket with the response of the server.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while talking to the server.
+ */
+ public RadiusPacket authenticate(String username, String state, String
response)
--- End diff --
> Null for response, and an additional field for password. I think I
figured the function calls were different enough that it made sense to split
them out, but I guess if the code is similar enough, it probably doesn't make
much sense.
Ah, I didn't notice that was `state` and not `password`.
I definitely agree that a prototype like `authenticate(String username,
String password, String state, String response)` wouldn't be reasonable.
Perhaps there's some other way to unify the common logic? Is the only
difference between these functions the way that the `AttributeList` is
populated?
> Could just pass in credentials and have this function look at the
provided credentials object and process that, but that feels like a little bit
more than just managing RADIUS connections, which is the purpose of this
class...
I agree passing in `Credentials` would be bad. This service shouldn't
concern itself with the Guacamole side of the challenge/response conversation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
