Am 2020-05-24 um 22:29 schrieb Thomas Maslen:
[Off-list reply just to you two gents. Feel free to forward it to the list
if it helps, or ignore it if it doesn't]
I'm just adding my 0.02 on zero vs 255 for the "no address" type in a
channel binding
---------- Forwarded message ----------
From: Michael Osipov <1983-01...@gmx.net>
To: Alexey Bakhtin <ale...@azul.com>, "core-libs-...@openjdk.java.net" <
core-libs-...@openjdk.java.net>
Cc: "security-dev@openjdk.java.net" <security-dev@openjdk.java.net>
Bcc:
Date: Sun, 24 May 2020 01:38:06 +0200
Subject: Re: RFR: 8245527: LDAP Cnannel Binding support for Java
GSS/Kerberos
Am 2020-05-21 um 09:35 schrieb Alexey Bakhtin:
Hello,
Could you please review the following patch:
JBS: https://bugs.openjdk.java.net/browse/JDK-8245527
Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v0/
Let's go through your changes statically:
[...]
private static final int CHANNEL_BINDING_AF_INET = 2;
private static final int CHANNEL_BINDING_AF_INET6 = 24;
private static final int CHANNEL_BINDING_AF_NULL_ADDR = 255;
+ private static final int CHANNEL_BINDING_AF_UNSPEC = 0;
This should sort from 0 to 255 and not at the end.
Understood, then this requires an inline comment.
Usually I would agree, but in this case there is a perverse relationship
between 255 and 0 (see below), so grouping them together like this _may_
help the reader to understand that there's something funny going on.
private int getAddrType(InetAddress addr) {
- int addressType = CHANNEL_BINDING_AF_NULL_ADDR;
+ int addressType = CHANNEL_BINDING_AF_UNSPEC;
// initialize addrtype in CB first
- cb->initiator_addrtype = GSS_C_AF_NULLADDR;
- cb->acceptor_addrtype = GSS_C_AF_NULLADDR;
+ cb->initiator_addrtype = GSS_C_AF_UNSPEC;
+ cb->acceptor_addrtype = GSS_C_AF_UNSPEC;
This looks wrong to me -- as you already mentioned -- this violates RFC
2744, section 3.11, last sentence:
or omit addressing information, specifying
GSS_C_AF_NULLADDR as the address-types.
Michael: what's going on here, I believe, is that in theory of course one
should do what the RFC says and use GSS_C_AF_NULLADDR (255) but instead all
the main implementations with which one would want to interoperate blithely
use zero (because, IMO, they were written by bloody C programmers) -- and
Alexey is doing the best he can to interoperate with those rather than with
the letter of the spec that noone implements.
viz previous discussion on the IETF kitten list:
https://mailarchive.ietf.org/arch/msg/kitten/Jx8ok5BBHu5GIrpdu8i6-4NtsZM/
which refers to:
https://mailarchive.ietf.org/arch/msg/kitten/ZGkBijVg080plyyQOv53bL08qRU/
/* release initiator address */
- if (cb->initiator_addrtype != GSS_C_AF_NULLADDR) {
+ if (cb->initiator_addrtype != GSS_C_AF_NULLADDR &&
+ cb->initiator_addrtype != GSS_C_AF_UNSPEC) {
resetGSSBuffer(&(cb->initiator_address));
}
/* release acceptor address */
- if (cb->acceptor_addrtype != GSS_C_AF_NULLADDR) {
+ if (cb->acceptor_addrtype != GSS_C_AF_NULLADDR &&
+ cb->acceptor_addrtype != GSS_C_AF_UNSPEC) {
resetGSSBuffer(&(cb->acceptor_address));
}
Unspecified does not mean that it is null.
[...]
Generally, I have two fundemental issues:
* How do you know that address type must be UNSPEC and not NULLADDR?
Trial and error?
Pretty much...
* Changing the default address type against the RFC will break every
installation using channel bindings relying on it with cross GSS-API
implementations.
In theory, yes; in practice, the opposite.
So I read the discussions. RFC 2744 shall not be changed, people
admitted that the spec of SASL GS2 mechs is wrong and should be changed,
but this has not happened yet. It remained at UNSPEC all the years.
So we have several issues here:
* GSS-API C bindings and SASL requests are two distinct RFCs which
require/mandate differnt things.
* The change in JGSS in unrelated to this patch because GSS-API knows
nothing about SASL and its fauly spec.
Since we are doing LDAP over SASL here and RFC 5801 requires to be
UNSPEC (0) the SASL TlsChannelBinding class must take that into account.
Unfortunately, JGSS implies with the args of the ChannelBinding the type
fo the adress. So will change my opinion a bit:
No property for AD/non-AD is necessary, but handling of UNSPEC is
required. JGSS shall remain at NULLADDR. The subtype
UnspecEmptyInetAddress should be at least evaluated.
Michael
