tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Surprising implementation of SSL.hasOp
Date Wed, 03 Oct 2012 16:02:36 GMT
All,

I was preparing a 6.0 patch for
https://issues.apache.org/bugzilla/show_bug.cgi?id=53481 (Add support
for OpenSSL SSLHonorCipherOrder) and I found a missing method SSL.hasOp
that is used in trunk and 7.0.

The javadoc for SSL.hasOp says:

    /**
     * Return true if SSL_OP_ if defined.
     * <p>
     * Currently used for testing weather the
     * SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION is supported by OpenSSL.
     * <p>
     * @param op SSL_OP to test.
     * @return true if SSL_OP is supported by OpenSSL library.
     */
    public static native boolean hasOp(int op);

Reading that, I would expect that the native method would check to see
if the OpenSSL implementation supported a particular setting. The
javadoc says that it's used to check
SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION, but not that it does not
support other checks.

Here's the implementation of the native method:

TCN_IMPLEMENT_CALL(jboolean, SSL, hasOp)(TCN_STDARGS, jint op)
{
#ifdef SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION
    if (op & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)
        return JNI_TRUE;
#endif
    return JNI_FALSE;
}

That very obviously only works specifically for the
SSL_OP_ALOW_UNSAFE_LEGACY_RENEGOTIATION flag.

I'm glad I found this because I'm using the same method to check for
SSL_OP_CIPHER_SERVER_PREFERENCE -- and that call is always going to
return FALSE, so this feature doesn't actually work :(

Looking at the OpenSSL API (wow, I really miss javadoc), it doesn't
appear that there's any function that can sniff the capabilities of the
engine and check to see whether a particular option is supported.
Instead, the technique of using #ifdefs to conditionally include code
that will return TRUE seems to be the only alternative.

My addition of this feature now requires an update to tcnative :(

Since I'm going to be adding this, shall I try to add any particular
subset of SSL options that can be checked? I'm actually wondering if
checking for SSL_OP_CIPHER_SERVER_PREFERENCE is worth it, since lack of
support from the OpenSSL library is highly unlikely.

At any rate, the list of supported options appears to be documented here:
http://www.openssl.org/docs/ssl/SSL_CTX_set_options.html#NOTES

There are only 26 documented options to check.

I'm also wondering if it wouldn't be a good idea to future-proof the
implementation of that method by having it throw an exception if you try
to check the support-status of an option that isn't known to the code.
Something like this:

TCN_IMPLEMENT_CALL(jboolean, SSL, hasOp)(TCN_STDARGS, jint op)
{
    int support = 0;

#ifdef SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION
    if (op & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION) {
      support |= (op & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION);
      op ^= SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION;
    }
#endif
#ifdef SSL_OP_FOO
    if (op & SSL_OP_FOO) {
      support |= (op & SSL_OP_FOO);
      op ^= SSL_OP_FOO; // Clear FOO
    }
#endif
#ifdef SSL_OP_BAR
    if (op & SSL_OP_BAR) {
      support |= (op & SSL_OP_BAR);
      op ^= SSL_OP_BAR; // Clear FOO
    }
#endif

    if(op) {
      char message[]
      tcn_Throw(e, 'Unsupported OpenSSL option to check: %#08lx', op);
    }

    return support == op ? JNI_TRUE : JNI_FALSE;
}

This is able to test option bitmasks that contain more than one option:
it will return true if all of them are supported and false if any one of
them is not supported. An exception will be thrown if you try to test
for an option that hasn't been coded into tcnative.

Thoughts?

-chris


Mime
View raw message