You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by sc...@apache.org on 2014/04/15 03:14:41 UTC

svn commit: r1587379 - 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/config/listeners.xml

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;
 
     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();
+
+            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.
+            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>
 



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


Re: svn commit: r1587379 - 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/config/listeners.xml

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Tim,

On 4/15/14, 5:30 PM, Tim Whittington wrote:
> 
> On 16/04/2014, at 8:30 am, Christopher Schultz <ch...@christopherschultz.net> wrote:
> 
>> Tim,
>>
>> On 4/14/14, 10:45 PM, Tim Whittington wrote:
>>>
>>> On 15/04/2014, at 1:14 pm, schultz@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).
>>
>> Fair enough.
>>
>>> 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).
>>
>> FIPS_mode() is documented to return whatever was passed to it before.
>> There is no other information that I could find available. I'm willing
>> to bet that everyone always uses "1" since the API basically says "use 1".
>>
>>>>  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?
>>
>> No. Other code could call FIPS_mode_set() and thus the return value
>> might be different. Yes, this is documented identically in multiple
>> places. I didn't want anyone to say "oh, having a constant for (int)1 is
>> stupid: let's just use 1 and be done with it" without actually reading
>> why there is a constant being used there.
> 
> OK, I had a look at the TCN implementation and see it doesn’t do much.
> Maybe it’d be cleaner to deal with the openssl API there, and thunk the possible values in SSL.fipsModeGet/Set to 0/1.

Please file a bug. It doesn't seem high-priority to make that change
right now. It also will require a change to tcnative which generally has
a slower release cycle.

>>>> +
>>>> +            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.
>>
>> This can't be in TCN because the Java code needs the constant value. I
>> suppose I could call a function in tcnative to get the value of "1" but
>> that seems a bit of overkill and I'd have to wait for yet another
>> tcnative release to get this out there.
> 
> I was more thinking about making the SSL.fipsMode* API accept/return only 0/1, which would make these comments unnecessary.
> Sorry, wasn’t super-clear on that.

Okay.

>>>> +            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?
>>
>> Please read the enhancement request where I've described the reasoning
>> behind this.
> 
> I read 56027 and your description of the options, but it doesn’t
> explain why those options are useful beyond a simple “I want FIPS”, “I
> don’t want/care about FIPS” toggle.
> 
> i.e. enter and require both enable FIPS mode if successful

No, "require" will not ever enter FIPS mode: it requires that OpenSSL
already be in FIPS mode. If it's not, it will fail. "enter" will enter
FIPS mode if necessary, but skip the FIPS_mode_enter call if we're
already in FIPS mode.

> so what do you gain by being able to ensure that something prior to
> Tomcat startup has or hasn’t already enabled FIPS mode?

You may want to validate that the system is in the state you expect:
FIPS-mode already enabled. Perhaps you don't want to start-up if that's
not the case. It's mostly a sanity check.

> (I haven’t dealt with a lot of FIPS site requirements, so there may
> well be something I’m missing here).

The original report was that calling FIPS_mode_set(1) when the library
is already in FIPS mode causes OpenSSL complain and the
AprLifecycleListener to fail to start. This basically makes it not
possible to "ensure" the server is in FIPS mode. Since I was going to
modify the behavior, I figured I would give the user some options.

Do you think it makes things too confusing?

Most people will just use FIPSmode="on" which is completely
backwards-compatible -- except that this one failure scenario is now
eliminated. The "enter" and "require" options were opportunities I
thought made sense to take.

>>> 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?
>>
>> I can change that if you feel it is important.
> 
> If the server was booted in FIPS mode and I explicitly (implicitly?)
> set FIPSMode=“off” then it would be unexpected for openssl to be
> limiting the connector to FIPS mode cipher suites.

I wouldn't say that /not/ specifying FIPSMode should be the same as
FIPSMode="off". I think you should have to explicitly disable FIPSMode.
Honestly, I can't imagine anyone intentionally disabling FIPS mode, but
I suppose if I'm going to bother to support some ... esoteric options,
then explicitly /exiting/ FIPS mode could be one of them.

Does this feel very important to you? Nobody ever asked for
FIPS-mode-exit, while the options I've added with this patch do address
some reasonable use-cases that have been brought up by a real person
asking for help.

>>> Current behaviour would appear to be ‘on’ == ‘make sure it’s on *with
>>> bug’, ‘off’ == ‘don’t really care, will take whatever’?
>>
>> Correct.
>>
>>> Would on/off still be sufficient, or do we need the four ‘require’,
>>> ‘support’, ‘not supported’, ‘initiate’ options ala transactions?
>>
>> I like being able to ensure that FIPS mode was already enabled.
> 
> This sounds like ‘require’ means ‘verify the server was booted in 
> FIPS mode', which sounds reassuring but not something that affects
> how Tomcat will operate?

Well, it will cause the AprLifecycleListener to fail if OpenSSL isn't
already in FIPS mode, so the connector won't work properly. So it's both
reassuring and will affect how Tomcat will operate.

-chris


Re: svn commit: r1587379 - 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/config/listeners.xml

Posted by Tim Whittington <ti...@apache.org>.
On 16/04/2014, at 8:30 am, Christopher Schultz <ch...@christopherschultz.net> wrote:

> Tim,
> 
> On 4/14/14, 10:45 PM, Tim Whittington wrote:
>> 
>> On 15/04/2014, at 1:14 pm, schultz@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).
> 
> Fair enough.
> 
>> 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).
> 
> FIPS_mode() is documented to return whatever was passed to it before.
> There is no other information that I could find available. I'm willing
> to bet that everyone always uses "1" since the API basically says "use 1".
> 
>>>  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?
> 
> No. Other code could call FIPS_mode_set() and thus the return value
> might be different. Yes, this is documented identically in multiple
> places. I didn't want anyone to say "oh, having a constant for (int)1 is
> stupid: let's just use 1 and be done with it" without actually reading
> why there is a constant being used there.

OK, I had a look at the TCN implementation and see it doesn’t do much.
Maybe it’d be cleaner to deal with the openssl API there, and thunk the possible values in SSL.fipsModeGet/Set to 0/1.

> 
>>> +
>>> +            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.
> 
> This can't be in TCN because the Java code needs the constant value. I
> suppose I could call a function in tcnative to get the value of "1" but
> that seems a bit of overkill and I'd have to wait for yet another
> tcnative release to get this out there.

I was more thinking about making the SSL.fipsMode* API accept/return only 0/1, which would make these comments unnecessary.
Sorry, wasn’t super-clear on 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?
> 
> Please read the enhancement request where I've described the reasoning
> behind this.

I read 56027 and your description of the options, but it doesn’t explain why those options are useful beyond a simple “I want FIPS”, “I don’t want/care about FIPS” toggle.

i.e. enter and require both enable FIPS mode if successful, so what do you gain by being able to ensure that something prior to Tomcat startup has or hasn’t already enabled FIPS mode?

(I haven’t dealt with a lot of FIPS site requirements, so there may well be something I’m missing here).

> 
>> 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?
> 
> I can change that if you feel it is important.

If the server was booted in FIPS mode and I explicitly (implicitly?) set FIPSMode=“off” then it would be unexpected for openssl to be limiting the connector to FIPS mode cipher suites.

> 
>> Current behaviour would appear to be ‘on’ == ‘make sure it’s on *with
>> bug’, ‘off’ == ‘don’t really care, will take whatever’?
> 
> Correct.
> 
>> Would on/off still be sufficient, or do we need the four ‘require’,
>> ‘support’, ‘not supported’, ‘initiate’ options ala transactions?
> 
> I like being able to ensure that FIPS mode was already enabled.

This sounds like ‘require’ means ‘verify the server was booted in FIPS mode', which sounds reassuring but not something that affects how Tomcat will operate?

> 
> -chris
> 


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


Re: svn commit: r1587379 - 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/config/listeners.xml

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Tim,

On 4/14/14, 10:45 PM, Tim Whittington wrote:
> 
> On 15/04/2014, at 1:14 pm, schultz@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).

Fair enough.

> 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).

FIPS_mode() is documented to return whatever was passed to it before.
There is no other information that I could find available. I'm willing
to bet that everyone always uses "1" since the API basically says "use 1".

>>   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?

No. Other code could call FIPS_mode_set() and thus the return value
might be different. Yes, this is documented identically in multiple
places. I didn't want anyone to say "oh, having a constant for (int)1 is
stupid: let's just use 1 and be done with it" without actually reading
why there is a constant being used there.

>> +
>> +            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.

This can't be in TCN because the Java code needs the constant value. I
suppose I could call a function in tcnative to get the value of "1" but
that seems a bit of overkill and I'd have to wait for yet another
tcnative release to get this out there.

>> +            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?

Please read the enhancement request where I've described the reasoning
behind this.

> 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?

I can change that if you feel it is important.

> Current behaviour would appear to be ‘on’ == ‘make sure it’s on *with
> bug’, ‘off’ == ‘don’t really care, will take whatever’?

Correct.

> Would on/off still be sufficient, or do we need the four ‘require’,
> ‘support’, ‘not supported’, ‘initiate’ options ala transactions?

I like being able to ensure that FIPS mode was already enabled.

-chris


Re: svn commit: r1587379 - 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/config/listeners.xml

Posted by Tim Whittington <ti...@apache.org>.
On 15/04/2014, at 1:14 pm, schultz@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1587379 - 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/config/listeners.xml

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Konstantin,

On 4/15/14, 1:21 PM, Konstantin Kolinko wrote:
> 2014-04-15 5:14 GMT+04:00  <sc...@apache.org>:
>> 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/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
> 
> The text below makes no sense.
> As you changed TCN_REQUIRED_PATCH in r1587378 to require 1.1.30,
> Tomcat will refuse to load older versions of TCNative.
> 
>> +        <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>

I'll remove that comment.

> Changelog entry for this change and for r1587378 =  where?

Lost in the circular-reasoning behind not modifying the changelog for
Tomcat 8 but Tomcat 8 being semi-released at this point. I'll make these
fixes in a new patch in the next few minutes.

Thanks,
-chris


Re: svn commit: r1587379 - 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/config/listeners.xml

Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-04-15 5:14 GMT+04:00  <sc...@apache.org>:
> 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/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

The text below makes no sense.
As you changed TCN_REQUIRED_PATCH in r1587378 to require 1.1.30,
Tomcat will refuse to load older versions of TCNative.

> +        <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>
>


Changelog entry for this change and for r1587378 =  where?

Best regards,
Konstantin Kolinko

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