Mercurial > hg > icedtea8-forest > jdk
changeset 14937:27b6d9a2b8a3
8258833: Cancel multi-part cipher operations in SunPKCS11 after failures
Reviewed-by: valeriep, phh, andrew
| author | mbalao |
|---|---|
| date | Mon, 25 Jan 2021 15:01:59 -0300 |
| parents | fa5b43603cf2 |
| children | 39e8ea32b425 |
| files | src/share/classes/sun/security/pkcs11/P11AEADCipher.java src/share/classes/sun/security/pkcs11/P11Cipher.java src/share/classes/sun/security/pkcs11/P11Mac.java src/share/classes/sun/security/pkcs11/P11PSSSignature.java src/share/classes/sun/security/pkcs11/P11Signature.java test/sun/security/pkcs11/Cipher/CancelMultipart.java |
| diffstat | 6 files changed, 292 insertions(+), 15 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/classes/sun/security/pkcs11/P11AEADCipher.java Thu Apr 30 14:04:39 2015 +0400 +++ b/src/share/classes/sun/security/pkcs11/P11AEADCipher.java Mon Jan 25 15:01:59 2021 -0300 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -347,6 +347,13 @@ 0, buffer, 0, bufLen); } } catch (PKCS11Exception e) { + if (e.getErrorCode() == CKR_OPERATION_NOT_INITIALIZED) { + // Cancel Operation may be invoked after an error on a PKCS#11 + // call. If the operation inside the token was already cancelled, + // do not fail here. This is part of a defensive mechanism for + // PKCS#11 libraries that do not strictly follow the standard. + return; + } if (encrypt) { throw new ProviderException("Cancel failed", e); } @@ -614,6 +621,12 @@ } return k; } catch (PKCS11Exception e) { + // As per the PKCS#11 standard, C_Encrypt and C_Decrypt may only + // keep the operation active on CKR_BUFFER_TOO_SMALL errors or + // successful calls to determine the output length. However, + // these cases are not expected here because the output length + // is checked in the OpenJDK side before making the PKCS#11 call. + // Thus, doCancel can safely be 'false'. doCancel = false; handleException(e); throw new ProviderException("doFinal() failed", e); @@ -700,6 +713,12 @@ outBuffer.position(outBuffer.position() + k); return k; } catch (PKCS11Exception e) { + // As per the PKCS#11 standard, C_Encrypt and C_Decrypt may only + // keep the operation active on CKR_BUFFER_TOO_SMALL errors or + // successful calls to determine the output length. However, + // these cases are not expected here because the output length + // is checked in the OpenJDK side before making the PKCS#11 call. + // Thus, doCancel can safely be 'false'. doCancel = false; handleException(e); throw new ProviderException("doFinal() failed", e);
--- a/src/share/classes/sun/security/pkcs11/P11Cipher.java Thu Apr 30 14:04:39 2015 +0400 +++ b/src/share/classes/sun/security/pkcs11/P11Cipher.java Mon Jan 25 15:01:59 2021 -0300 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -439,6 +439,13 @@ token.p11.C_DecryptFinal(session.id(), 0, buffer, 0, bufLen); } } catch (PKCS11Exception e) { + if (e.getErrorCode() == CKR_OPERATION_NOT_INITIALIZED) { + // Cancel Operation may be invoked after an error on a PKCS#11 + // call. If the operation inside the token was already cancelled, + // do not fail here. This is part of a defensive mechanism for + // PKCS#11 libraries that do not strictly follow the standard. + return; + } if (encrypt) { throw new ProviderException("Cancel failed", e); } @@ -628,7 +635,11 @@ throw (ShortBufferException) (new ShortBufferException().initCause(e)); } - reset(false); + // Some implementations such as the NSS Software Token do not + // cancel the operation upon a C_EncryptUpdate/C_DecryptUpdate + // failure (as required by the PKCS#11 standard). See JDK-8258833 + // for further information. + reset(true); throw new ProviderException("update() failed", e); } } @@ -746,7 +757,11 @@ throw (ShortBufferException) (new ShortBufferException().initCause(e)); } - reset(false); + // Some implementations such as the NSS Software Token do not + // cancel the operation upon a C_EncryptUpdate/C_DecryptUpdate + // failure (as required by the PKCS#11 standard). See JDK-8258833 + // for further information. + reset(true); throw new ProviderException("update() failed", e); } } @@ -770,9 +785,14 @@ 0, padBuffer, 0, actualPadLen, 0, out, outOfs, outLen); } + // Some implementations such as the NSS Software Token do not + // cancel the operation upon a C_EncryptUpdate failure (as + // required by the PKCS#11 standard). Cancel is not needed + // only after this point. See JDK-8258833 for further + // information. + doCancel = false; k += token.p11.C_EncryptFinal(session.id(), 0, out, (outOfs + k), (outLen - k)); - doCancel = false; } else { // Special handling to match SunJCE provider behavior if (bytesBuffered == 0 && padBufferLen == 0) { @@ -784,22 +804,26 @@ padBuffer, 0, padBufferLen, 0, padBuffer, 0, padBuffer.length); } + // Some implementations such as the NSS Software Token do not + // cancel the operation upon a C_DecryptUpdate failure (as + // required by the PKCS#11 standard). Cancel is not needed + // only after this point. See JDK-8258833 for further + // information. + doCancel = false; k += token.p11.C_DecryptFinal(session.id(), 0, padBuffer, k, padBuffer.length - k); - doCancel = false; int actualPadLen = paddingObj.unpad(padBuffer, k); k -= actualPadLen; System.arraycopy(padBuffer, 0, out, outOfs, k); } else { + doCancel = false; k = token.p11.C_DecryptFinal(session.id(), 0, out, outOfs, outLen); - doCancel = false; } } return k; } catch (PKCS11Exception e) { - doCancel = false; handleException(e); throw new ProviderException("doFinal() failed", e); } finally { @@ -845,9 +869,14 @@ 0, padBuffer, 0, actualPadLen, outAddr, outArray, outOfs, outLen); } + // Some implementations such as the NSS Software Token do not + // cancel the operation upon a C_EncryptUpdate failure (as + // required by the PKCS#11 standard). Cancel is not needed + // only after this point. See JDK-8258833 for further + // information. + doCancel = false; k += token.p11.C_EncryptFinal(session.id(), outAddr, outArray, (outOfs + k), (outLen - k)); - doCancel = false; } else { // Special handling to match SunJCE provider behavior if (bytesBuffered == 0 && padBufferLen == 0) { @@ -861,18 +890,23 @@ 0, padBuffer, 0, padBuffer.length); padBufferLen = 0; } + // Some implementations such as the NSS Software Token do not + // cancel the operation upon a C_DecryptUpdate failure (as + // required by the PKCS#11 standard). Cancel is not needed + // only after this point. See JDK-8258833 for further + // information. + doCancel = false; k += token.p11.C_DecryptFinal(session.id(), 0, padBuffer, k, padBuffer.length - k); - doCancel = false; int actualPadLen = paddingObj.unpad(padBuffer, k); k -= actualPadLen; outArray = padBuffer; outOfs = 0; } else { + doCancel = false; k = token.p11.C_DecryptFinal(session.id(), outAddr, outArray, outOfs, outLen); - doCancel = false; } } if ((!encrypt && paddingObj != null) || @@ -884,7 +918,6 @@ } return k; } catch (PKCS11Exception e) { - doCancel = false; handleException(e); throw new ProviderException("doFinal() failed", e); } finally {
--- a/src/share/classes/sun/security/pkcs11/P11Mac.java Thu Apr 30 14:04:39 2015 +0400 +++ b/src/share/classes/sun/security/pkcs11/P11Mac.java Mon Jan 25 15:01:59 2021 -0300 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -144,6 +144,13 @@ try { token.p11.C_SignFinal(session.id(), 0); } catch (PKCS11Exception e) { + if (e.getErrorCode() == CKR_OPERATION_NOT_INITIALIZED) { + // Cancel Operation may be invoked after an error on a PKCS#11 + // call. If the operation inside the token was already cancelled, + // do not fail here. This is part of a defensive mechanism for + // PKCS#11 libraries that do not strictly follow the standard. + return; + } throw new ProviderException("Cancel failed", e); } } @@ -206,6 +213,12 @@ ensureInitialized(); return token.p11.C_SignFinal(session.id(), 0); } catch (PKCS11Exception e) { + // As per the PKCS#11 standard, C_SignFinal may only + // keep the operation active on CKR_BUFFER_TOO_SMALL errors or + // successful calls to determine the output length. However, + // these cases are handled at OpenJDK's libj2pkcs11 native + // library. Thus, P11Mac::reset can be called with a 'false' + // doCancel argument from here. throw new ProviderException("doFinal() failed", e); } finally { reset(false);
--- a/src/share/classes/sun/security/pkcs11/P11PSSSignature.java Thu Apr 30 14:04:39 2015 +0400 +++ b/src/share/classes/sun/security/pkcs11/P11PSSSignature.java Mon Jan 25 15:01:59 2021 -0300 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -271,6 +271,13 @@ } } } catch (PKCS11Exception e) { + if (e.getErrorCode() == CKR_OPERATION_NOT_INITIALIZED) { + // Cancel Operation may be invoked after an error on a PKCS#11 + // call. If the operation inside the token was already cancelled, + // do not fail here. This is part of a defensive mechanism for + // PKCS#11 libraries that do not strictly follow the standard. + return; + } if (mode == M_SIGN) { throw new ProviderException("cancel failed", e); } @@ -594,6 +601,11 @@ doCancel = false; return signature; } catch (PKCS11Exception pe) { + // As per the PKCS#11 standard, C_Sign and C_SignFinal may only + // keep the operation active on CKR_BUFFER_TOO_SMALL errors or + // successful calls to determine the output length. However, + // these cases are handled at OpenJDK's libj2pkcs11 native + // library. Thus, doCancel can safely be 'false' here. doCancel = false; throw new ProviderException(pe); } catch (ProviderException e) {
--- a/src/share/classes/sun/security/pkcs11/P11Signature.java Thu Apr 30 14:04:39 2015 +0400 +++ b/src/share/classes/sun/security/pkcs11/P11Signature.java Mon Jan 25 15:01:59 2021 -0300 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -297,6 +297,13 @@ } } } catch (PKCS11Exception e) { + if (e.getErrorCode() == CKR_OPERATION_NOT_INITIALIZED) { + // Cancel Operation may be invoked after an error on a PKCS#11 + // call. If the operation inside the token was already cancelled, + // do not fail here. This is part of a defensive mechanism for + // PKCS#11 libraries that do not strictly follow the standard. + return; + } if (mode == M_VERIFY) { long errorCode = e.getErrorCode(); if ((errorCode == CKR_SIGNATURE_INVALID) || @@ -637,6 +644,11 @@ return signature; } } catch (PKCS11Exception e) { + // As per the PKCS#11 standard, C_Sign and C_SignFinal may only + // keep the operation active on CKR_BUFFER_TOO_SMALL errors or + // successful calls to determine the output length. However, + // these cases are handled at OpenJDK's libj2pkcs11 native + // library. Thus, doCancel can safely be 'false' here. doCancel = false; throw new ProviderException(e); } finally {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/sun/security/pkcs11/Cipher/CancelMultipart.java Mon Jan 25 15:01:59 2021 -0300 @@ -0,0 +1,188 @@ +/* + * Copyright (c) 2021, Red Hat, Inc. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8258833 + * @library /lib/security .. + * @run main/othervm CancelMultipart + */ + +import java.lang.reflect.Field; +import java.nio.ByteBuffer; +import java.security.Key; +import java.security.Provider; +import java.security.ProviderException; +import javax.crypto.Cipher; +import javax.crypto.IllegalBlockSizeException; +import javax.crypto.spec.SecretKeySpec; + +public class CancelMultipart extends PKCS11Test { + + private static Provider provider; + private static Key key; + + static { + key = new SecretKeySpec(new byte[16], "AES"); + } + + private static class SessionLeaker { + private LeakOperation op; + private LeakInputType type; + + SessionLeaker(LeakOperation op, LeakInputType type) { + this.op = op; + this.type = type; + } + + private void leakAndTry() throws Exception { + Cipher cipher = op.getCipher(); + try { + type.doOperation(cipher, + (op instanceof LeakDecrypt ? + LeakInputType.DECRYPT_MODE : + null)); + throw new Exception("PKCS11Exception expected, invalid block" + + "size"); + } catch (ProviderException | IllegalBlockSizeException e) { + // Exception expected - session returned to the SessionManager + // should be cancelled. That's what will be tested now. + } + + tryCipherInit(); + } + } + + private static interface LeakOperation { + Cipher getCipher() throws Exception; + } + + private static interface LeakInputType { + static int DECRYPT_MODE = 1; + void doOperation(Cipher cipher, int mode) throws Exception; + } + + private static class LeakDecrypt implements LeakOperation { + public Cipher getCipher() throws Exception { + Cipher cipher = Cipher.getInstance( + "AES/ECB/PKCS5Padding", provider); + cipher.init(Cipher.DECRYPT_MODE, key); + return cipher; + } + } + + private static class LeakByteBuffer implements LeakInputType { + public void doOperation(Cipher cipher, int mode) throws Exception { + if (mode == DECRYPT_MODE) { + cipher.update(ByteBuffer.allocate(1), ByteBuffer.allocate(1)); + cipher.doFinal(ByteBuffer.allocate(0), ByteBuffer.allocate(1)); + } + } + } + + private static class LeakByteArray implements LeakInputType { + public void doOperation(Cipher cipher, int mode) throws Exception { + if (mode == DECRYPT_MODE) { + cipher.update(new byte[1]); + cipher.doFinal(new byte[1], 0, 0); + } + } + } + + public static void main(String[] args) throws Exception { + main(new CancelMultipart(), args); + } + + @Override + public void main(Provider p) throws Exception { + init(p); + + // Try multiple paths: + + executeTest(new SessionLeaker(new LeakDecrypt(), new LeakByteArray()), + "P11Cipher::implDoFinal(byte[], int, int)"); + + executeTest(new SessionLeaker(new LeakDecrypt(), new LeakByteBuffer()), + "P11Cipher::implDoFinal(ByteBuffer)"); + + System.out.println("TEST PASS - OK"); + } + + private static void executeTest(SessionLeaker sl, String testName) + throws Exception { + try { + sl.leakAndTry(); + System.out.println(testName + ": OK"); + } catch (Exception e) { + System.out.println(testName + ": FAILED"); + throw e; + } + } + + private static void init(Provider p) throws Exception { + provider = p; + + // The max number of sessions is 2 because, in addition to the + // operation (i.e. PKCS11::getNativeKeyInfo), a session to hold + // the P11Key object is needed. + setMaxSessions(2); + } + + /* + * This method is intended to generate pression on the number of sessions + * to be used from the NSS Software Token, so sessions with (potentially) + * active operations are reused. + */ + private static void setMaxSessions(int maxSessions) throws Exception { + Field tokenField = Class.forName("sun.security.pkcs11.SunPKCS11") + .getDeclaredField("token"); + tokenField.setAccessible(true); + Field sessionManagerField = Class.forName("sun.security.pkcs11.Token") + .getDeclaredField("sessionManager"); + sessionManagerField.setAccessible(true); + Field maxSessionsField = Class.forName("sun.security.pkcs11.SessionManager") + .getDeclaredField("maxSessions"); + maxSessionsField.setAccessible(true); + Object sessionManagerObj = sessionManagerField.get( + tokenField.get(provider)); + maxSessionsField.setInt(sessionManagerObj, maxSessions); + } + + private static void tryCipherInit() throws Exception { + Cipher cipher = Cipher.getInstance("AES/ECB/NoPadding", provider); + + // A CKR_OPERATION_ACTIVE error may be thrown if a session was + // returned to the Session Manager with an active operation, and + // we try to initialize the Cipher using it. + // + // Given that the maximum number of sessions was forced to 2, we know + // that the session to be used here was already used in a previous + // (failed) operation. Thus, the test asserts that the operation was + // properly cancelled. + cipher.init(Cipher.ENCRYPT_MODE, key); + + // If initialization passes, finish gracefully so other paths can + // be tested under the current maximum number of sessions. + cipher.doFinal(new byte[16], 0, 0); + } +}