> On Jul 3, 2019, at 11:28 AM, Xuelei Fan <xuelei....@oracle.com> wrote:
>
> To make it clearer, I would like to have a brief comment about the unkeyed
> checksum checking as you cited of section 6.1 of RFC 3961, probably in
> Checksum.verifyAnyChecksum() or/and CksumType.verifyChecksum().
>
> Checksum.verifyAnyChecksum():
> 191 if (!cksumEngine.isSafe())
> 192 throw new KrbApErrException(Krb5.KRB_AP_ERR_INAPP_CKSUM);
>
> 204 if (!cksumEngine.isSafe()) {
> 205 return cksumEngine.verifyChecksum(data, checksum);
>
> CksumType.java:
> 159 public boolean verifyChecksum(byte[] data, byte[] checksum)
> 160 throws KrbCryptoException {
> 161 throw new UnsupportedOperationException("Not supported");
> 162 }
OK, I'll add some.
>
> I was wondering, if a CksumType object is not safe (!isSafe()) and does not
> implement the verifyChecksum() method, an USPOE will be thrown. For example,
> I'm not sure if Crc32CksumType.verifyChecksum() could be called or not in
> practice. Is it better to have CksumType.verifyChecksum() returns 'false'?
Maybe not. In KrbKdcRep::check when an exception is thrown there will be
something written to debug output, but if it's false, nothing shows. In fact,
before this fix the RsaMd5CksumType::verifyKeyedChecksum was called and it
always returns false. I spent some time debugging and found out the reason. The
USPOE would be more loudly and friendly for supporting.
>
> Otherwise, looks good to me. Need no more code review from me if you have a
> good reason to keep it as-is, or take the comments above.
Thanks,
Max
>
> Xuelei
>
> On 7/2/2019 6:34 PM, Weijun Wang wrote:
>> More justification from https://tools.ietf.org/html/rfc3961#section-6.1:
>> 6.1. Unkeyed Checksums
>> These checksum types use no encryption keys and thus can be used in
>> combination with any encryption type, but they may only be used with
>> caution, in limited circumstances where the lack of a key does not
>> provide a window for an attack, preferably as part of an encrypted
>> message [6]. Keyed checksum algorithms are recommended.
>> Here the PA_REQ_ENC_PA_REP is a part of an encrypted message EncKDCRepPart.
>> Thanks,
>> Max
>>> On Jul 2, 2019, at 11:13 AM, Weijun Wang <weijun.w...@oracle.com> wrote:
>>>
>>> Please take a review at
>>>
>>> http://cr.openjdk.java.net/~weijun/8226719/webrev.00/
>>>
>>> This happens when authenticating to a Windows 2000 Server using DES
>>> encryption type. The PA_REQ_ENC_PA_REP in the reply is using
>>> RsaMd5CksumType but it is treated unsafe and rejected.
>>>
>>> Here, unsafe means un-keyed. While it's unsafe to use it as a standalone
>>> checksum but in this case the PA_REQ_ENC_PA_REP is embedded inside
>>> EncKDCRepPart which is already encrypted. Therefore an attacker will not be
>>> able to modify it without knowing the key. (Please note that when a keyed
>>> checksum is used, the key is exactly the same as the one used to encrypt
>>> the EncKDCRepPart field).
>>>
>>> This fix added a new method verifyAnyChecksum() that can verify both a
>>> keyed and an un-keyed checksum. The method is currently only used by the
>>> PA_REQ_ENC_PA_REP verification.
>>>
>>> Noreg-hard. Only reproducible when accessing a Windows 2000 Server, which
>>> is exactly how our internal SQE test caught it.
>>>
>>> Thanks,
>>> Max
>>>
>>>
>>>
