Re: RFR: 8248268: Support KWP in addition to KW [v4]

2021-04-03 Thread Michael StJohns

On 4/3/2021 11:35 AM, Greg Rubin wrote:

I'd advise against the AutoPadding scheme without more careful analysis and 
discussion. Have we seen either KW or KWP specifications which recommend that 
behavior?

My concern is that we've seen cases before where two different cryptographic 
algorithms could be selected transparently upon decryption and it lowers the 
security of the overall system. (A variant of in-band signalling.) The general 
consensus that I've been seeing in the (applied) cryptographic community is 
strongly away from in-band signalling and towards the decryptor fully 
specifying the algorithms and behavior prior to attempting decryption.


I think this is in response to my comment?

The wrap function can take a Key as an input and can have the unwrap 
method produce a Key as an output - indeed it should be used primarily 
for this rather than the more general encrypt/decrypt functions.  The 
problem is that the encoding of the key may not be known prior to the 
attempt to wrap it - hence it's not known whether or not padding need be 
applied.  This is especially problematic with HSMs.  Providing an 
AutoPadding mode would allow the wrapping algorithm to decide whether to 
use either of the RFC 3394 (AKA KW) Integrity Check Value (ICV) or the 
RFC5649 (aka KWP) value and padding length.


The key thing to remember here is that the IV (initial value - RFC 
language) /ICV (integrity check value - NIST language)actually isn't an 
IV(initialization vector) in the ordinary meaning, it's a flag, padding 
and integrity indicator and will be fixed for all keys of the same 
length that use the specified values.   E.g. unlike other modes that 
require an initialization vector, you don't need to know the ICV to 
decrypt the underlying key stream, but you can  (and for that matter 
MUST) easily test the recovered first block against the expected ICV to 
determine whether the output needs padding removed or not.


FWIW, the actual cryptographic operations between padded data and 
non-padded data (of the right multiple length) are identical. It's only 
the pre or post processing that's looking for different data.


Obviously, this doesn't work if someone provides their own IV - but 
that's fairly unlikely.  CF CCM and its non-normative example formatting 
function appendix A -  each and every implementation I've seen uses that 
formatting function, even though it isn't actually required by the 
standard.  I'd be surprised if anyone decided to use a different set of 
non-standard IV values.


If an AutoPadding mode were implemented, I'd throw exceptions if someone 
tried to set the IV.


Later, Mike




RFR: 8264681: Use the blessed modifier order in java.security

2021-04-03 Thread Alex Blewitt
8264681: Use the blessed modifier order in java.security

-

Commit messages:
 - 8264681: Use the blessed modifier order in java.security

Changes: https://git.openjdk.java.net/jdk/pull/3338/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3338=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264681
  Stats: 162 lines in 55 files changed: 0 ins; 0 del; 162 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3338.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3338/head:pull/3338

PR: https://git.openjdk.java.net/jdk/pull/3338


Re: RFR: 8248268: Support KWP in addition to KW [v4]

2021-04-03 Thread Greg Rubin
On Sat, 27 Mar 2021 00:25:09 GMT, Valerie Peng  wrote:

>> This change updates SunJCE provider as below:
>> - updated existing AESWrap support with AES/KW/NoPadding cipher 
>> transformation. 
>> - added support for AES/KWP/NoPadding and AES/KW/PKCS5Padding.
>> 
>> Existing AESWrap impl, i.e. AESWrapCipher class, is re-factored and renamed 
>> to KeyWrapCipher class. The W and W_inverse functions are moved to KWUtil 
>> class. The KW and KWP support are in the new AESKeyWrap and AESKeyWrapPadded 
>> classes which extend FeedbackCipher and used in KeyWrapCipher class. To 
>> minimize data copying, AESKeyWrap and AESKeyWrapPadded will do the crypto 
>> operation over the same input buffer which is allocated and managed by 
>> KeyWrapCipher class. 
>> 
>> Also note that existing AESWrap impl does not take IV. However, the 
>> corresponding PKCS#11 mechanisms do, so I added support for accepting IVs to 
>> both KW and KWP.
>> 
>> Thanks,
>> Valerie
>
> Valerie Peng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refactor code to reduce code duplication
>   Address review comments
>   Add more test vectors

I'd advise against the AutoPadding scheme without more careful analysis and 
discussion. Have we seen either KW or KWP specifications which recommend that 
behavior?

My concern is that we've seen cases before where two different cryptographic 
algorithms could be selected transparently upon decryption and it lowers the 
security of the overall system. (A variant of in-band signalling.) The general 
consensus that I've been seeing in the (applied) cryptographic community is 
strongly away from in-band signalling and towards the decryptor fully 
specifying the algorithms and behavior prior to attempting decryption.

src/java.base/share/classes/com/sun/crypto/provider/AESKeyWrapPadded.java line 
71:

> 69: match &= (ivAndLen[i] == iv[i]);
> 70: }
> 71: if (!match) {

True nitpick (thus ignorable): I believe that using bitwise math is slightly 
more resistant to compiler and/or CPU optimization to defend against 
timing-attacks. (Since I haven't even seen an attack against KW or KWP, this is 
simply a note in general rather than something which needs to be fixed.)

src/java.base/share/classes/com/sun/crypto/provider/AESKeyWrapPadded.java line 
78:

> 76: for (int k = 5; k < SEMI_BLKSIZE; k++) {
> 77: if (outLen != 0) {
> 78: outLen <<= SEMI_BLKSIZE;

While technically, this is correct (as it is shifting by 8 bits), it is pure 
coincidence that `SEMI_BLKSIZE` (8 bytes) happens to have the name integer 
value as the number of bits in one byte. It took me more reads than I care to 
admit to understand why this worked. Could we just replace this one with an `8` 
as it is clearer and more accurate?

-

PR: https://git.openjdk.java.net/jdk/pull/2404


RE: [11u] RFR: 8254631: Better support ALPN byte wire values in SunJSSE

2021-04-03 Thread Doerr, Martin
Hi Christoph,

thanks for the review and the approval!

Best regards,
Martin


From: Langer, Christoph 
Sent: Donnerstag, 1. April 2021 23:50
To: Doerr, Martin ; jdk-updates-dev 
; security-dev 
Subject: RE: [11u] RFR: 8254631: Better support ALPN byte wire values in SunJSSE

Hi Martin,

looks good to me.

Best regards
Christoph

From: Doerr, Martin mailto:martin.do...@sap.com>>
Sent: Dienstag, 30. März 2021 14:01
To: jdk-updates-dev 
mailto:jdk-updates-...@openjdk.java.net>>; 
security-dev 
mailto:security-dev@openjdk.java.net>>
Cc: Langer, Christoph 
mailto:christoph.lan...@sap.com>>
Subject: [11u] RFR: 8254631: Better support ALPN byte wire values in SunJSSE

Hi,

JDK-8254631 is backported to 11.0.12-oracle. I'd like to backport it for parity.
It applies cleanly, but the javadoc parts don't compile with 11u. They are not 
compatible with 11u and are documented to be dropped in the CSR (linked below).
As also documented in the CSR, the old behavior can get restored by specifying 
the property: jdk.tls.alpnCharset=UTF-8

Bug:
https://bugs.openjdk.java.net/browse/JDK-8254631

Original change:
https://git.openjdk.java.net/jdk/commit/fe51

11u CSR:
https://bugs.openjdk.java.net/browse/JDK-8264177

11u backport:
http://cr.openjdk.java.net/~mdoerr/8254631_ALPN_11u/webrev.00/

Please review.

Best regards,
Martin