From f3186148daf2a3fead3fdf20e92b7cb14464f74a Mon Sep 17 00:00:00 2001
From: coulbeck <coulbeck@localhost>
Date: Thu, 15 Mar 2007 18:50:44 +0000
Subject: [PATCH] Fix some more TODOs in the ACI.

---
 opends/src/server/org/opends/server/authorization/dseecompat/AciMessages.java                      |   19 +++
 opends/src/server/org/opends/server/authorization/dseecompat/AciEvalContext.java                   |   16 +
 opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java                     |  127 ++++++++------------
 opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/AciTests.java |   10 +
 opends/src/server/org/opends/server/authorization/dseecompat/AuthMethod.java                       |   86 +++++++-------
 opends/src/server/org/opends/server/authorization/dseecompat/EnumAuthMethod.java                   |   65 +---------
 6 files changed, 141 insertions(+), 182 deletions(-)

diff --git a/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java b/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java
index 498f52a..baea9bd 100644
--- a/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java
+++ b/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java
@@ -30,10 +30,10 @@
 import org.opends.server.types.*;
 import org.opends.server.api.ClientConnection;
 import org.opends.server.api.Group;
+import org.opends.server.api.ConnectionSecurityProvider;
 import org.opends.server.core.AddOperation;
 import org.opends.server.core.Operation;
 import org.opends.server.extensions.TLSConnectionSecurityProvider;
-import org.opends.server.util.ServerConstants;
 import java.net.InetAddress;
 import java.util.LinkedList;
 
@@ -353,88 +353,63 @@
     }
 
     /**
-     * Try to determine the authentication information from the current
-     * client connection. The checks are for simple and SASL, anything else
-     * is not a match. If the bind rule requires any SSL client authentication
-     * information then the "wantSSL" flag should be set. This code is used by
-     * the authmethod bind rule keyword.
-     *
-     * @param wantSSL True if the bind rule needs the ssl client auth check.
-     * @return  Return an enumeration containing the authentication method
-     * type for this client connection.
+     * {@inheritDoc}
      */
-    public EnumAuthMethod getAuthenticationMethod(boolean wantSSL) {
-        EnumAuthMethod method=EnumAuthMethod.AUTHMETHOD_NOMATCH;
+    public EnumEvalResult hasAuthenticationMethod(EnumAuthMethod authMethod,
+                                                  String saslMech) {
+      EnumEvalResult matched=EnumEvalResult.FALSE;
+
+      if(authMethod==EnumAuthMethod.AUTHMETHOD_NONE) {
+        /**
+         * None actually means any, in that we don't care what method was used.
+         * This doesn't seem very intuitive or useful, but that's the way it is.
+         */
+        matched = EnumEvalResult.TRUE;
+      } else {
+        /*
+         * Some kind of authentication is required.
+         */
         AuthenticationInfo authInfo=clientConnection.getAuthenticationInfo();
         if(authInfo.isAuthenticated()) {
-            if(authInfo.hasAuthenticationType(AuthenticationType.SIMPLE))
-                method=EnumAuthMethod.AUTHMETHOD_SIMPLE;
-            else if(authInfo.hasAuthenticationType(AuthenticationType.SASL))
-                method=getSaslAuthenticationMethod(authInfo, wantSSL);
-            else
-                method=EnumAuthMethod.AUTHMETHOD_NOMATCH;
+          if(authMethod==EnumAuthMethod.AUTHMETHOD_SIMPLE) {
+            if(authInfo.hasAuthenticationType(AuthenticationType.SIMPLE)) {
+              matched = EnumEvalResult.TRUE;
+            }
+          } else if(authMethod == EnumAuthMethod.AUTHMETHOD_SSL) {
+            /*
+             * This means authentication using a certificate over TLS.
+             *
+             * We check the following:
+             * - SASL EXTERNAL has been used, and
+             * - TLS is the security provider, and
+             * - The client provided a certificate.
+             */
+            if (authInfo.hasAuthenticationType(AuthenticationType.SASL) &&
+                 authInfo.hasSASLMechanism(saslMech)) {
+              ConnectionSecurityProvider provider =
+                    clientConnection.getConnectionSecurityProvider();
+              if (provider instanceof TLSConnectionSecurityProvider) {
+                TLSConnectionSecurityProvider tlsProvider =
+                      (TLSConnectionSecurityProvider) provider;
+                 if (tlsProvider.getClientCertificateChain() != null) {
+                   matched = EnumEvalResult.TRUE;
+                 }
+              }
+            }
+          } else {
+            // A particular SASL mechanism.
+            if (authInfo.hasAuthenticationType(AuthenticationType.SASL) &&
+                 authInfo.hasSASLMechanism(saslMech)) {
+              matched = EnumEvalResult.TRUE;
+            }
+          }
         }
-        return method;
-    }
-
-    /*
-     * TODO This method needs to be tested.
-     * TODO Investigate multi-factor authentication.
-     *   Second, OpenDS is devised so that it could be possible to use
-     *   multi-factor or step-up authentication, in which the same client
-     *   has provided multiple forms of credentials, but this method
-     *   expects only a single authentication type.
-     */
-    /**
-     * This method attempts to figure out what the SASL method was/is or
-     * what the client auth is.
-     * @param authInfo The authentication information to use.
-     * @param wantSSL The bin drule wants the SSL client auth status.
-     * @return An enumeration containing the SASL bind information.
-     */
-    private EnumAuthMethod
-    getSaslAuthenticationMethod(AuthenticationInfo authInfo, boolean wantSSL) {
-        EnumAuthMethod method=EnumAuthMethod.AUTHMETHOD_NOMATCH;
-        if(authInfo.hasAuthenticationType(AuthenticationType.SASL)) {
-            if(authInfo.hasSASLMechanism(ServerConstants.
-                    SASL_MECHANISM_DIGEST_MD5))
-                method=EnumAuthMethod.AUTHMETHOD_SASL_MD5;
-            else if(authInfo.hasSASLMechanism(ServerConstants.
-                    SASL_MECHANISM_GSSAPI))
-                method=EnumAuthMethod.AUTHMETHOD_SASL_GSSAPI;
-            else if(authInfo.hasSASLMechanism(ServerConstants.
-                    SASL_MECHANISM_EXTERNAL)) {
-                /*
-                 * The bind rule wants ssl client auth information. Need the
-                 * security provider to see if the clientAuthPolicy is
-                 * required. If it is optional, we really can't determine if
-                 * the client auth.
-                */
-                if(wantSSL) {
-                    String mechName=
-                        clientConnection.getConnectionSecurityProvider().
-                                getSecurityMechanismName();
-                    if(mechName.equalsIgnoreCase("TLS")) {
-                        TLSConnectionSecurityProvider tlsProv=
-                            (TLSConnectionSecurityProvider)clientConnection.
-                                           getConnectionSecurityProvider();
-                        SSLClientAuthPolicy clientAuthPolicy=
-                            tlsProv.getSSLClientAuthPolicy();
-                        if(clientAuthPolicy == SSLClientAuthPolicy.REQUIRED)
-                            method=EnumAuthMethod.AUTHMETHOD_SSL;
-                    } else
-                       method=EnumAuthMethod.AUTHMETHOD_NOMATCH;
-                } else {
-                    method=EnumAuthMethod.AUTHMETHOD_SASL_EXTERNAL;
-                }
-            } else
-                method=EnumAuthMethod.AUTHMETHOD_NOMATCH;
-        }
-        return method;
+      }
+      return matched;
     }
 
     /**
-     * Convienance method that checks if the the clientDN is a member of the
+     * Convenience method that checks if the the clientDN is a member of the
      * specified group.
      * @param group The group to check membership in.
      * @return True if the clientDN is a member of the specified group.
diff --git a/opends/src/server/org/opends/server/authorization/dseecompat/AciEvalContext.java b/opends/src/server/org/opends/server/authorization/dseecompat/AciEvalContext.java
index 8f50949..3d94604 100644
--- a/opends/src/server/org/opends/server/authorization/dseecompat/AciEvalContext.java
+++ b/opends/src/server/org/opends/server/authorization/dseecompat/AciEvalContext.java
@@ -110,12 +110,18 @@
     public String getHostName();
 
     /**
-     * Get the authentication method.
-     * @param wantSSL The authmethod bind rule needs the SSL client auth
-     * status.
-     * @return An Enumeration of the auth method bound as.
+     * Determine whether the client connection has been authenticated using
+     * a specified authentication method.  This method is used for the
+     * authmethod bind rule keyword.
+     *
+     * @param authMethod The required authentication method.
+     * @param saslMech The required SASL mechanism if the authentication method
+     * is SASL.
+     * @return An evaluation result indicating whether the client connection
+     * has been authenticated using the required authentication method.
      */
-    public EnumAuthMethod getAuthenticationMethod(boolean wantSSL);
+    public EnumEvalResult hasAuthenticationMethod(EnumAuthMethod authMethod,
+                                                  String saslMech);
 
     /**
      * Get the  address of the bound connection.
diff --git a/opends/src/server/org/opends/server/authorization/dseecompat/AciMessages.java b/opends/src/server/org/opends/server/authorization/dseecompat/AciMessages.java
index 716fefb..fba579b 100644
--- a/opends/src/server/org/opends/server/authorization/dseecompat/AciMessages.java
+++ b/opends/src/server/org/opends/server/authorization/dseecompat/AciMessages.java
@@ -610,6 +610,16 @@
         int MSGID_ACI_SYNTAX_INVALID_ATTRIBUTE_TYPE_NAME =
          CATEGORY_MASK_ACCESS_CONTROL | SEVERITY_MASK_SEVERE_WARNING | 59;
 
+     /**
+      * The message ID for the message that will be used if a bind rule
+      * authmethod expression contains a SASL mechanism that is not currrently
+      * registered in the server.  This takes one argument, which is the
+      * SASL mechanism string parsed from the authmethod expression.
+      */
+     public static final
+        int MSGID_ACI_SYNTAX_DUBIOUS_AUTHMETHOD_SASL_MECHANISM =
+         CATEGORY_MASK_ACCESS_CONTROL | SEVERITY_MASK_NOTICE | 60;
+
     /**
      * Associates a set of generic messages with the message IDs defined in
      * this class.
@@ -773,7 +783,14 @@
                 "The provided Access Control Instruction (ACI) bind rule " +
                 "authmethod expression value \"%s\" is invalid. A valid " +
                 "authmethod value is one of the following: none, simple," +
-                "SSL, sasl EXTERNAL, sasl DIGEST-MD5, or sasl GSSAPI.");
+                "SSL, or \"sasl mechanism\", where mechanism is one of the" +
+                "supported SASL mechanisms including CRAM-MD5, DIGEST-MD5, " +
+                "and GSSAPI.");
+
+        registerMessage(MSGID_ACI_SYNTAX_DUBIOUS_AUTHMETHOD_SASL_MECHANISM,
+                "The SASL mechanism \"%s\" provided in the Access Control " +
+                "Instruction (ACI) bind rule authmethod expression is not " +
+                "one of the currently registered mechanisms in the server");
 
         registerMessage(MSGID_ACI_SYNTAX_INVALID_USERATTR_EXPRESSION,
                 "The provided Access Control Instruction (ACI) bind rule " +
diff --git a/opends/src/server/org/opends/server/authorization/dseecompat/AuthMethod.java b/opends/src/server/org/opends/server/authorization/dseecompat/AuthMethod.java
index 5adc0ec..b7d6984 100644
--- a/opends/src/server/org/opends/server/authorization/dseecompat/AuthMethod.java
+++ b/opends/src/server/org/opends/server/authorization/dseecompat/AuthMethod.java
@@ -29,6 +29,10 @@
 
 import static org.opends.server.authorization.dseecompat.AciMessages.*;
 import static org.opends.server.messages.MessageHandler.getMessage;
+import org.opends.server.core.DirectoryServer;
+import static org.opends.server.loggers.Error.logError;
+import org.opends.server.types.ErrorLogCategory;
+import org.opends.server.types.ErrorLogSeverity;
 
 /**
  * The AuthMethod class represents an authmethod bind rule keyword expression.
@@ -40,6 +44,11 @@
      */
     private EnumAuthMethod authMethod=null;
 
+    /**
+     * The SASL mechanism if the authentication method is SASL.
+     */
+    private String saslMech = null;
+
     /*
      * Enumeration representing the bind rule operation type.
      */
@@ -48,73 +57,62 @@
     /**
      * Create a class representing an authmethod bind rule keyword from the
      * provided method and bind rule type.
-     * @param method An enumeration representing the method of the expression.
      * @param type An enumeration representing the type of the expression.
      */
-    private AuthMethod(EnumAuthMethod method, EnumBindRuleType type) {
+    private AuthMethod(EnumAuthMethod method, String saslMech,
+                       EnumBindRuleType type) {
         this.authMethod=method;
+        this.saslMech = saslMech;
         this.type=type;
     }
 
     /**
-     * Decode a string representing a authmethod bind rule.
+     * Decode a string representing an authmethod bind rule.
      * @param expr  The string representing the bind rule.
      * @param type An enumeration representing the bind rule type.
-     * @return  An keyword bind rule class that can be used to evaluate the
+     * @return  A keyword bind rule class that can be used to evaluate the
      * bind rule.
      * @throws AciException If the expression string is invalid.
      */
     public static KeywordBindRule decode(String expr, EnumBindRuleType type)
     throws AciException  {
-        EnumAuthMethod method=EnumAuthMethod.createAuthmethod(expr);
-        if (method == null)
-        {
-            int msgID = MSGID_ACI_SYNTAX_INVALID_AUTHMETHOD_EXPRESSION;
-            String message = getMessage(msgID, expr);
-            throw new AciException(msgID, message);
+      String lowerExpr = expr.toLowerCase();
+      if (lowerExpr.equals("none"))
+      {
+        return new AuthMethod(EnumAuthMethod.AUTHMETHOD_NONE, null, type);
+      }
+      else if (lowerExpr.equals("simple"))
+      {
+        return new AuthMethod(EnumAuthMethod.AUTHMETHOD_SIMPLE, null, type);
+      }
+      else if (lowerExpr.equals("ssl"))
+      {
+        return new AuthMethod(EnumAuthMethod.AUTHMETHOD_SSL, "EXTERNAL", type);
+      }
+      else if (expr.length() > 5 && lowerExpr.startsWith("sasl "))
+      {
+        String saslMech = expr.substring(5);
+        if (DirectoryServer.getSASLMechanismHandler(saslMech) == null) {
+          int msgID = MSGID_ACI_SYNTAX_DUBIOUS_AUTHMETHOD_SASL_MECHANISM;
+          logError(ErrorLogCategory.ACCESS_CONTROL,
+                   ErrorLogSeverity.NOTICE, msgID, saslMech);
         }
-        return new AuthMethod(method, type);
+        return new AuthMethod(EnumAuthMethod.AUTHMETHOD_SASL, saslMech, type);
+      }
+
+      int msgID = MSGID_ACI_SYNTAX_INVALID_AUTHMETHOD_EXPRESSION;
+      String message = getMessage(msgID, expr);
+      throw new AciException(msgID, message);
     }
 
-    /*
-     * TODO Evaluate if AUTHMETHOD_NONE processing is correct. This was fixed
-     * prior to Neil's review. Verify in a unit test.
-     *
-     * I'm not sure that the evaluate() method handles AUTHMETHOD_NONE
-     * correctly. My understanding is that it should only match in cases
-     * in which no authentication has been performed, but you have it
-     * always matching.
-     */
     /**
      * Evaluate authmethod bind rule using the provided evaluation context.
      * @param evalCtx  An evaluation context to use.
      * @return  An enumeration evaluation result.
      */
     public EnumEvalResult evaluate(AciEvalContext evalCtx) {
-        EnumEvalResult matched=EnumEvalResult.FALSE;
-        if(authMethod==EnumAuthMethod.AUTHMETHOD_NONE) {
-            matched=EnumEvalResult.TRUE;
-        } else if(authMethod==EnumAuthMethod.AUTHMETHOD_SIMPLE) {
-            if(evalCtx.getAuthenticationMethod(false)
-                    == EnumAuthMethod.AUTHMETHOD_SIMPLE){
-                matched=EnumEvalResult.TRUE;
-            }
-        } else if(authMethod == EnumAuthMethod.AUTHMETHOD_SSL) {
-            /*
-             * TODO Verfiy that SSL authemethod is correctly handled in a
-             * unit test.
-             * I'm not sure that the evaluate() method correctly handles
-             * SASL EXTERNAL in all cases.  My understanding is that in
-             * DS 5/6, an authmethod of SSL is the same as an authmethod of
-             * SASL EXTERNAL.  If that's true, then you don't properly handle
-             * that condition.
-             */
-            if(authMethod == evalCtx.getAuthenticationMethod(true))
-                    matched=EnumEvalResult.TRUE;
-        } else {
-            if(authMethod ==evalCtx.getAuthenticationMethod(false))
-                matched=EnumEvalResult.TRUE;
-        }
+        EnumEvalResult matched =
+             evalCtx.hasAuthenticationMethod(authMethod, saslMech);
         return matched.getRet(type, false);
     }
 }
diff --git a/opends/src/server/org/opends/server/authorization/dseecompat/EnumAuthMethod.java b/opends/src/server/org/opends/server/authorization/dseecompat/EnumAuthMethod.java
index 7f3fe72..7e170fd 100644
--- a/opends/src/server/org/opends/server/authorization/dseecompat/EnumAuthMethod.java
+++ b/opends/src/server/org/opends/server/authorization/dseecompat/EnumAuthMethod.java
@@ -26,17 +26,7 @@
  */
 
 package org.opends.server.authorization.dseecompat;
-/*
- * TODO Evaluate moving this to a non-enumeration class that can add
- * SASL mechanisms dynamically.
- *
- *  Given our previous discussion about needing to support any kind of SASL
- *  mechanism that may be registered with the server, perhaps an enum isn't
- *  the right way to handle this because we don't know ahead of time what
- *  auth methods might be available (certainly not at compile time, but
- *  potentially not even at runtime since I can add support for a new SASL
- *  mechanism on the fly without restarting the server).
- */
+
 /**
  * This class provides an enumeration of the allowed authmethod types.
  */
@@ -47,35 +37,24 @@
      * none.
      */
     AUTHMETHOD_NONE          ("none"),
+
     /**
-      * The enumeration type when the bind rule has specified authentication of
-     *  simple.
+     * The enumeration type when the bind rule has specified authentication of
+     * simple.
      */
     AUTHMETHOD_SIMPLE        ("simple"),
+
     /**
-      * The enumeration type when the bind rule has specified authentication of
-     *  ssl client auth.
+     * The enumeration type when the bind rule has specified authentication of
+     * ssl client auth.
      */
     AUTHMETHOD_SSL           ("ssl"),
+
     /**
      * The enumeration type when the bind rule has specified authentication of
-     * sasl DIGEST-MD5.
+     * a sasl mechanism.
      */
-    AUTHMETHOD_SASL_MD5      ("sasl DIGEST-MD5"),
-    /**
-     * The enumeration type when the bind rule has specified authentication of
-     * sasl EXTERNAL.
-     */
-    AUTHMETHOD_SASL_EXTERNAL ("sasl EXTERNAL"),
-    /**
-     * The enumeration type when the bind rule has specified authentication of
-     * sasl GSSAPI.
-     */
-    AUTHMETHOD_SASL_GSSAPI   ("sasl GSSAPI"),
-    /**
-     * Special internal enumeration for when there is no match.
-     */
-    AUTHMETHOD_NOMATCH       ("nomatch");
+    AUTHMETHOD_SASL          ("sasl");
 
     /*
      * The name of the authmethod.
@@ -90,28 +69,4 @@
         this.authmethod = authmethod;
     }
 
-    /**
-     * Checks if a authmethod name is equal to this enumeration.
-     * @param myauthmethod  The name to test for.
-     * @return  True if the names match.
-     */
-    public boolean isAuthMethod(String myauthmethod){
-        return myauthmethod.equalsIgnoreCase(this.authmethod);
-    }
-
-    /**
-     * Creates an authmethod enumeration from the name passed in.
-     * @param myauthmethod The name to create.
-     * @return An authmethod enumeration if the name was found or null if not.
-     */
-    public static EnumAuthMethod createAuthmethod(String myauthmethod){
-        if (myauthmethod != null){
-            for (EnumAuthMethod t : EnumAuthMethod.values()){
-                if (t.isAuthMethod(myauthmethod)){
-                    return t;
-                }
-            }
-        }
-        return null;
-    }
 }
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/AciTests.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/AciTests.java
index 7e32912..3437f32 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/AciTests.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/AciTests.java
@@ -208,7 +208,7 @@
 
   private static final String BIND_RULE_AUTHMETHOD_SIMPLE = "authmethod=\"simple\"";
   private static final String BIND_RULE_AUTHMETHOD_SSL = "authmethod=\"ssl\"";
-  private static final String BIND_RULE_AUTHMETHOD_SASL = "authmethod=\"sasl\"";
+  private static final String BIND_RULE_AUTHMETHOD_SASL_DIGEST_MD5 = "authmethod=\"sasl DIGEST-MD5\"";
 
   // Admin, but not anonymous
   private static final String BIND_RULE_USERDN_NOT_ADMIN = and(not(BIND_RULE_USERDN_ADMIN), BIND_RULE_AUTHMETHOD_SIMPLE);
@@ -373,6 +373,9 @@
   private static final String ALLOW_ALL_TO_SSL =
           buildAciValue("name", "allow all to ssl", "targetattr", "*", "allow(all)", BIND_RULE_AUTHMETHOD_SSL);
 
+  private static final String ALLOW_ALL_TO_SASL_DIGEST_MD5 =
+          buildAciValue("name", "allow all to sasl DIGEST-MD5", "targetattr", "*", "allow(all)", BIND_RULE_AUTHMETHOD_SASL_DIGEST_MD5);
+
   private static final String DENY_ALL_TO_SIMPLE =
           buildAciValue("name", "deny all to simple", "targetattr", "*", "deny(all)", BIND_RULE_AUTHMETHOD_SIMPLE);
 
@@ -581,6 +584,7 @@
      DENY_ALL_TO_DNS_LOCALHOST,
      buildAciValue("name", "deny all to example.com", "targetattr", "*", "deny(all)", "dns=\"*.example.com\""),
      ALLOW_ALL_TO_SSL,
+     ALLOW_ALL_TO_SASL_DIGEST_MD5,
      DENY_ALL_TO_SIMPLE,
      DENY_ALL_TODAY,
      DENY_ALL_TODAY_AND_TOMORROW,
@@ -1043,6 +1047,9 @@
   private static final String ALLOW_ALL_BASE_TO_SSL_AUTH =
           makeAddAciLdif(OU_BASE_DN, ALLOW_ALL_TO_SSL);
 
+  private static final String ALLOW_ALL_BASE_TO_SASL_DIGEST_MD5_AUTH =
+          makeAddAciLdif(OU_BASE_DN, ALLOW_ALL_TO_SASL_DIGEST_MD5);
+
   private static final String ALLOW_ALL_BASE_DENY_ALL_TO_SIMPLE_AUTH =
           makeAddAciLdif(OU_BASE_DN, ALLOW_ALL_TO_ALL) +
           makeAddAciLdif(OU_INNER_DN, DENY_ALL_TO_SIMPLE);
@@ -1215,6 +1222,7 @@
             ALLOW_ALL_BASE_DENY_ALL_TO_MISC_AND_LOCALHOST,
             ALLOW_ALL_BASE_TO_NON_DNS_LOCALHOST,
             ALLOW_ALL_BASE_TO_SSL_AUTH,
+            ALLOW_ALL_BASE_TO_SASL_DIGEST_MD5_AUTH,
             ALLOW_ALL_BASE_DENY_ALL_TO_SIMPLE_AUTH,
             ALLOW_ALL_BASE_DENY_ALL_TODAY,
             ALLOW_ALL_BASE_DENY_ALL_TODAY_AND_TOMORROW,

--
Gitblit v1.10.0