On 15/04/2014, at 1:14 pm, schu...@apache.org wrote:

> Author: schultz
> Date: Tue Apr 15 01:14:40 2014
> New Revision: 1587379
> 
> URL: http://svn.apache.org/r1587379
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56027
> Add more nuanced support for entering/requiring FIPS mode when using APR 
> connector.
> 
> 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/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=1587379&r1=1587378&r2=1587379&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/core/AprLifecycleListener.java 
> (original)
> +++ tomcat/trunk/java/org/apache/catalina/core/AprLifecycleListener.java Tue 
> Apr 15 01:14:40 2014
> @@ -70,6 +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.
> +     */
> +    private static final int FIPS_ON = 1;


This looks like it belongs in SSL (as one of the valid return values of 
SSL.fipsModeGet).

Also, does FIPS_mode() always return 1 if FIPS mode is enabled at boot (I see 
there’s no adaptation of the return code in the TCN imll).


>   protected static final Object lock = new Object();
> 
> @@ -110,7 +122,7 @@ public class AprLifecycleListener
>                   }
>               }
>               // Failure to initialize FIPS mode is fatal
> -                if ("on".equalsIgnoreCase(FIPSMode) && !isFIPSModeActive()) {
> +                if (!(null == FIPSMode || "off".equalsIgnoreCase(FIPSMode)) 
> && !isFIPSModeActive()) {
>                   Error e = new Error(
>                           sm.getString("aprListener.initializeFIPSFailed"));
>                   // Log here, because thrown error might be not logged
> @@ -252,13 +264,59 @@ public class AprLifecycleListener
>       method = clazz.getMethod(methodName, paramTypes);
>       method.invoke(null, paramValues);
> 
> -        if("on".equalsIgnoreCase(FIPSMode)) {
> +        final boolean enterFipsMode;
> +
> +        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.
> +            final int fipsModeState = SSL.fipsModeGet();


I don’t think this needs to be documented twice, and maybe not at all?
Aren’t we already handling the FIPS_* ‘peculiarities' in TCN so that 
SSL.fipsModeGet() has a sane 0/1 return code?


> +
> +            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)
> +                    
> log.info(sm.getString("aprListener.skipFIPSInitialization"));
> +            } else if("require".equalsIgnoreCase(FIPSMode)) {
> +                if(enterFipsMode) {
> +                    String message = 
> sm.getString("aprListener.alreadyInFIPSMode");
> +                    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"));
> 
> -            int result = SSL.fipsModeSet(1);
> +            // 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.


Again, these docs look like they belong on SSL.fipsModeSet or in the TCN 
implementation of that.


> +            final int result = SSL.fipsModeSet(FIPS_ON);
> 
> -            // success is defined as return value = 1
> -            if(1 == result) {
> +            // success is defined as return value = last argument to 
> FIPS_mode_set()
> +            if(FIPS_ON == result) {
>               fipsModeActive = true;
> 
>               log.info(sm.getString("aprListener.initializeFIPSSuccess"));
> 
> 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=1587379&r1=1587378&r2=1587379&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties 
> (original)
> +++ tomcat/trunk/java/org/apache/catalina/core/LocalStrings.properties Tue 
> Apr 15 01:14:40 2014
> @@ -57,6 +57,9 @@ aprListener.aprDestroy=Failed shutdown o
> aprListener.sslInit=Failed to initialize the SSLEngine.
> aprListener.tcnValid=Loaded APR based Apache Tomcat Native library {0} using 
> APR version {1}.
> 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.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=1587379&r1=1587378&r2=1587379&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/jni/SSL.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/jni/SSL.java Tue Apr 15 01:14:40 2014
> @@ -230,6 +230,14 @@ public final class SSL {
>   public static native int initialize(String engine);
> 
>   /**
> +     * Get the status of FIPS Mode.
> +     *
> +     * @return 0 If OpenSSL is not in FIPS mode, 1 if OpenSSL is in FIPS 
> Mode.
> +     * @throws Exception If tcnative was not compiled with FIPS Mode 
> available.
> +     */
> +    public static native int fipsModeGet();
> +
> +    /**
>    * Enable/Disable FIPS Mode.
>    *
>    * @param mode 1 - enable, 0 - disable
> 
> Modified: tomcat/trunk/webapps/docs/config/listeners.xml
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/listeners.xml?rev=1587379&r1=1587378&r2=1587379&view=diff
> ==============================================================================
> --- tomcat/trunk/webapps/docs/config/listeners.xml (original)
> +++ tomcat/trunk/webapps/docs/config/listeners.xml Tue Apr 15 01:14:40 2014
> @@ -112,12 +112,22 @@
>     </attribute>
> 
>     <attribute name="FIPSMode" required="false">
> -        <p>Set to <code>on</code> to instruct OpenSSL to go into FIPS mode.
> +        <p>Set to <code>on</code> to request that OpenSSL be in FIPS mode
> +        (if OpenSSL is already in FIPS mode, it will remain in FIPS mode).
> +        Set to <code>enter</code> to force OpenSSL to enter FIPS mode (an 
> error
> +        will occur if OpenSSL is already in FIPS mode).
> +        Set to <code>require</code> to require that OpenSSL <i>already</i> be
> +        in FIPS mode (an error will occur if OpenSSL is not already in FIPS
> +        mode).
>       FIPS mode <em>requires you to have a FIPS-capable OpenSSL library which
>       you must build yourself</em>.
> -        FIPS mode also requires Tomcat native library version 1.1.23 or 
> later,
> -        which <em>must be built against the FIPS-compatible OpenSSL</em> 
> library.
> -        If this attribute is "on", <b>SSLEngine</b> must be enabled as well.
> +        <code>FIPSMode="on"</code> or <code>FIPSMode="require"</code> 
> requires
> +        Tomcat native library version 1.1.30 or later, while
> +        <code>FIPSMode="enter"</code> can probably be done with Tomcat native
> +        library version 1.2.23 or later -- either of which <em>must be built
> +        against the FIPS-compatible OpenSSL</em> library.
> +        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>
>     </attribute>
> 
> 


Why the ‘enter’ and ‘require’ options?

I can understand ‘on’ == ‘I want FIPS mode’, but I don’t get how ‘require’ == 
‘I require FIPS mode turned on before Tomcat starts’ affects Tomcat, and I 
don’t get how ‘enter’ == ‘Try FIPS mode or fail if already set’ is useful.

Also, it seems inconsistent given the above that ‘off’ does nothing if TCN is 
already in FIPS mode at startup?

Current behaviour would appear to be ‘on’ == ‘make sure it’s on *with bug’, 
‘off’ == ‘don’t really care, will take whatever’?
Would on/off still be sufficient, or do we need the four ‘require’, ‘support’, 
‘not supported’, ‘initiate’ options ala transactions?

cheers
tim


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

Reply via email to