[7u-dev] RFA for JDK-8204344: Release session if initialization of SunPKCS11 Signature fails

[Martin Balao]

Thanks for review.

I need you to push it on my behalf, as I don't have permissions to do so.

On Tue, Jun 5, 2018 at 10:44 PM, Andrew Hughes <gnu.andrew at redhat.com>
wrote:

> On 5 June 2018 at 17:10, Martin Balao <mbalao at redhat.com> wrote:
> > Hi,
> >
> > Here there is a backport of JDK-8203182 [1] to JDK7:
> >
> >  *
> > http://cr.openjdk.java.net/~mbalao/webrevs/8203182/
> backports/7/8203182.webrev.01/
> >  *
> > http://cr.openjdk.java.net/~mbalao/webrevs/8203182/
> backports/7/8203182.webrev.01.zip
> >
> > Backport ticket: JDK-8204344 [2].
> > JDK commit: http://hg.openjdk.java.net/jdk/jdk/rev/00ebc17f3cc6
> > Review approval:
> > http://mail.openjdk.java.net/pipermail/security-dev/2018-May/017224.html
> >
> > I'd be grateful if someone can approve it.
> >
> > Kind regards,
> > Martin.-
> >
> > --
> > [1] - https://bugs.openjdk.java.net/browse/JDK-8203182
> > [2] - https://bugs.openjdk.java.net/browse/JDK-8204344
>
> Looks good to me. It's a pretty simple fix, made a little clearer if
> you do a diff which ignores whitespace:
>
> $ hg diff -b
> diff --git a/src/share/classes/sun/security/pkcs11/P11Signature.java
> b/src/share/classes/sun/security/pkcs11/P11Signature.java
> --- a/src/share/classes/sun/security/pkcs11/P11Signature.java
> +++ b/src/share/classes/sun/security/pkcs11/P11Signature.java
> @@ -257,6 +257,8 @@
>              session = token.killSession(session);
>              return;
>          }
> +
> +        try {
>          // "cancel" operation by finishing it
>          // XXX make sure all this always works correctly
>          if (mode == M_SIGN) {
> @@ -299,6 +301,9 @@
>                  // XXX check error code
>              }
>          }
> +        } finally {
> +            session = token.releaseSession(session);
> +        }
>      }
>
>      // assumes current state is initialized == false
> @@ -316,6 +321,8 @@
>              }
>              initialized = true;
>          } catch (PKCS11Exception e) {
> +            // release session when initialization failed
> +            session = token.releaseSession(session);
>              throw new ProviderException("Initialization failed", e);
>          }
>          if (bytesProcessed != 0) {
> @@ -443,6 +450,8 @@
>                  }
>                  bytesProcessed += len;
>              } catch (PKCS11Exception e) {
> +                initialized = false;
> +                session = token.releaseSession(session);
>                  throw new ProviderException(e);
>              }
>              break;
> @@ -490,6 +499,8 @@
>                  bytesProcessed += len;
>                  byteBuffer.position(ofs + len);
>              } catch (PKCS11Exception e) {
> +                initialized = false;
> +                session = token.releaseSession(session);
>                  throw new ProviderException("Update failed", e);
>              }
>              break;
>
> Are you able to push or shall I do it on your behalf?
> --
> Andrew :)
>
> Senior Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
>
> Web Site: http://fuseyism.com
> Twitter: https://twitter.com/gnu_andrew_java
> PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
> Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
>