-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8258833: Cancel multi-part cipher operations in SunPKCS11 after failures #1901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
aa15874
542b234
91363c1
80efdbb
5bf00de
3ccb24d
1724fa1
2a9b7ae
f705b7f
6abecdd
ee90166
0f96ddf
4c892a4
65dcc0d
bb52a06
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 @@ private void cancelOperation() { | |
| 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 @@ private int implUpdate(byte[] in, int inOfs, int inLen, | |
| 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); | ||
martinuy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| throw new ProviderException("update() failed", e); | ||
| } | ||
| } | ||
|
|
@@ -746,7 +757,11 @@ private int implUpdate(ByteBuffer inBuffer, ByteBuffer outBuffer) | |
| 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); | ||
martinuy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| throw new ProviderException("update() failed", e); | ||
| } | ||
| } | ||
|
|
@@ -770,9 +785,14 @@ private int implDoFinal(byte[] out, int outOfs, int outLen) | |
| 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; | ||
|
Comment on lines
+788
to
+793
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @valeriepeng I made a code change here so I'd like you to have a final look and validate. I'm just aligning the 'P11Cipher::implDoFinal(byte[]..' function to 'P11Cipher::implDoFinal(ByteBuffer..'. The rationale is that 'doFalse = false' can be placed before the C_EncryptFinal call because any error on it does not require a cancel (it already cancels the operation)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, aligning them is better. |
||
| 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 @@ private int implDoFinal(byte[] out, int outOfs, int outLen) | |
| 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; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment that for line 793 of P11Cipher
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, looks good. |
||
| 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; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment than for line 793 of P11Cipher |
||
| k = token.p11.C_DecryptFinal(session.id(), 0, out, outOfs, | ||
| outLen); | ||
| doCancel = false; | ||
| } | ||
| } | ||
| return k; | ||
| } catch (PKCS11Exception e) { | ||
| doCancel = false; | ||
martinuy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| handleException(e); | ||
| throw new ProviderException("doFinal() failed", e); | ||
| } finally { | ||
|
|
@@ -845,9 +869,14 @@ private int implDoFinal(ByteBuffer outBuffer) | |
| 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 @@ private int implDoFinal(ByteBuffer outBuffer) | |
| 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 @@ private int implDoFinal(ByteBuffer outBuffer) | |
| } | ||
| return k; | ||
| } catch (PKCS11Exception e) { | ||
| doCancel = false; | ||
martinuy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| handleException(e); | ||
| throw new ProviderException("doFinal() failed", e); | ||
| } finally { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.