[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r313178602
##
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusAuthenticationProviderModule.java
##
@@ -57,6 +62,24 @@ public
RadiusAuthenticationProviderModule(AuthenticationProvider authProvider)
// Get local environment
this.environment = new LocalEnvironment();
+
+// Check for MD4 requirement
+RadiusAuthenticationProtocol authProtocol =
environment.getProperty(RadiusGuacamoleProperties.RADIUS_AUTH_PROTOCOL);
+RadiusAuthenticationProtocol innerProtocol =
environment.getProperty(RadiusGuacamoleProperties.RADIUS_EAP_TTLS_INNER_PROTOCOL);
+if ((authProtocol != null
+&& (authProtocol == RadiusAuthenticationProtocol.MSCHAPv1
+|| authProtocol == RadiusAuthenticationProtocol.MSCHAPv2))
+|| (innerProtocol != null
+&& (innerProtocol == RadiusAuthenticationProtocol.MSCHAPv1
+|| innerProtocol ==
RadiusAuthenticationProtocol.MSCHAPv2))) {
Review comment:
Cleaned up.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r278371816
##
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##
@@ -0,0 +1,64 @@
+/*
+ * To change this license header, choose License Headers in Project Properties.
+ * To change this template file, choose Tools | Templates
+ * and open the template in the editor.
+ */
+package org.apache.guacamole.auth.radius.conf;
+
+/**
+ * This enum represents supported RADIUS authentication protocols for
+ * the guacamole-auth-radius extension.
+ */
+public enum RadiusAuthenticationProtocol {
+
+// Password authentication protocol
+PAP("pap"),
+
+// Challenge-Handshake AUthentication Protocol
+CHAP("chap"),
+
+// Microsoft CHAP version 1
+MSCHAPv1("mschapv1"),
+
+// Microsoft CHAP version 2
+MSCHAPv2("mschapv2"),
+
+// Extensible authentication protocol with MD5 hashing.
+EAP_MD5("eap-md5"),
+
+// Extensible authentication protocol with TLS
+EAP_TLS("eap-tls"),
+
+// Extensible authentication protocol with Tunneled TLS
+EAP_TTLS("eap-ttls");
+
+// Store the string value used in the configuration file.
+private final String strValue;
+
+/**
+ * Create a new RadiusAuthenticationProtocol object having the
+ * given string value.
+ *
+ * @param strValue
+ * The string value of the protocol.
+ */
+public RadiusAuthenticationProtocol(String strValue) {
+this.strValue = strValue;
+}
+
+@Override
+public String toString() {
Review comment:
Doc'd.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r278371733
##
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##
@@ -0,0 +1,64 @@
+/*
+ * To change this license header, choose License Headers in Project Properties.
+ * To change this template file, choose Tools | Templates
+ * and open the template in the editor.
+ */
+package org.apache.guacamole.auth.radius.conf;
+
+/**
+ * This enum represents supported RADIUS authentication protocols for
+ * the guacamole-auth-radius extension.
+ */
+public enum RadiusAuthenticationProtocol {
+
+// Password authentication protocol
+PAP("pap"),
+
+// Challenge-Handshake AUthentication Protocol
+CHAP("chap"),
+
+// Microsoft CHAP version 1
+MSCHAPv1("mschapv1"),
+
+// Microsoft CHAP version 2
+MSCHAPv2("mschapv2"),
+
+// Extensible authentication protocol with MD5 hashing.
+EAP_MD5("eap-md5"),
+
+// Extensible authentication protocol with TLS
+EAP_TLS("eap-tls"),
+
+// Extensible authentication protocol with Tunneled TLS
+EAP_TTLS("eap-ttls");
+
+// Store the string value used in the configuration file.
+private final String strValue;
+
+/**
+ * Create a new RadiusAuthenticationProtocol object having the
+ * given string value.
+ *
+ * @param strValue
+ * The string value of the protocol.
Review comment:
Took a better slice at this documentation.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r278371670
##
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##
@@ -0,0 +1,64 @@
+/*
+ * To change this license header, choose License Headers in Project Properties.
+ * To change this template file, choose Tools | Templates
+ * and open the template in the editor.
+ */
+package org.apache.guacamole.auth.radius.conf;
+
+/**
+ * This enum represents supported RADIUS authentication protocols for
+ * the guacamole-auth-radius extension.
+ */
+public enum RadiusAuthenticationProtocol {
+
+// Password authentication protocol
Review comment:
JavaDoc'd.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r278371697
##
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##
@@ -0,0 +1,64 @@
+/*
+ * To change this license header, choose License Headers in Project Properties.
+ * To change this template file, choose Tools | Templates
+ * and open the template in the editor.
+ */
+package org.apache.guacamole.auth.radius.conf;
+
+/**
+ * This enum represents supported RADIUS authentication protocols for
+ * the guacamole-auth-radius extension.
+ */
+public enum RadiusAuthenticationProtocol {
+
+// Password authentication protocol
+PAP("pap"),
+
+// Challenge-Handshake AUthentication Protocol
+CHAP("chap"),
+
+// Microsoft CHAP version 1
+MSCHAPv1("mschapv1"),
+
+// Microsoft CHAP version 2
+MSCHAPv2("mschapv2"),
+
+// Extensible authentication protocol with MD5 hashing.
+EAP_MD5("eap-md5"),
+
+// Extensible authentication protocol with TLS
+EAP_TLS("eap-tls"),
+
+// Extensible authentication protocol with Tunneled TLS
+EAP_TTLS("eap-ttls");
+
+// Store the string value used in the configuration file.
Review comment:
JavaDoc'd, and added some detail.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2 URL: https://github.com/apache/guacamole-client/pull/392#discussion_r278371425 ## File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocolProperty.java ## @@ -0,0 +1,38 @@ +/* + * To change this license header, choose License Headers in Project Properties. Review comment: As with others, I think this is actually okay in the most recent code. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2 URL: https://github.com/apache/guacamole-client/pull/392#discussion_r278371471 ## File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java ## @@ -0,0 +1,64 @@ +/* + * To change this license header, choose License Headers in Project Properties. Review comment: I think this is good in most recent code. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r278371176
##
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/conf/RadiusAuthenticationProtocol.java
##
@@ -0,0 +1,64 @@
+/*
+ * To change this license header, choose License Headers in Project Properties.
+ * To change this template file, choose Tools | Templates
+ * and open the template in the editor.
+ */
+package org.apache.guacamole.auth.radius.conf;
+
+/**
+ * This enum represents supported RADIUS authentication protocols for
+ * the guacamole-auth-radius extension.
+ */
+public enum RadiusAuthenticationProtocol {
+
+// Password authentication protocol
+PAP("pap"),
+
+// Challenge-Handshake AUthentication Protocol
+CHAP("chap"),
+
+// Microsoft CHAP version 1
+MSCHAPv1("mschapv1"),
+
+// Microsoft CHAP version 2
+MSCHAPv2("mschapv2"),
+
+// Extensible authentication protocol with MD5 hashing.
+EAP_MD5("eap-md5"),
+
+// Extensible authentication protocol with TLS
+EAP_TLS("eap-tls"),
+
+// Extensible authentication protocol with Tunneled TLS
+EAP_TTLS("eap-ttls");
+
+// Store the string value used in the configuration file.
+private final String strValue;
+
+/**
+ * Create a new RadiusAuthenticationProtocol object having the
+ * given string value.
+ *
+ * @param strValue
+ * The string value of the protocol.
+ */
+public RadiusAuthenticationProtocol(String strValue) {
+this.strValue = strValue;
+}
+
+@Override
+public String toString() {
+return strValue;
+}
+
+@Override
+public static RadiusAuthenticationProtocol valueOf(String value) {
Review comment:
Again, not sure if you were looking at a specific commit instead of the
latest round of code, but the latest has `getEnum()` defined (which I failed to
document, so that needs fixing) - turns out that I don't think it's even valid
to override `valueOf()` for `enum`s.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2 URL: https://github.com/apache/guacamole-client/pull/392#discussion_r278370105 ## File path: extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java ## @@ -68,6 +70,27 @@ private ConfigurationService confService; +/** + * Review comment: Not seeing this in the current set of commits...were you looking at an earlier commit?? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r276471238
##
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java
##
@@ -129,6 +133,18 @@ private RadiusAuthenticator
setupRadiusAuthenticator(RadiusClient radiusClient)
if (radAuth == null)
throw new GuacamoleException("Could not get a valid
RadiusAuthenticator for specified protocol: " +
confService.getRadiusAuthProtocol());
+// For MSCHAPv1/2, we need MD4 support
+if (radAuth instanceof MSCHAPv1Authenticator
+|| radAuth instanceof MSCHAPv2Authenticator) {
Review comment:
This should be implemented as requested.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r276471178
##
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java
##
@@ -129,6 +133,18 @@ private RadiusAuthenticator
setupRadiusAuthenticator(RadiusClient radiusClient)
if (radAuth == null)
throw new GuacamoleException("Could not get a valid
RadiusAuthenticator for specified protocol: " +
confService.getRadiusAuthProtocol());
+// For MSCHAPv1/2, we need MD4 support
+if (radAuth instanceof MSCHAPv1Authenticator
+|| radAuth instanceof MSCHAPv2Authenticator) {
+
+Security.addProvider(new Provider("MD4", 0.00, "MD4 for MSCHAPv1/2
RADIUS") {
Review comment:
Okay, I've put the initialization for this into the Guice module, which I
think will suffice to load it only a single time.
I queried the JRadius repository for any other instances of "MD4", and the
MSCHAP library is the only place it shows up, so I think that covers the
requirement:
https://github.com/coova/jradius/search?q=MD4&unscoped_q=MD4
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r273660800
##
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java
##
@@ -129,6 +133,18 @@ private RadiusAuthenticator
setupRadiusAuthenticator(RadiusClient radiusClient)
if (radAuth == null)
throw new GuacamoleException("Could not get a valid
RadiusAuthenticator for specified protocol: " +
confService.getRadiusAuthProtocol());
+// For MSCHAPv1/2, we need MD4 support
+if (radAuth instanceof MSCHAPv1Authenticator
+|| radAuth instanceof MSCHAPv2Authenticator) {
Review comment:
Okay, I'll work up a new property type and `enum` for this.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in MD4 support for MSCHAPv1/2
necouchman commented on a change in pull request #392: GUACAMOLE-774: Add in
MD4 support for MSCHAPv1/2
URL: https://github.com/apache/guacamole-client/pull/392#discussion_r273660601
##
File path:
extensions/guacamole-auth-radius/src/main/java/org/apache/guacamole/auth/radius/RadiusConnectionService.java
##
@@ -129,6 +133,18 @@ private RadiusAuthenticator
setupRadiusAuthenticator(RadiusClient radiusClient)
if (radAuth == null)
throw new GuacamoleException("Could not get a valid
RadiusAuthenticator for specified protocol: " +
confService.getRadiusAuthProtocol());
+// For MSCHAPv1/2, we need MD4 support
+if (radAuth instanceof MSCHAPv1Authenticator
+|| radAuth instanceof MSCHAPv2Authenticator) {
+
+Security.addProvider(new Provider("MD4", 0.00, "MD4 for MSCHAPv1/2
RADIUS") {
Review comment:
Yeah, probably a good point. I could move it to the constructor for the
`RadiusConnectionService` class, but not sure if that would still happen each
time someone logged in, or would be a one-shot thing?
> Is it known that no other auth protocols use MD4?
I do not know one way or the other if others require it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
