Hi:
On Wed, Nov 13, 2013 at 10:58 AM, Jan Willamowius jan@willamowius.de wrote:
what I fixed was a different issue where H323Plus endpoints wouldn't try to re-register. Francisco issue is which endpoint id is used.
Thanks, I was trying to analyze that commit ( which seemed totally unrelated ) when your message popped, that saved me a lot of work.
I can reproduce the issue Francisco reports with GnuGk when I set AcceptEndpointIdentifier=0. With that setting H323Plus endpoints will keep sending their old endpoint identifier, even if the gatekeeper assigned them a new identifier after a full RRQ.
I will then consider this a bug. Presently we have made our gatekeeper resistant to this behaviour by implementing something like your AcceptEndpointIdentifier flag in our gatekeeper, it basically is a one (logical) line modification of H323GateKeeperServer::CreateRegisteredEndpoint in gkserver.cxx ( plus one boolean member and a little logic to set it from our config file ). I've shamelessly copied you idea of controlling it via a flag. In case it is useful for someone our new method is:
H323RegisteredEndPoint * gk_server::CreateRegisteredEndPoint(H323GatekeeperRRQ & request) { return new reg_ep(*this, CSTR(accept_ep_identifier && request.rrq.HasOptionalField(H225_RegistrationRequest::e_endpointIdentifier) ? request.rrq.m_endpointIdentifier.GetValue() : CreateEndPointIdentifier() ) ); }
( gk_server and reg_ep are our derived classed ) I analized the code paths to assert that no endpoint with RRQ endpointIdentifier could be in the gatekeeper when this method is reached, which seemed correct.
I think the same behaviour could be bolted into the standard gkserver.cxx, making it more robust on the presence of undisciplined endpoints, by just adding the flag to it and using the appropiate classes.
With this behaviour our system works with both h32plus v1.18 ( some legacy code we have ), v1.24, v1.25 and v1.25 with patches 1.23&24 reverted in gkclient and with the one I previously sent ( which I'm considering reverting it, as my patch contains an if(!empty) which I think is erroneous, as EndpointIdentifier seems to be defined as
EndpointIdentifier ::= BMPString (SIZE(1..128))
which means it will never be empty, and it should fail on decoding.
I believe the change in gkclient.cxx should be reverted so the endpointIdentifier provided by the gatekeeper in RCF should unconditionally be used, not only if the id was empty before.
FWIW, before modifying my gatekeeper I reverted patches v1.23 & 24 from h323plus 1.25, and it worked fine, the endpoints started working. So presently I'm doubly protected, as my patched h323plus sends the endpointIdentifier received in the LAST RCF, instead of the first, and my gatekeeper uses the endpointIdentifier passed from the endpoint if present.
I think the wording is unclear in the little documents I have, they just says "endpointIdentifier – A gatekeeper assigned terminal identity string; shall be echoed in subsequent RAS messages.", but omits to say what to do if the endpoint has received more than one. Even though, I think the logical thing to do in this protocolos is to always echo the last one.
I'll continue on upgrading all our machines to v1.25, patched, and our gatekeepers to the new release. If you want some test or any other thing, I'll monitor this and try to do what I can.
Francisco Olarte.