Re: RFR (+CSR) 8201627: Kerberos sequence number issues

[Weijun Wang]

> On May 15, 2018, at 8:32 AM, Valerie Peng  wrote:
> 
> Hi Max,
> 
> I think it'd be clearer to mention default behavior first and then mention 
> the system property for overriding it if necessary. Something like following:
> 
> When mutual auth is not requested by the Kerberos 5 initiator, there is no 
> way to negotiate acceptor's initial sequence number. With this fix, the 
> SunJGSS provider will use initiator's initial sequence number as the initial 
> sequence number. To override this default behavior and to use 0 instead, 
> please set the system property 
> "sun.security.krb5.acceptor.sequence.number.nonmutual" to "zero" or "0". 
> Values other than "initiator", "zero", and "0" are illegal".

Great.

> 
> Maybe it'd also be nice to mention how the illegal values are handled, i.e. 
> ignored, exception thrown, etc.

An error will be thrown.

Thanks
Max

> 
> Valerie
> 
> 
> On 5/4/2018 10:53 PM, Weijun Wang wrote:
>> Hi Valerie
>> 
>> Can you also review the release note at 
>> https://bugs.openjdk.java.net/browse/JDK-8202681?
>> 
>> Thanks
>> Max
>> 
>> 
>>> On Apr 27, 2018, at 5:58 AM, Valerie Peng  wrote:
>>> 
>>> Sure, should be fine...
>>> Valerie
>>> 
>>> On 4/25/2018 9:48 PM, Weijun Wang wrote:
 I filed https://bugs.openjdk.java.net/browse/JDK-8202300 but might not 
 have time to make it into JDK 11.
 
 --Max
 
> On Apr 26, 2018, at 12:06 AM, Weijun Wang  wrote:
> 
> I'll keep using int32 (at least in this fix), both Java and MIT krb5 
> contain these words
> 
> * Workaround implementation incompatibilities by not generating
> * initial sequence numbers greater than 2^30
> 
> So bad thing could only happen after 2^30 messages.
> 
> --Max
> 
>> On Apr 25, 2018, at 10:38 PM, Weijun Wang  wrote:
>> 
>> It's complicated. Looks like MIT krb5 uses a uint32 for old etypes (DES, 
>> 3DES, RC4) and a uint64 for new ones (AES) [1][2].
>> 
>> I'll do more experiments.
>> 
>> Thanks
>> Max
>> 
>> [1] 
>> https://github.com/krb5/krb5/blob/master/src/lib/gssapi/generic/util_seqstate.c#L76
>> [2] 
>> https://github.com/krb5/krb5/blob/master/src/lib/gssapi/krb5/init_sec_context.c#L825
>> 
>>> On Apr 24, 2018, at 8:55 PM, Wang Weijun  wrote:
>>> 
>>> RFC 4120 5.5.1 has
 seq-number
 This optional field includes the initial sequence number to be used by 
 the KRB_PRIV or KRB_SAFE messages when sequence numbers are used to 
 detect replays. (It may also be used by application specific 
 messages.) When included in the authenticator, this field specifies 
 the initial sequence number for messages from the client to the 
 server. When included in the AP-REP message, the initial sequence 
 number is that for messages from the server to the client. When used 
 in KRB_PRIV or KRB_SAFE messages, it is incremented by one after each 
 message is sent. Sequence numbers fall in the range 0 through 2^32 - 1 
 and wrap to zero following the value 2^32 - 1.
>>> If it wraps, then it’s 4 bytes.
>>> 
>>> I will read more on it.
>>> 
>>> Thanks
>>> Max
>>> 
 在 2018年4月24日，18:08，Valerie Peng  写道：
 
 Hi Max,
 
 Most changes look good. I have only some comments and questions (see 
 below):
 
 - InitSecContextToken.java, line 62: bad -> unrecognized?
 - According to the class javadoc of various Token classes and Kerberos 
 spec, the sequence number is 8-byte integer from header byte 8-15, but 
 java int have only 4 bytes. The current code seems to assume the first 
 4 bytes of the sequence number are always 0. For the sake of 
 compliance and max inter-operability, maybe we should use long to 
 store the sequence number?
 
 CSR looks good to me.
 Thanks,
 Valerie
 
 
 
> On 4/18/2018 10:40 AM, Weijun Wang wrote:
> Please take a review of this fix:
> 
> webrev: http://cr.openjdk.java.net/~weijun/8201627/webrev.00/
> CSR: https://bugs.openjdk.java.net/browse/JDK-8201814
> 
> Basically we fix some bugs and introduce a system property so we can 
> interop with everyone.
> 
> Thanks
> Max
> 
> 

Re: RFR (+CSR) 8201627: Kerberos sequence number issues

[Valerie Peng]

Hi Max,

I think it'd be clearer to mention default behavior first and then 
mention the system property for overriding it if necessary. Something 
like following:


When mutual auth is not requested by the Kerberos 5 initiator, there is 
no way to negotiate acceptor's initial sequence number. With this fix, 
the SunJGSS provider will use initiator's initial sequence number as the 
initial sequence number. To override this default behavior and to use 0 
instead, please set the system property 
"sun.security.krb5.acceptor.sequence.number.nonmutual" to "zero" or "0". 
Values other than "initiator", "zero", and "0" are illegal".


Maybe it'd also be nice to mention how the illegal values are handled, 
i.e. ignored, exception thrown, etc.


Valerie


On 5/4/2018 10:53 PM, Weijun Wang wrote:

Hi Valerie

Can you also review the release note at 
https://bugs.openjdk.java.net/browse/JDK-8202681?

Thanks
Max



On Apr 27, 2018, at 5:58 AM, Valerie Peng  wrote:

Sure, should be fine...
Valerie

On 4/25/2018 9:48 PM, Weijun Wang wrote:

I filed https://bugs.openjdk.java.net/browse/JDK-8202300 but might not have 
time to make it into JDK 11.

--Max


On Apr 26, 2018, at 12:06 AM, Weijun Wang  wrote:

I'll keep using int32 (at least in this fix), both Java and MIT krb5 contain 
these words

* Workaround implementation incompatibilities by not generating
* initial sequence numbers greater than 2^30

So bad thing could only happen after 2^30 messages.

--Max


On Apr 25, 2018, at 10:38 PM, Weijun Wang  wrote:

It's complicated. Looks like MIT krb5 uses a uint32 for old etypes (DES, 3DES, 
RC4) and a uint64 for new ones (AES) [1][2].

I'll do more experiments.

Thanks
Max

[1] 
https://github.com/krb5/krb5/blob/master/src/lib/gssapi/generic/util_seqstate.c#L76
[2] 
https://github.com/krb5/krb5/blob/master/src/lib/gssapi/krb5/init_sec_context.c#L825


On Apr 24, 2018, at 8:55 PM, Wang Weijun  wrote:

RFC 4120 5.5.1 has

seq-number
This optional field includes the initial sequence number to be used by the 
KRB_PRIV or KRB_SAFE messages when sequence numbers are used to detect replays. 
(It may also be used by application specific messages.) When included in the 
authenticator, this field specifies the initial sequence number for messages 
from the client to the server. When included in the AP-REP message, the initial 
sequence number is that for messages from the server to the client. When used 
in KRB_PRIV or KRB_SAFE messages, it is incremented by one after each message 
is sent. Sequence numbers fall in the range 0 through 2^32 - 1 and wrap to zero 
following the value 2^32 - 1.

If it wraps, then it’s 4 bytes.

I will read more on it.

Thanks
Max


在 2018年4月24日，18:08，Valerie Peng  写道：

Hi Max,

Most changes look good. I have only some comments and questions (see below):

- InitSecContextToken.java, line 62: bad -> unrecognized?
- According to the class javadoc of various Token classes and Kerberos spec, 
the sequence number is 8-byte integer from header byte 8-15, but java int have 
only 4 bytes. The current code seems to assume the first 4 bytes of the 
sequence number are always 0. For the sake of compliance and max 
inter-operability, maybe we should use long to store the sequence number?

CSR looks good to me.
Thanks,
Valerie




On 4/18/2018 10:40 AM, Weijun Wang wrote:
Please take a review of this fix:

webrev: http://cr.openjdk.java.net/~weijun/8201627/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8201814

Basically we fix some bugs and introduce a system property so we can interop 
with everyone.

Thanks
Max

Re: RFR (+CSR) 8201627: Kerberos sequence number issues

[Weijun Wang]

Hi Valerie

Can you also review the release note at 
https://bugs.openjdk.java.net/browse/JDK-8202681?

Thanks
Max


> On Apr 27, 2018, at 5:58 AM, Valerie Peng  wrote:
> 
> Sure, should be fine...
> Valerie
> 
> On 4/25/2018 9:48 PM, Weijun Wang wrote:
>> I filed https://bugs.openjdk.java.net/browse/JDK-8202300 but might not have 
>> time to make it into JDK 11.
>> 
>> --Max
>> 
>>> On Apr 26, 2018, at 12:06 AM, Weijun Wang  wrote:
>>> 
>>> I'll keep using int32 (at least in this fix), both Java and MIT krb5 
>>> contain these words
>>> 
>>> * Workaround implementation incompatibilities by not generating
>>> * initial sequence numbers greater than 2^30
>>> 
>>> So bad thing could only happen after 2^30 messages.
>>> 
>>> --Max
>>> 
 On Apr 25, 2018, at 10:38 PM, Weijun Wang  wrote:
 
 It's complicated. Looks like MIT krb5 uses a uint32 for old etypes (DES, 
 3DES, RC4) and a uint64 for new ones (AES) [1][2].
 
 I'll do more experiments.
 
 Thanks
 Max
 
 [1] 
 https://github.com/krb5/krb5/blob/master/src/lib/gssapi/generic/util_seqstate.c#L76
 [2] 
 https://github.com/krb5/krb5/blob/master/src/lib/gssapi/krb5/init_sec_context.c#L825
 
> On Apr 24, 2018, at 8:55 PM, Wang Weijun  wrote:
> 
> RFC 4120 5.5.1 has
>> seq-number
>> This optional field includes the initial sequence number to be used by 
>> the KRB_PRIV or KRB_SAFE messages when sequence numbers are used to 
>> detect replays. (It may also be used by application specific messages.) 
>> When included in the authenticator, this field specifies the initial 
>> sequence number for messages from the client to the server. When 
>> included in the AP-REP message, the initial sequence number is that for 
>> messages from the server to the client. When used in KRB_PRIV or 
>> KRB_SAFE messages, it is incremented by one after each message is sent. 
>> Sequence numbers fall in the range 0 through 2^32 - 1 and wrap to zero 
>> following the value 2^32 - 1.
> 
> If it wraps, then it’s 4 bytes.
> 
> I will read more on it.
> 
> Thanks
> Max
> 
>> 在 2018年4月24日，18:08，Valerie Peng  写道：
>> 
>> Hi Max,
>> 
>> Most changes look good. I have only some comments and questions (see 
>> below):
>> 
>> - InitSecContextToken.java, line 62: bad -> unrecognized?
>> - According to the class javadoc of various Token classes and Kerberos 
>> spec, the sequence number is 8-byte integer from header byte 8-15, but 
>> java int have only 4 bytes. The current code seems to assume the first 4 
>> bytes of the sequence number are always 0. For the sake of compliance 
>> and max inter-operability, maybe we should use long to store the 
>> sequence number?
>> 
>> CSR looks good to me.
>> Thanks,
>> Valerie
>> 
>> 
>> 
>>> On 4/18/2018 10:40 AM, Weijun Wang wrote:
>>> Please take a review of this fix:
>>> 
>>> webrev: http://cr.openjdk.java.net/~weijun/8201627/webrev.00/
>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8201814
>>> 
>>> Basically we fix some bugs and introduce a system property so we can 
>>> interop with everyone.
>>> 
>>> Thanks
>>> Max
>>> 
> 

Re: RFR (+CSR) 8201627: Kerberos sequence number issues

[Valerie Peng]

Sure, should be fine...
Valerie

On 4/25/2018 9:48 PM, Weijun Wang wrote:

I filed https://bugs.openjdk.java.net/browse/JDK-8202300 but might not have 
time to make it into JDK 11.

--Max


On Apr 26, 2018, at 12:06 AM, Weijun Wang  wrote:

I'll keep using int32 (at least in this fix), both Java and MIT krb5 contain 
these words

* Workaround implementation incompatibilities by not generating
* initial sequence numbers greater than 2^30

So bad thing could only happen after 2^30 messages.

--Max


On Apr 25, 2018, at 10:38 PM, Weijun Wang  wrote:

It's complicated. Looks like MIT krb5 uses a uint32 for old etypes (DES, 3DES, 
RC4) and a uint64 for new ones (AES) [1][2].

I'll do more experiments.

Thanks
Max

[1] 
https://github.com/krb5/krb5/blob/master/src/lib/gssapi/generic/util_seqstate.c#L76
[2] 
https://github.com/krb5/krb5/blob/master/src/lib/gssapi/krb5/init_sec_context.c#L825


On Apr 24, 2018, at 8:55 PM, Wang Weijun  wrote:

RFC 4120 5.5.1 has

seq-number
This optional field includes the initial sequence number to be used by the 
KRB_PRIV or KRB_SAFE messages when sequence numbers are used to detect replays. 
(It may also be used by application specific messages.) When included in the 
authenticator, this field specifies the initial sequence number for messages 
from the client to the server. When included in the AP-REP message, the initial 
sequence number is that for messages from the server to the client. When used 
in KRB_PRIV or KRB_SAFE messages, it is incremented by one after each message 
is sent. Sequence numbers fall in the range 0 through 2^32 - 1 and wrap to zero 
following the value 2^32 - 1.


If it wraps, then it’s 4 bytes.

I will read more on it.

Thanks
Max


在 2018年4月24日，18:08，Valerie Peng  写道：

Hi Max,

Most changes look good. I have only some comments and questions (see below):

- InitSecContextToken.java, line 62: bad -> unrecognized?
- According to the class javadoc of various Token classes and Kerberos spec, 
the sequence number is 8-byte integer from header byte 8-15, but java int have 
only 4 bytes. The current code seems to assume the first 4 bytes of the 
sequence number are always 0. For the sake of compliance and max 
inter-operability, maybe we should use long to store the sequence number?

CSR looks good to me.
Thanks,
Valerie




On 4/18/2018 10:40 AM, Weijun Wang wrote:
Please take a review of this fix:

webrev: http://cr.openjdk.java.net/~weijun/8201627/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8201814

Basically we fix some bugs and introduce a system property so we can interop 
with everyone.

Thanks
Max

Re: RFR (+CSR) 8201627: Kerberos sequence number issues

[Weijun Wang]

I filed https://bugs.openjdk.java.net/browse/JDK-8202300 but might not have 
time to make it into JDK 11.

--Max

> On Apr 26, 2018, at 12:06 AM, Weijun Wang  wrote:
> 
> I'll keep using int32 (at least in this fix), both Java and MIT krb5 contain 
> these words
> 
> * Workaround implementation incompatibilities by not generating
> * initial sequence numbers greater than 2^30
> 
> So bad thing could only happen after 2^30 messages.
> 
> --Max
> 
>> On Apr 25, 2018, at 10:38 PM, Weijun Wang  wrote:
>> 
>> It's complicated. Looks like MIT krb5 uses a uint32 for old etypes (DES, 
>> 3DES, RC4) and a uint64 for new ones (AES) [1][2].
>> 
>> I'll do more experiments.
>> 
>> Thanks
>> Max
>> 
>> [1] 
>> https://github.com/krb5/krb5/blob/master/src/lib/gssapi/generic/util_seqstate.c#L76
>> [2] 
>> https://github.com/krb5/krb5/blob/master/src/lib/gssapi/krb5/init_sec_context.c#L825
>> 
>>> On Apr 24, 2018, at 8:55 PM, Wang Weijun  wrote:
>>> 
>>> RFC 4120 5.5.1 has
 seq-number
>>> 
 This optional field includes the initial sequence number to be used by the 
 KRB_PRIV or KRB_SAFE messages when sequence numbers are used to detect 
 replays. (It may also be used by application specific messages.) When 
 included in the authenticator, this field specifies the initial sequence 
 number for messages from the client to the server. When included in the 
 AP-REP message, the initial sequence number is that for messages from the 
 server to the client. When used in KRB_PRIV or KRB_SAFE messages, it is 
 incremented by one after each message is sent. Sequence numbers fall in 
 the range 0 through 2^32 - 1 and wrap to zero following the value 2^32 - 1.
>>> 
>>> 
>>> If it wraps, then it’s 4 bytes. 
>>> 
>>> I will read more on it. 
>>> 
>>> Thanks
>>> Max
>>> 
 在 2018年4月24日，18:08，Valerie Peng  写道：
 
 Hi Max,
 
 Most changes look good. I have only some comments and questions (see 
 below):
 
 - InitSecContextToken.java, line 62: bad -> unrecognized?
 - According to the class javadoc of various Token classes and Kerberos 
 spec, the sequence number is 8-byte integer from header byte 8-15, but 
 java int have only 4 bytes. The current code seems to assume the first 4 
 bytes of the sequence number are always 0. For the sake of compliance and 
 max inter-operability, maybe we should use long to store the sequence 
 number?
 
 CSR looks good to me.
 Thanks,
 Valerie
 
 
 
> On 4/18/2018 10:40 AM, Weijun Wang wrote:
> Please take a review of this fix:
> 
> webrev: http://cr.openjdk.java.net/~weijun/8201627/webrev.00/
> CSR: https://bugs.openjdk.java.net/browse/JDK-8201814
> 
> Basically we fix some bugs and introduce a system property so we can 
> interop with everyone.
> 
> Thanks
> Max
> 
 
>>> 
>> 
> 

Re: RFR (+CSR) 8201627: Kerberos sequence number issues

[Weijun Wang]

I'll keep using int32 (at least in this fix), both Java and MIT krb5 contain 
these words

* Workaround implementation incompatibilities by not generating
* initial sequence numbers greater than 2^30

So bad thing could only happen after 2^30 messages.

--Max

> On Apr 25, 2018, at 10:38 PM, Weijun Wang  wrote:
> 
> It's complicated. Looks like MIT krb5 uses a uint32 for old etypes (DES, 
> 3DES, RC4) and a uint64 for new ones (AES) [1][2].
> 
> I'll do more experiments.
> 
> Thanks
> Max
> 
> [1] 
> https://github.com/krb5/krb5/blob/master/src/lib/gssapi/generic/util_seqstate.c#L76
> [2] 
> https://github.com/krb5/krb5/blob/master/src/lib/gssapi/krb5/init_sec_context.c#L825
> 
>> On Apr 24, 2018, at 8:55 PM, Wang Weijun  wrote:
>> 
>> RFC 4120 5.5.1 has
>>> seq-number
>> 
>>> This optional field includes the initial sequence number to be used by the 
>>> KRB_PRIV or KRB_SAFE messages when sequence numbers are used to detect 
>>> replays. (It may also be used by application specific messages.) When 
>>> included in the authenticator, this field specifies the initial sequence 
>>> number for messages from the client to the server. When included in the 
>>> AP-REP message, the initial sequence number is that for messages from the 
>>> server to the client. When used in KRB_PRIV or KRB_SAFE messages, it is 
>>> incremented by one after each message is sent. Sequence numbers fall in the 
>>> range 0 through 2^32 - 1 and wrap to zero following the value 2^32 - 1.
>> 
>> 
>> If it wraps, then it’s 4 bytes. 
>> 
>> I will read more on it. 
>> 
>> Thanks
>> Max
>> 
>>> 在 2018年4月24日，18:08，Valerie Peng  写道：
>>> 
>>> Hi Max,
>>> 
>>> Most changes look good. I have only some comments and questions (see below):
>>> 
>>> - InitSecContextToken.java, line 62: bad -> unrecognized?
>>> - According to the class javadoc of various Token classes and Kerberos 
>>> spec, the sequence number is 8-byte integer from header byte 8-15, but java 
>>> int have only 4 bytes. The current code seems to assume the first 4 bytes 
>>> of the sequence number are always 0. For the sake of compliance and max 
>>> inter-operability, maybe we should use long to store the sequence number?
>>> 
>>> CSR looks good to me.
>>> Thanks,
>>> Valerie
>>> 
>>> 
>>> 
 On 4/18/2018 10:40 AM, Weijun Wang wrote:
 Please take a review of this fix:
 
  webrev: http://cr.openjdk.java.net/~weijun/8201627/webrev.00/
  CSR: https://bugs.openjdk.java.net/browse/JDK-8201814
 
 Basically we fix some bugs and introduce a system property so we can 
 interop with everyone.
 
 Thanks
 Max
 
>>> 
>> 
> 

Re: RFR (+CSR) 8201627: Kerberos sequence number issues

[Weijun Wang]

It's complicated. Looks like MIT krb5 uses a uint32 for old etypes (DES, 3DES, 
RC4) and a uint64 for new ones (AES) [1][2].

I'll do more experiments.

Thanks
Max

[1] 
https://github.com/krb5/krb5/blob/master/src/lib/gssapi/generic/util_seqstate.c#L76
[2] 
https://github.com/krb5/krb5/blob/master/src/lib/gssapi/krb5/init_sec_context.c#L825

> On Apr 24, 2018, at 8:55 PM, Wang Weijun  wrote:
> 
> RFC 4120 5.5.1 has
>> seq-number
> 
>> This optional field includes the initial sequence number to be used by the 
>> KRB_PRIV or KRB_SAFE messages when sequence numbers are used to detect 
>> replays. (It may also be used by application specific messages.) When 
>> included in the authenticator, this field specifies the initial sequence 
>> number for messages from the client to the server. When included in the 
>> AP-REP message, the initial sequence number is that for messages from the 
>> server to the client. When used in KRB_PRIV or KRB_SAFE messages, it is 
>> incremented by one after each message is sent. Sequence numbers fall in the 
>> range 0 through 2^32 - 1 and wrap to zero following the value 2^32 - 1.
> 
> 
> If it wraps, then it’s 4 bytes. 
> 
> I will read more on it. 
> 
> Thanks
> Max
> 
>> 在 2018年4月24日，18:08，Valerie Peng  写道：
>> 
>> Hi Max,
>> 
>> Most changes look good. I have only some comments and questions (see below):
>> 
>> - InitSecContextToken.java, line 62: bad -> unrecognized?
>> - According to the class javadoc of various Token classes and Kerberos spec, 
>> the sequence number is 8-byte integer from header byte 8-15, but java int 
>> have only 4 bytes. The current code seems to assume the first 4 bytes of the 
>> sequence number are always 0. For the sake of compliance and max 
>> inter-operability, maybe we should use long to store the sequence number?
>> 
>> CSR looks good to me.
>> Thanks,
>> Valerie
>> 
>> 
>> 
>>> On 4/18/2018 10:40 AM, Weijun Wang wrote:
>>> Please take a review of this fix:
>>> 
>>>   webrev: http://cr.openjdk.java.net/~weijun/8201627/webrev.00/
>>>   CSR: https://bugs.openjdk.java.net/browse/JDK-8201814
>>> 
>>> Basically we fix some bugs and introduce a system property so we can 
>>> interop with everyone.
>>> 
>>> Thanks
>>> Max
>>> 
>> 
> 

Re: RFR (+CSR) 8201627: Kerberos sequence number issues

[Valerie Peng]

Hi Max,

Most changes look good. I have only some comments and questions (see below):

- InitSecContextToken.java, line 62: bad -> unrecognized?
- According to the class javadoc of various Token classes and Kerberos 
spec, the sequence number is 8-byte integer from header byte 8-15, but 
java int have only 4 bytes. The current code seems to assume the first 4 
bytes of the sequence number are always 0. For the sake of compliance 
and max inter-operability, maybe we should use long to store the 
sequence number?


CSR looks good to me.
Thanks,
Valerie



On 4/18/2018 10:40 AM, Weijun Wang wrote:

Please take a review of this fix:

webrev: http://cr.openjdk.java.net/~weijun/8201627/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8201814

Basically we fix some bugs and introduce a system property so we can interop 
with everyone.

Thanks
Max

RFR (+CSR) 8201627: Kerberos sequence number issues

[Weijun Wang]

Please take a review of this fix:

   webrev: http://cr.openjdk.java.net/~weijun/8201627/webrev.00/
   CSR: https://bugs.openjdk.java.net/browse/JDK-8201814

Basically we fix some bugs and introduce a system property so we can interop 
with everyone.

Thanks
Max