Thanks for the update, there are still some build problems due to compiler warnings/errors.

Perhaps https://wiki.openjdk.java.net/display/Build/Submit+Repo and https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms would help?

Regards,
Valerie

On 5/2/2018 4:50 PM, Martin Balao wrote:
Minor update (webrev 06):

 * Rebased to cece972575ac [1] (latest JDK revision today)
 * Compiler warnings fixed

 * http://cr.openjdk.java.net/~mbalao/webrevs/6913047/6913047.webrev.06/ <http://cr.openjdk.java.net/%7Embalao/webrevs/6913047/6913047.webrev.06/>  * http://cr.openjdk.java.net/~mbalao/webrevs/6913047/6913047.webrev.06.zip <http://cr.openjdk.java.net/%7Embalao/webrevs/6913047/6913047.webrev.06.zip>

Kind regards,
Martin.-

--
[1] - http://hg.openjdk.java.net/jdk/jdk/rev/cece972575ac <http://hg.openjdk.java.net/jdk/jdk/rev/cece972575ac>

On Thu, Oct 12, 2017 at 10:00 AM, Martin Balao <mba...@redhat.com <mailto:mba...@redhat.com>> wrote:

    Webrev 04 uploaded to cr.openjdk.java.net
    <http://cr.openjdk.java.net>:

     *
    http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6913047/webrev.04/
    
<http://cr.openjdk.java.net/%7Esgehwolf/webrevs/mbalaoal/JDK-6913047/webrev.04/>
 (browse
    online)
     *
    
http://cr.openjdk.java.net/~sgehwolf/webrevs/mbalaoal/JDK-6913047/webrev.04/6913047.webrev.04.zip
    
<http://cr.openjdk.java.net/%7Esgehwolf/webrevs/mbalaoal/JDK-6913047/webrev.04/6913047.webrev.04.zip>
 (download)

    On Wed, Oct 11, 2017 at 10:31 AM, Martin Balao <mba...@redhat.com
    <mailto:mba...@redhat.com>> wrote:

        Hi,

        I'd like to propose a fix for bug JDK-6913047: "Long term
        memory leak when using PKCS11 and JCE exceeds 32 bit process
        address space" [1]. This fix does not contain changes in the
        GC and is SunPKCS11 internal only.

        PROBLEM
        ........................................................

        When using the SunPKCS11 crypto provider (for cipher,
        signature, mac, key generation or any other operation),
        multiple key objects may be created. I.e.: every time a TLS
        session is established, a unique master key (derived from the
        pre-master-key) has to be created and then used for encryption
        and decryption operations. This is a legitimate use case in
        which key caching does not make sense as each key is unique
        per session. These keys are of P11Key type and have a
        corresponding native key object created. In the case of NSS
        SunPKCS11 backend (PKCS11 software token), this native key
        object is temporarily stored in the process native heap. The
        interface is simple: a JNI call is done to create a native key
        object (C_CreateObject, C_CopyObject, C_DeriveKey,
        C_GenerateKeys, etc., according to the PKCS11 interface) and
        an integer handler is kept in the Java side (P11Key). When the
        P11Key object is destroyed, a finalizer code is executed to
        free the native key object (through C_DestroyObject JNI call).
        The problem is that finalizer code execution happens only if
        the JVM garbage-collector cleans up the P11Key object. That
        may be delayed or not done at all, depending on different GC
        algorithms, parameters and environment conditions. As a
        result, the native heap may be exhausted with not freed native
        key objects, and the JVM will then crash -this is particularly
        true for 32 bits VMs where the virtual address space can be
        exhausted-.


        SCOPE
        ........................................................

        The fix is proposed for SunPKCS11 with NSS backend only. Other
        PKCS11 backends are not currently under scope. It's likely
        that hardware PKCS11 backends store native key objects in
        their own memory, preventing a native heap exhaustion and a
        JVM crash. However, it might be possible to cause an
        exhaustion on their own memory blocking key objects creation
        at some point. In any case, this is speculative as no tests
        were done on our side with real hardware.


        SOLUTION
        ........................................................

        Assuming that native keys are extractable, the idea is to hold
        native key data in the Java heap while keys are not in use.
        When a P11Key is created, every CK_ATTRIBUTE (PKCS11) value
        for the native key is queried, data stored in an opaque Java
        byte[] (inside the P11Key object) and native key destroyed.
        Every time the P11Key is about to be used, the native key is
        created with the stored data. After usage, the native key is
        again destroyed. Thus, it's not necessary to wait for a
        finalizer execution to cleanup native resources: cleanup is
        done at deterministic and previously-known points. This comes
        with a resposibility for key users -which are all SunPKCS11
        internal services like P11Signature, P11Cipher,
        P11KeyGenerator, etc.-: create and destroy native keys through
        a reference counting scheme exposed by P11Key class. There are
        two kind of usages:

         1) stateless: the native key is "atomically" created, used
        and destroyed. I.e.: MAC calculation, getEncodedInternal
        operation (on P11Key objects), signature operations, TLS key
        derivation, etc.

         2) statefull: the native key is created, one or multiple
        intermediate actions are performed by the key user, a final
        action is performed and finally the native key is destroyed.
        I.e.: cipher operations.

        For keys that are extractable but sensitive (CKA_SENSITIVE
        attribute is true), as the case when operating in FIPS mode,
        wrapping/unwrapping is used as a workaround to extract session
        keys. Wrapper key is global and lives forever.

        There are no interface changes for SunPKCS11 external users.

        If keys are not extractable or the feature cannot be enabled
        for any other reason, the previous finalizer scheme is used as
        a fallback.


        ADDITIONAL IMPLEMENTATION NOTES
        ........................................................

        When a P11Key is created, a constructor parameter exists to
        indicate if the feature is enabled for that specific key. For
        this feature to be enabled, 2 additional conditions apply: 1)
        SunPKCS11 backend has to be NSS, and 2) key has to be
        extractable. If the feature is not enabled for a key, behavior
        is as previous to this patch (native key destruction relies on
        finalizer execution).

        The only P11Key user that explicitly does not use this feature
        is P11KeyStore. This is because these keys (token keys) are
        managed by alias names and makes no sense to remove them from
        the key store (they need to be accessible by an alias at any
        time).

        Because P11Key objects can be used by multiple threads at a
        time, there is a thread-safe reference counting scheme in
        order to decide when a native key object has to be created or
        destroyed. The SunPKCS11 internal API to use a P11Key is as
        follows: 1) increment the reference counter (which will
        eventually create the native key object if it doesn't exist),
        2) use the key and 3) decrement the reference counter (which
        will eventually destroy the native key if there it's not being
        used by anyone else).

        The reason why an opaque byte[] is used in P11Key objects to
        store native keys data (instead of a CK_ATTRIBUTE[] Java
        objects, queried by Java's C_GetAttributeValue function) is
        performance. My prototypes show a difference of 4x in speed. 2
        functions were added to libj2pkcs11 library: getNativeKeyInfo
        (to extract the opaque byte[] from a native key object) and
        createNativeKey (to create a native key object from an opaque
        byte[]).


        CHANGESET
        ........................................................

        This changeset is JDK-10 (at jdk c8796a577885 rev) based:

         *
        
http://people.redhat.com/mbalaoal/webrevs/jdk_6913047_sunpkcs11_nss_memory_leak/2017_10_06/6913047.webrev.04/
        
<http://people.redhat.com/mbalaoal/webrevs/jdk_6913047_sunpkcs11_nss_memory_leak/2017_10_06/6913047.webrev.04/>
        (browse online)
         *
        
http://people.redhat.com/mbalaoal/webrevs/jdk_6913047_sunpkcs11_nss_memory_leak/2017_10_06/6913047.webrev.04.zip
        
<http://people.redhat.com/mbalaoal/webrevs/jdk_6913047_sunpkcs11_nss_memory_leak/2017_10_06/6913047.webrev.04.zip>
        (download)


        TESTING
        ........................................................

        Test suite for 32 bits JVMs only:
        
http://people.redhat.com/mbalaoal/webrevs/jdk_6913047_sunpkcs11_nss_memory_leak/2017_10_06/Bug6913047.java
        
<http://people.redhat.com/mbalaoal/webrevs/jdk_6913047_sunpkcs11_nss_memory_leak/2017_10_06/Bug6913047.java>

         * Suite (Bug6913047.java)
          * Tests JVM memory exhaustion while using keys for different
        services: P11Cipher, P11Signature, P11KeyAgreement, P11Mac,
        P11Digest, P11KeyGenerator, P11KeyFactory, etc.
          * Tests functional regression.
           * Including Key Stores (P11KeyStore)

        Parameters to run the reproducer (on JDK-10):
         * javac: --add-modules jdk.crypto.cryptoki --add-exports
        java.base/sun.security.internal.spec=ALL-UNNAMED
         * java: -XX:+UseParallelGC -Xmx3500m --add-modules
        jdk.crypto.cryptoki --add-opens
        java.base/javax.crypto=ALL-UNNAMED --add-opens
        jdk.crypto.cryptoki/sun.security.pkcs11=ALL-UNNAMED

        You can also use jtreg.


        PERFORMANCE
        ........................................................

        For a quick reproducer previously developed (which looped
        100000 times creating P11Cipher and P11Key objects to encrypt
        a plaintext), these are the figures I got:

         * real 1m11.328s (without fix)
         * real 1m12.795s (with fix)

        Performance penalty seems to be low in current state.

        OTHER
        .......................................................

        My employer has an OCA agreement with Oracle and this work has
        been done in that context.

        Look forward to your comments.

        Kind regards,
        Martin.-

        --
        [1] - https://bugs.openjdk.java.net/browse/JDK-6913047
        <https://bugs.openjdk.java.net/browse/JDK-6913047>




Reply via email to