Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ping implementation send Empty Message with Uri-Host option #677

Closed
hirokiht opened this issue Jun 25, 2018 · 18 comments
Closed

Ping implementation send Empty Message with Uri-Host option #677

hirokiht opened this issue Jun 25, 2018 · 18 comments

Comments

@hirokiht
Copy link

hirokiht commented Jun 25, 2018

According to RFC7252,

Empty Message
A message with a Code of 0.00; neither a request nor a response.
An Empty message only contains the 4-byte header.

The ping implementation sends an empty message with Uri-Host option, makes it incompatible with other standard compliant CoAP libraries such as libcoap.

@vikram919
Copy link
Contributor

vikram919 commented Jun 25, 2018

IIUC

request.setURI(uri);

this means not setting UriHost option, but to mention about destination address of the empty ping request.

to set uri host option in a request : request.getOptions().setUriHost(uri);

makes it incompatible with other standard compliant CoAP libraries such as libcoap.

Can I know how you have tested it?

@hirokiht
Copy link
Author

I discovered when using the CoAP Shell (https://github.com/tzolov/coap-shell), which is based on Callifornium. As libcoap fails the packet sanity check, I fired up Wireshark to take a look. I confirmed it by removing pdu->used_size != 0 in the coap_pdu_parse_opt (pdu.c) in libcoap, then recompiled it.

@boaks
Copy link
Contributor

boaks commented Jun 25, 2018

This smells very strong for a RFC ambiguity :-) .

https://tools.ietf.org/html/rfc7252#section-5.10.1

A "ping" of 4 bytes could then not be used, if "multiple virtual servers" are used.
"Explicit Uri-Host and Uri-Port Options are typically used when an endpoint hosts multiple virtual servers."

@vikram919
Copy link
Contributor

vikram919 commented Jun 25, 2018

I have tested with PlugtestClient.java from demo-apps/cf-plugtest-client californium with coap-server.o libcoap it's true that libcoap server prints "WARN discard malformed PDU" message

@hirokiht
Copy link
Author

IMHO, a "ping" doesn't really need to reach "virtual server", just the hosting server is good enough. After all, ping is just a simplistic tool. If the virtual server somehow is unable to respond to normal requests, the virtual server (or hosting server if configured so) would be responsible to send error respond codes.

@boaks
Copy link
Contributor

boaks commented Jun 26, 2018

I agree, that one interpretation maybe, that the ping targets only the real interface (hosting server).
But I don't see, that this is the only possible interpretation. It's also possible, that the combination was simply not analysed and so not specified. So I would prefer to have an explicit statement from the IETF core to clarify the expected behaviour.

@hirokiht
Copy link
Author

hirokiht commented Jun 26, 2018

In Section 4.1 of RFC7252:

An Empty message has the Code field set to 0.00. The Token Length
field MUST be set to 0 and bytes of data MUST NOT be present after
the Message ID field. If there are any bytes, they MUST be processed
as a message format error.

Maybe Californium can provide an alternative "ping" implementation which doesn't uses Empty Message can be used to ping virtual servers, otherwise implementation using Empty Message shouldn't include additional options. Although for virtual servers which doesn't support resource discovery, it might be difficult to determine which request method to use for pinging.

@boaks
Copy link
Contributor

boaks commented Jun 26, 2018

Sorry, too frequently, sticking to one sentence, which obviously not mention the other use case, is not sufficient for clarification. That's at least my experience. Such a "one maverick sentence" may just be an artefact of a "maverick idea", which is not fitted into the other definitions.
So, FMPOV, a clarification from the IETF core would be the preferred way to handle this issue.

@hirokiht
Copy link
Author

IMHO, compatibility should be a priority concern, even if revision is made, there could be other CoAP implementations which doesn't support additional options for Empty message. I agree that it'd be preferable to get a clarification from IETF for future reference.

@boaks
Copy link
Contributor

boaks commented Jun 26, 2018

OK, then there are two ways to continue:

  1. search for other implementations than californium and libcoap.
    If the majority sticks to 4 bytes, then change it for compatibility with the majority
    (and potentially back, if the clarification results in a different interpretation).

  2. try to ask the core mailing list for clarification.

tzolov added a commit to tzolov/coap-shell that referenced this issue Jun 26, 2018
 Resolves #1

  - Workaround until eclipse-californium/californium#677 is resolved in a way that allows libcoap compatibility
  - Adjust README for 1.0.7 release
@tzolov
Copy link

tzolov commented Jun 26, 2018

@boaks this sounds like a reasonable plan. Even the best specification efforts leave ambiguous definitions and in result alternative interpretations and implementations. From practical standpoint adherence to the de-facto standards (e.g. majority of vendor implementations) is more useful than sticking with a particular spec. interpretation (even if it is the correct one).

@hirokiht , in the meantime if dropped the ping from the coap-shell' connection command. Hopefully this would allow you to use the other commands.

@sbernard31
Copy link
Contributor

sbernard31 commented Jun 26, 2018

Just to let you know, the Uri-Host Option is only set if you are using an hostname. If you are using an IP address. The ping should only contains 4 bytes. (tested with californium 2.0.0-M10)

We could also open a ticket in libcoap to see if it makes sense to us to be less strict about this 4 bytes constraints ...

@boaks
Copy link
Contributor

boaks commented Jun 26, 2018

Just to mention, a 4 byte ping will not be able to contain a "path".

https://github.com/tzolov/coap-shell/blob/master/src/main/java/io/datalake/coap/coapshell/command/CoapShellCommands.java#L177

this.coapClient.setURI(this.coapClient.getURI() + path);
return this.coapClient.ping(5000);

So, if this 4 bytes ping is the intended interpretation, you don't need to change the URI.
For californium it seems the to be the best, to change the serializer. Even if I don't really like it, we can add a "temporary configuration switch" for that. Should we?

@boaks
Copy link
Contributor

boaks commented Jun 26, 2018

When someone can check, if changing the private boolean CoapClient.ping(Long timeout) to

Response response = send(request, outEndpoint).waitForResponse(timeout);
return request.isRejected() || response != null;

works with libcoap, then FMPOV we can change californium.

@vikram919
Copy link
Contributor

vikram919 commented Jun 26, 2018

sorry it did not work

@boaks
Copy link
Contributor

boaks commented Jul 3, 2018

I checked nCoAP and with that, pings containing the URI host option works.

Anyway, in the meantime, I have my doubts, how the "virtual server" stuff should work. Should multiple "virtual server" share the same coap-endpoint? If so, how could then a client endpoint ensure, that the request tokens are unique per endpoint? This means the client must determine the host for the destination URI ahead to detect, if two host are "virtual server" and share the same endpoint.

So, for now, I think, make the "auto URI host" feature configurable will be the best.

@boaks
Copy link
Contributor

boaks commented Dec 17, 2018

If you still interested, please retest with PR #828

@boaks boaks added this to the 2.0.0-M13 milestone Dec 17, 2018
@boaks
Copy link
Contributor

boaks commented Jan 9, 2019

Fixed with merging PR #828

@boaks boaks closed this as completed Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants