Skip to content
This repository was archived by the owner on Sep 2, 2022. It is now read-only.

Commit ce14719

Browse files
RealCLangerRealCLanger
committed
8261355: No data buffering in SunPKCS11 Cipher encryption when the underlying mechanism has no padding
Backport-of: 1ee80e03adfae5f428519f7c134e78a0f277a0a5
1 parent e4651f3 commit ce14719

File tree

2 files changed

+216
-82
lines changed

2 files changed

+216
-82
lines changed

‎src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Cipher.java‎

Lines changed: 112 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ final class P11Cipher extends CipherSpi {
7070
private static interface Padding {
7171
// ENC: format the specified buffer with padding bytes and return the
7272
// actual padding length
73-
int setPaddingBytes(byte[] paddingBuffer, int padLen);
73+
int setPaddingBytes(byte[] paddingBuffer, int startOff, int padLen);
7474

7575
// DEC: return the length of trailing padding bytes given the specified
7676
// padded data
@@ -91,8 +91,8 @@ private static class PKCS5Padding implements Padding {
9191
this.blockSize = blockSize;
9292
}
9393

94-
public int setPaddingBytes(byte[] paddingBuffer, int padLen) {
95-
Arrays.fill(paddingBuffer, 0, padLen, (byte) (padLen & 0x007f));
94+
public int setPaddingBytes(byte[] paddingBuffer, int startOff, int padLen) {
95+
Arrays.fill(paddingBuffer, startOff, startOff + padLen, (byte) (padLen & 0x007f));
9696
return padLen;
9797
}
9898

@@ -169,6 +169,14 @@ public int unpad(byte[] paddedData, int len)
169169
// specification mandates a fixed size of the key
170170
private int fixedKeySize = -1;
171171

172+
// Indicates whether the underlying PKCS#11 library requires block-sized
173+
// updates during multi-part operations. In such case, we buffer data in
174+
// padBuffer up to a block-size. This may be needed only if padding is
175+
// applied on the Java side. An example of the previous is when the
176+
// CKM_AES_ECB mechanism is used and the PKCS#11 library is NSS. See more
177+
// on JDK-8261355.
178+
private boolean reqBlockUpdates = false;
179+
172180
P11Cipher(Token token, String algorithm, long mechanism)
173181
throws PKCS11Exception, NoSuchAlgorithmException {
174182
super();
@@ -252,6 +260,10 @@ protected void engineSetPadding(String padding)
252260
// no native padding support; use our own padding impl
253261
paddingObj = new PKCS5Padding(blockSize);
254262
padBuffer = new byte[blockSize];
263+
char[] tokenLabel = token.tokenInfo.label;
264+
// NSS requires block-sized updates in multi-part operations.
265+
reqBlockUpdates = ((tokenLabel[0] == 'N' && tokenLabel[1] == 'S'
266+
&& tokenLabel[2] == 'S') ? true : false);
255267
}
256268
} else {
257269
throw new NoSuchPaddingException("Unsupported padding " + padding);
@@ -580,46 +592,54 @@ private int implUpdate(byte[] in, int inOfs, int inLen,
580592
try {
581593
ensureInitialized();
582594
int k = 0;
583-
if (encrypt) {
584-
k = token.p11.C_EncryptUpdate(session.id(), 0, in, inOfs, inLen,
585-
0, out, outOfs, outLen);
586-
} else {
587-
int newPadBufferLen = 0;
588-
if (paddingObj != null) {
589-
if (padBufferLen != 0) {
590-
// NSS throws up when called with data not in multiple
591-
// of blocks. Try to work around this by holding the
592-
// extra data in padBuffer.
593-
if (padBufferLen != padBuffer.length) {
594-
int bufCapacity = padBuffer.length - padBufferLen;
595-
if (inLen > bufCapacity) {
596-
bufferInputBytes(in, inOfs, bufCapacity);
597-
inOfs += bufCapacity;
598-
inLen -= bufCapacity;
599-
} else {
600-
bufferInputBytes(in, inOfs, inLen);
601-
return 0;
602-
}
595+
int newPadBufferLen = 0;
596+
if (paddingObj != null && (!encrypt || reqBlockUpdates)) {
597+
if (padBufferLen != 0) {
598+
if (padBufferLen != padBuffer.length) {
599+
int bufCapacity = padBuffer.length - padBufferLen;
600+
if (inLen > bufCapacity) {
601+
bufferInputBytes(in, inOfs, bufCapacity);
602+
inOfs += bufCapacity;
603+
inLen -= bufCapacity;
604+
} else {
605+
bufferInputBytes(in, inOfs, inLen);
606+
return 0;
603607
}
608+
}
609+
if (encrypt) {
610+
k = token.p11.C_EncryptUpdate(session.id(),
611+
0, padBuffer, 0, padBufferLen,
612+
0, out, outOfs, outLen);
613+
} else {
604614
k = token.p11.C_DecryptUpdate(session.id(),
605615
0, padBuffer, 0, padBufferLen,
606616
0, out, outOfs, outLen);
607-
padBufferLen = 0;
608617
}
609-
newPadBufferLen = inLen & (blockSize - 1);
610-
if (newPadBufferLen == 0) {
611-
newPadBufferLen = padBuffer.length;
612-
}
613-
inLen -= newPadBufferLen;
618+
padBufferLen = 0;
619+
}
620+
newPadBufferLen = inLen & (blockSize - 1);
621+
if (!encrypt && newPadBufferLen == 0) {
622+
// While decrypting with implUpdate, the last encrypted block
623+
// is always held in a buffer. If it's the final one (unknown
624+
// at this point), it may contain padding bytes and need further
625+
// processing. In implDoFinal (where we know it's the final one)
626+
// the buffer is decrypted, unpadded and returned.
627+
newPadBufferLen = padBuffer.length;
614628
}
615-
if (inLen > 0) {
629+
inLen -= newPadBufferLen;
630+
}
631+
if (inLen > 0) {
632+
if (encrypt) {
633+
k += token.p11.C_EncryptUpdate(session.id(), 0, in, inOfs,
634+
inLen, 0, out, (outOfs + k), (outLen - k));
635+
} else {
616636
k += token.p11.C_DecryptUpdate(session.id(), 0, in, inOfs,
617637
inLen, 0, out, (outOfs + k), (outLen - k));
618638
}
619-
// update 'padBuffer' if using our own padding impl.
620-
if (paddingObj != null) {
621-
bufferInputBytes(in, inOfs + inLen, newPadBufferLen);
622-
}
639+
}
640+
// update 'padBuffer' if using our own padding impl.
641+
if (paddingObj != null && newPadBufferLen > 0) {
642+
bufferInputBytes(in, inOfs + inLen, newPadBufferLen);
623643
}
624644
bytesBuffered += (inLen - k);
625645
return k;
@@ -676,60 +696,62 @@ private int implUpdate(ByteBuffer inBuffer, ByteBuffer outBuffer)
676696
}
677697

678698
int k = 0;
679-
if (encrypt) {
680-
if (inAddr == 0 && inArray == null) {
681-
inArray = new byte[inLen];
682-
inBuffer.get(inArray);
683-
} else {
684-
inBuffer.position(origPos + inLen);
685-
}
686-
k = token.p11.C_EncryptUpdate(session.id(),
687-
inAddr, inArray, inOfs, inLen,
688-
outAddr, outArray, outOfs, outLen);
689-
} else {
690-
int newPadBufferLen = 0;
691-
if (paddingObj != null) {
692-
if (padBufferLen != 0) {
693-
// NSS throws up when called with data not in multiple
694-
// of blocks. Try to work around this by holding the
695-
// extra data in padBuffer.
696-
if (padBufferLen != padBuffer.length) {
697-
int bufCapacity = padBuffer.length - padBufferLen;
698-
if (inLen > bufCapacity) {
699-
bufferInputBytes(inBuffer, bufCapacity);
700-
inOfs += bufCapacity;
701-
inLen -= bufCapacity;
702-
} else {
703-
bufferInputBytes(inBuffer, inLen);
704-
return 0;
705-
}
699+
int newPadBufferLen = 0;
700+
if (paddingObj != null && (!encrypt || reqBlockUpdates)) {
701+
if (padBufferLen != 0) {
702+
if (padBufferLen != padBuffer.length) {
703+
int bufCapacity = padBuffer.length - padBufferLen;
704+
if (inLen > bufCapacity) {
705+
bufferInputBytes(inBuffer, bufCapacity);
706+
inOfs += bufCapacity;
707+
inLen -= bufCapacity;
708+
} else {
709+
bufferInputBytes(inBuffer, inLen);
710+
return 0;
706711
}
712+
}
713+
if (encrypt) {
714+
k = token.p11.C_EncryptUpdate(session.id(), 0,
715+
padBuffer, 0, padBufferLen, outAddr, outArray,
716+
outOfs, outLen);
717+
} else {
707718
k = token.p11.C_DecryptUpdate(session.id(), 0,
708719
padBuffer, 0, padBufferLen, outAddr, outArray,
709720
outOfs, outLen);
710-
padBufferLen = 0;
711-
}
712-
newPadBufferLen = inLen & (blockSize - 1);
713-
if (newPadBufferLen == 0) {
714-
newPadBufferLen = padBuffer.length;
715721
}
716-
inLen -= newPadBufferLen;
722+
padBufferLen = 0;
717723
}
718-
if (inLen > 0) {
719-
if (inAddr == 0 && inArray == null) {
720-
inArray = new byte[inLen];
721-
inBuffer.get(inArray);
722-
} else {
723-
inBuffer.position(inBuffer.position() + inLen);
724-
}
724+
newPadBufferLen = inLen & (blockSize - 1);
725+
if (!encrypt && newPadBufferLen == 0) {
726+
// While decrypting with implUpdate, the last encrypted block
727+
// is always held in a buffer. If it's the final one (unknown
728+
// at this point), it may contain padding bytes and need further
729+
// processing. In implDoFinal (where we know it's the final one)
730+
// the buffer is decrypted, unpadded and returned.
731+
newPadBufferLen = padBuffer.length;
732+
}
733+
inLen -= newPadBufferLen;
734+
}
735+
if (inLen > 0) {
736+
if (inAddr == 0 && inArray == null) {
737+
inArray = new byte[inLen];
738+
inBuffer.get(inArray);
739+
} else {
740+
inBuffer.position(inBuffer.position() + inLen);
741+
}
742+
if (encrypt) {
743+
k += token.p11.C_EncryptUpdate(session.id(), inAddr,
744+
inArray, inOfs, inLen, outAddr, outArray,
745+
(outOfs + k), (outLen - k));
746+
} else {
725747
k += token.p11.C_DecryptUpdate(session.id(), inAddr,
726748
inArray, inOfs, inLen, outAddr, outArray,
727749
(outOfs + k), (outLen - k));
728750
}
729-
// update 'padBuffer' if using our own padding impl.
730-
if (paddingObj != null && newPadBufferLen != 0) {
731-
bufferInputBytes(inBuffer, newPadBufferLen);
732-
}
751+
}
752+
// update 'padBuffer' if using our own padding impl.
753+
if (paddingObj != null && newPadBufferLen > 0) {
754+
bufferInputBytes(inBuffer, newPadBufferLen);
733755
}
734756
bytesBuffered += (inLen - k);
735757
if (!(outBuffer instanceof DirectBuffer) &&
@@ -764,10 +786,14 @@ private int implDoFinal(byte[] out, int outOfs, int outLen)
764786
int k = 0;
765787
if (encrypt) {
766788
if (paddingObj != null) {
789+
int startOff = 0;
790+
if (reqBlockUpdates) {
791+
startOff = padBufferLen;
792+
}
767793
int actualPadLen = paddingObj.setPaddingBytes(padBuffer,
768-
requiredOutLen - bytesBuffered);
794+
startOff, requiredOutLen - bytesBuffered);
769795
k = token.p11.C_EncryptUpdate(session.id(),
770-
0, padBuffer, 0, actualPadLen,
796+
0, padBuffer, 0, startOff + actualPadLen,
771797
0, out, outOfs, outLen);
772798
}
773799
k += token.p11.C_EncryptFinal(session.id(),
@@ -839,10 +865,14 @@ private int implDoFinal(ByteBuffer outBuffer)
839865

840866
if (encrypt) {
841867
if (paddingObj != null) {
868+
int startOff = 0;
869+
if (reqBlockUpdates) {
870+
startOff = padBufferLen;
871+
}
842872
int actualPadLen = paddingObj.setPaddingBytes(padBuffer,
843-
requiredOutLen - bytesBuffered);
873+
startOff, requiredOutLen - bytesBuffered);
844874
k = token.p11.C_EncryptUpdate(session.id(),
845-
0, padBuffer, 0, actualPadLen,
875+
0, padBuffer, 0, startOff + actualPadLen,
846876
outAddr, outArray, outOfs, outLen);
847877
}
848878
k += token.p11.C_EncryptFinal(session.id(),

‎test/jdk/sun/security/pkcs11/Cipher/EncryptionPadding.java‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*
2+
* Copyright (c) 2021, Red Hat, Inc.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8261355
27+
* @library /test/lib ..
28+
* @run main/othervm EncryptionPadding
29+
*/
30+
31+
import java.nio.ByteBuffer;
32+
import java.security.Key;
33+
import java.security.Provider;
34+
import java.util.Arrays;
35+
import javax.crypto.Cipher;
36+
import javax.crypto.spec.SecretKeySpec;
37+
38+
public class EncryptionPadding extends PKCS11Test {
39+
40+
private static String transformation = "AES/ECB/PKCS5Padding";
41+
private static Key key = new SecretKeySpec(new byte[16], "AES");
42+
43+
public static void main(String[] args) throws Exception {
44+
main(new EncryptionPadding(), args);
45+
}
46+
47+
@Override
48+
public void main(Provider p) throws Exception {
49+
testWithInputSize(p, 1);
50+
testWithInputSize(p, 15);
51+
testWithInputSize(p, 16);
52+
testWithInputSize(p, 17);
53+
System.out.println("TEST PASS - OK");
54+
}
55+
56+
private static void testWithInputSize(Provider p, int inputSize)
57+
throws Exception {
58+
testWithInputSize(p, inputSize, false);
59+
testWithInputSize(p, inputSize, true);
60+
}
61+
62+
private static void testWithInputSize(Provider p, int inputSize,
63+
boolean isByteBuffer) throws Exception {
64+
byte[] plainText = new byte[inputSize];
65+
Arrays.fill(plainText, (byte)(inputSize & 0xFF));
66+
ByteBuffer cipherText =
67+
ByteBuffer.allocate(((inputSize / 16 ) + 1) * 16);
68+
byte[] tmp;
69+
70+
Cipher sunPKCS11cipher = Cipher.getInstance(transformation, p);
71+
sunPKCS11cipher.init(Cipher.ENCRYPT_MODE, key);
72+
for (int i = 0; i < ((inputSize - 1) / 16) + 1; i++) {
73+
int updateLength = Math.min(inputSize - (16 * i), 16);
74+
if (!isByteBuffer) {
75+
tmp = sunPKCS11cipher.update(plainText, i * 16,
76+
updateLength);
77+
if (tmp != null) {
78+
cipherText.put(tmp);
79+
}
80+
} else {
81+
ByteBuffer bb = ByteBuffer.allocate(updateLength);
82+
bb.put(plainText, i * 16, updateLength);
83+
bb.flip();
84+
sunPKCS11cipher.update(bb, cipherText);
85+
}
86+
}
87+
if (!isByteBuffer) {
88+
tmp = sunPKCS11cipher.doFinal();
89+
if (tmp != null) {
90+
cipherText.put(tmp);
91+
}
92+
} else {
93+
sunPKCS11cipher.doFinal(ByteBuffer.allocate(0), cipherText);
94+
}
95+
96+
Cipher sunJCECipher = Cipher.getInstance(transformation, "SunJCE");
97+
sunJCECipher.init(Cipher.DECRYPT_MODE, key);
98+
byte[] sunJCEPlain = sunJCECipher.doFinal(cipherText.array());
99+
100+
if (!Arrays.equals(plainText, sunJCEPlain)) {
101+
throw new Exception("Cross-provider cipher test failed.");
102+
}
103+
}
104+
}

0 commit comments

Comments
 (0)