You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2014/04/26 23:27:19 UTC

svn commit: r1590300 - in /tomcat/trunk: java/org/apache/catalina/core/AprLifecycleListener.java java/org/apache/catalina/core/LocalStrings.properties java/org/apache/tomcat/jni/SSL.java webapps/docs/changelog.xml webapps/docs/config/listeners.xml

Author: kkolinko
Date: Sat Apr 26 21:27:18 2014
New Revision: 1590300

URL: http://svn.apache.org/r1590300
Log:
Fix http://issues.apache.org/bugzilla/show_bug.cgi?id=56027
Alternative implementation for the new values of FIPSMode option in AprLifecycleListener

Modified:
    tomcat/trunk/java/org/apache/catalina/core/AprLifecycleListener.java
    tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties
    tomcat/trunk/java/org/apache/tomcat/jni/SSL.java
    tomcat/trunk/webapps/docs/changelog.xml
    tomcat/trunk/webapps/docs/config/listeners.xml

Modified: tomcat/trunk/java/org/apache/catalina/core/AprLifecycleListener.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AprLifecycleListener.java?rev=1590300&r1=1590299&r2=1590300&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/AprLifecycleListener.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/AprLifecycleListener.java Sat Apr 26 21:27:18 2014
@@ -70,16 +70,18 @@ public class AprLifecycleListener
     protected static boolean aprInitialized = false;
     protected static boolean aprAvailable = false;
     protected static boolean fipsModeActive = false;
+
     /**
-     * FIPS_mode documentation states that the return value will be
-     * whatever value was originally passed-in to FIPS_mode_set().
-     * FIPS_mode_set docs say the argument should be non-zero to enter
-     * FIPS mode, and that upon success, the return value will be the
-     * same as the argument passed-in. Docs also highly recommend
-     * that the value "1" be used "to avoid compatibility issues".
-     * In order to avoid the argument and check-value from getting out
-     * of sync for some reason, we are using the class constant
-     * FIPS_ON here.
+     * The "FIPS mode" level that we use as the argument to OpenSSL method
+     * <code>FIPS_mode_set()</code> to enable FIPS mode and that we expect as
+     * the return value of <code>FIPS_mode()</code> when FIPS mode is enabled.
+     * <p>
+     * In the future the OpenSSL library might grow support for different
+     * non-zero "FIPS" modes that specify different allowed subsets of ciphers
+     * or whatever, but nowadays only "1" is the supported value.
+     * </p>
+     * @see "OpenSSL method <a href="http://wiki.openssl.org/index.php/FIPS_mode_set%28%29">FIPS_mode_set()</a>"
+     * @see "OpenSSL method <a href="http://wiki.openssl.org/index.php/FIPS_mode%28%29">FIPS_mode()</a>"
      */
     private static final int FIPS_ON = 1;
 
@@ -234,10 +236,7 @@ public class AprLifecycleListener
         aprAvailable = true;
     }
 
-    private static void initializeSSL()
-        throws ClassNotFoundException, NoSuchMethodException,
-               IllegalAccessException, InvocationTargetException
-    {
+    private static void initializeSSL() throws Exception {
 
         if ("off".equalsIgnoreCase(SSLEngine)) {
             return;
@@ -264,73 +263,61 @@ public class AprLifecycleListener
         method = clazz.getMethod(methodName, paramTypes);
         method.invoke(null, paramValues);
 
-        final boolean enterFipsMode;
+        if (!(null == FIPSMode || "off".equalsIgnoreCase(FIPSMode))) {
 
-        if("on".equalsIgnoreCase(FIPSMode)
-           || "require".equalsIgnoreCase(FIPSMode)) {
-            // FIPS_mode documentation states that the return value will be
-            // whatever value was originally passed-in to FIPS_mode_set().
-            // FIPS_mode_set docs say the argument should be non-zero to enter
-            // FIPS mode, and that upon success, the return value will be the
-            // same as the argument passed-in. Docs also highly recommend
-            // that the value "1" be used "to avoid compatibility issues".
-            // In order to avoid the argument and check-value from getting out
-            // of sync for some reason, we are using the class constant
-            // FIPS_ON here.
-            int fipsModeState;
-            try {
-                fipsModeState = SSL.fipsModeGet();
-            } catch (Exception e) {
-                throw new IllegalStateException(e);
-            }
+            fipsModeActive = false;
+
+            final boolean enterFipsMode;
+            int fipsModeState = SSL.fipsModeGet();
 
-            if(log.isDebugEnabled())
+            if(log.isDebugEnabled()) {
                 log.debug(sm.getString("aprListener.currentFIPSMode",
                                        Integer.valueOf(fipsModeState)));
+            }
 
-            // Return values: 0=Not in FIPS mode, 1=In FIPS mode,
-            // exception if FIPS totally unavailable
-            enterFipsMode = 1 != fipsModeState;
-
-            if("on".equalsIgnoreCase(FIPSMode)) {
-                if(!enterFipsMode)
+            if ("on".equalsIgnoreCase(FIPSMode)) {
+                if (fipsModeState == FIPS_ON) {
                     log.info(sm.getString("aprListener.skipFIPSInitialization"));
-            } else if("require".equalsIgnoreCase(FIPSMode)) {
-                if(enterFipsMode) {
-                    String message = sm.getString("aprListener.alreadyInFIPSMode");
+                    fipsModeActive = true;
+                    enterFipsMode = false;
+                } else {
+                    enterFipsMode = true;
+                }
+            } else if ("require".equalsIgnoreCase(FIPSMode)) {
+                if (fipsModeState == FIPS_ON) {
+                    fipsModeActive = true;
+                    enterFipsMode = false;
+                } else {
+                    throw new IllegalStateException(
+                            sm.getString("aprListener.requireNotInFIPSMode"));
+                }
+            } else if ("enter".equalsIgnoreCase(FIPSMode)) {
+                if (fipsModeState == 0) {
+                    enterFipsMode = true;
+                } else {
+                    throw new IllegalStateException(sm.getString(
+                            "aprListener.enterAlreadyInFIPSMode",
+                            Integer.valueOf(fipsModeState)));
+                }
+            } else {
+                throw new IllegalArgumentException(sm.getString(
+                        "aprListener.wrongFIPSMode", FIPSMode));
+            }
+
+            if (enterFipsMode) {
+                log.info(sm.getString("aprListener.initializingFIPS"));
+
+                fipsModeState = SSL.fipsModeSet(FIPS_ON);
+                if (fipsModeState != FIPS_ON) {
+                    // This case should be handled by the native method,
+                    // but we'll make absolutely sure, here.
+                    String message = sm.getString("aprListener.initializeFIPSFailed");
                     log.error(message);
                     throw new IllegalStateException(message);
                 }
-            }
-        }
-        else if("enter".equalsIgnoreCase(FIPSMode)) {
-            enterFipsMode = true;
-        } else
-            enterFipsMode = false;
-
-        if(enterFipsMode) {
-            log.info(sm.getString("aprListener.initializingFIPS"));
-
-            // FIPS_mode_set docs say the argument should be non-zero to enter
-            // FIPS mode, and that upon success, the return value will be the
-            // same as the argument passed-in. Docs also highly recommend
-            // that the value "1" be used "to avoid compatibility issues".
-            // In order to avoid the argument and check-value from getting out
-            // of sync for some reason, we are using the class constant
-            // FIPS_ON here.
-            final int result = SSL.fipsModeSet(FIPS_ON);
 
-            // success is defined as return value = last argument to FIPS_mode_set()
-            if(FIPS_ON == result) {
                 fipsModeActive = true;
-
                 log.info(sm.getString("aprListener.initializeFIPSSuccess"));
-            } else {
-                // This case should be handled by the native method,
-                // but we'll make absolutely sure, here.
-                String message = sm.getString("aprListener.initializeFIPSFailed");
-                log.error(message);
-                throw new IllegalStateException(message);
             }
         }
 

Modified: tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties?rev=1590300&r1=1590299&r2=1590300&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties Sat Apr 26 21:27:18 2014
@@ -59,7 +59,9 @@ aprListener.tcnValid=Loaded APR based Ap
 aprListener.flags=APR capabilities: IPv6 [{0}], sendfile [{1}], accept filters [{2}], random [{3}].
 aprListener.currentFIPSMode=Current FIPS mode: {0}
 aprListener.skipFIPSInitialization=Already in FIPS mode; skipping FIPS initialization.
-aprListener.alreadyInFIPSMode=AprLifecycleListener requested to force entering FIPS mode, but FIPS mode was already enabled.
+aprListener.enterAlreadyInFIPSMode=AprLifecycleListener is configured to force entering FIPS mode, but library is already in FIPS mode ({0})
+aprListener.requireNotInFIPSMode=AprLifecycleListener is configured to require the library to already be in FIPS mode, but it was not in FIPS mode
+aprListener.wrongFIPSMode=Unexpected value of FIPSMode option of AprLifecycleListener: "{0}"
 aprListener.initializingFIPS=Initializing FIPS mode...
 aprListener.initializeFIPSSuccess=Successfully entered FIPS mode
 aprListener.initializeFIPSFailed=Failed to enter FIPS mode

Modified: tomcat/trunk/java/org/apache/tomcat/jni/SSL.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/jni/SSL.java?rev=1590300&r1=1590299&r2=1590300&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/jni/SSL.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/jni/SSL.java Sat Apr 26 21:27:18 2014
@@ -232,8 +232,10 @@ public final class SSL {
     /**
      * Get the status of FIPS Mode.
      *
-     * @return 0 If OpenSSL is not in FIPS mode, 1 if OpenSSL is in FIPS Mode.
+     * @return FIPS_mode return code. It is <code>0</code> if OpenSSL is not
+     *  in FIPS mode, <code>1</code> if OpenSSL is in FIPS Mode.
      * @throws Exception If tcnative was not compiled with FIPS Mode available.
+     * @see "OpenSSL method <a href="http://wiki.openssl.org/index.php/FIPS_mode%28%29">FIPS_mode()</a>"
      */
     public static native int fipsModeGet() throws Exception;
 
@@ -243,8 +245,11 @@ public final class SSL {
      * @param mode 1 - enable, 0 - disable
      *
      * @return FIPS_mode_set return code
+     * @throws Exception If tcnative was not compiled with FIPS Mode available,
+     *  or if {@code FIPS_mode_set()} call returned an error value.
+     * @see "OpenSSL method <a href="http://wiki.openssl.org/index.php/FIPS_mode_set%28%29">FIPS_mode_set()</a>"
      */
-    public static native int fipsModeSet(int mode);
+    public static native int fipsModeSet(int mode) throws Exception;
 
     /**
      * Add content of the file to the PRNG

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1590300&r1=1590299&r2=1590300&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Sat Apr 26 21:27:18 2014
@@ -60,8 +60,8 @@
         7&apos;s but it is still buggy. (markt)
       </fix>
       <fix>
-        <bug>56027</bug>: Add more nuanced options for managing FIPS mode in the
-        AprLifecycleListener. (schultz)
+        <bug>56027</bug>: Add more options for managing FIPS mode in the
+        AprLifecycleListener. (schultz/kkolinko)
       </fix>
       <fix>
         <bug>56320</bug>: Fix a file descriptor leak in the default servlet when

Modified: tomcat/trunk/webapps/docs/config/listeners.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/listeners.xml?rev=1590300&r1=1590299&r2=1590300&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/listeners.xml (original)
+++ tomcat/trunk/webapps/docs/config/listeners.xml Sat Apr 26 21:27:18 2014
@@ -121,9 +121,9 @@
         mode).
         FIPS mode <em>requires you to have a FIPS-capable OpenSSL library which
         you must build yourself</em>.
-        If this attribute is set to any of the above values, <b>SSLEngine</b>
-        must be enabled as well for any effect.
-        The default value is <code>off</code>.</p>
+        If this attribute is set to any of the above values, the <b>SSLEngine</b>
+        must be enabled as well.</p>
+        <p>The default value is <code>off</code>.</p>
       </attribute>
 
     </attributes>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org