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

Request.scheme is not set properly or at all #38

Closed
calif-devel opened this issue Apr 21, 2016 · 9 comments
Closed

Request.scheme is not set properly or at all #38

calif-devel opened this issue Apr 21, 2016 · 9 comments
Labels

Comments

@calif-devel
Copy link
Contributor

I'm working on a HTTP-CoAP ProxyServer based on Californium and I'm using cf-rd from californium.tools project. My intention is to support both coap and coaps endpoints on the proxy server and with help of cf-rd tools, the CoAP peers can POST /rd using either coap or coaps.

However I noticed that although my CoAP device registered using coaps, the logs showed

INFO [RDResource]: Adding new endpoint: coap://127.0.0.1:50961

On further investigation, in RDNodeResource class, when LinkFormat.CONTEXT is either missing or empty, the code assumes the scheme to be coap. I can't set LinkFormat.CONTEXT in URIQuery when positng on resource /rd, because of NAT. Based on RFC, if LinkFormat.CONTEXT is not set, then source address and port should be used. RDNodeResource class uses Request object to correctly set the source address and port, but not the scheme.

So I decided to change the logic in class RDNodeResource to rely on Request.getScheme() method to create appropriate LinkFormat.CONTEXT. But this change produced the following in logs

INFO [RDResource]: Adding new endpoint: //127.0.0.1:50961

Is Request.scheme ever set? Which component is responsible of setting this variable to right value? Is there a better way to setting the LinkFormat.CONTEXT in RDNodeResource other than using Request object?

@sophokles73
Copy link
Contributor

My understanding is that we are talking about an incoming CoAP request, right?
The problem here seems to be that (currently) Californium does not know whether the incoming request has been sent via coap or coaps. The problem is that Californium's CoAP layer is independent from the underlying transport layer so we will need to figure out a way for the CoAP layer to determine whether the Endpoint we have received the request on is secure (DTLS) or not.

@calif-devel
Copy link
Contributor Author

Yes, you are right. Currently it is not possible to know if a request is coming from a coaps endpoint. So if a CoAP peer wants to send a follow up request to a peer, using the RDNodeResource.getContext(), sends in this new request as plain (non-DTLS) CoAP request to a port listening for DTLS CoAP requests.

In Class CoapEndpoint.InboxImpl where request object created, is this a valid fix?

if (raw.getCorrelationContext().get(DtlsCorrelationContext.KEY_SESSION_ID) != null) {
    request.setScheme(CoAP.COAP_SECURE_URI_SCHEME);
} else {
    request.setScheme(CoAP.COAP_URI_SCHEME);
}
request.setSource(raw.getAddress());
request.setSourcePort(raw.getPort());
request.setSenderIdentity(raw.getSenderIdentity());

@calif-devel
Copy link
Contributor Author

I guess additionally, RDNodeResource class need to be changed to take the request.getScheme() into consideration to properly set the RDNodeResource.context variable.

@sophokles73
Copy link
Contributor

@calif-devel,

In Class CoapEndpoint.InboxImpl where request object created, is this a valid fix?

I had exactly the same idea :-)
However, I think we should encapsulate this test in RawData, e.g.

if (raw.isSecure()) {
    request.setScheme(CoAP.COAP_SECURE_URI_SCHEME);
} else {
    request.setScheme(CoAP.COAP_URI_SCHEME);
}

This way we do not expose too much of the underlying mechanism around CorrelationContext to upper layers ...

Would you like to create a PR? :-)

@mkovatsc
Copy link

I like the fix.

Another comment: Do not rely too much on the RD implementation. It was a contribution that has never been fully overhauled, and hence is of lower code quality... (yet I did some improvements a while ago).

This means, whenever you find something strange in the RD code, do not hesitate to ask about it! ;)

@calif-devel
Copy link
Contributor Author

Fixed and submitted as Pull Request #42

@sophokles73
Copy link
Contributor

I will take a look (and merge) tomorrow.
Thanks again, @calif-devel

sophokles73 pushed a commit that referenced this issue Apr 29, 2016
Set scheme variable of Request object when it is created in the
CoapEndpoint.InboxImpl.  This helps applications to inspect the scheme
just as source address, port or principle.  New method
RawData.isSecure() inspects the CorrelationContext to determine if it is
from a secure context.

Bug: #38
Signed-off-by: Vinod Mukkamala <vmukkamala@hotmail.com>
Also-by: Kai Hudalla <kai.hudalla@bosch-si.com>
@sophokles73
Copy link
Contributor

Hi @calif-devel,

I guess you can now close this issue ... thanks again for contributing :-)

@calif-devel
Copy link
Contributor Author

Thank you @sophokles73 and @mkovatsc for resolving this.

maradrian pushed a commit to maradrian/californium that referenced this issue May 19, 2016
Set scheme variable of Request object when it is created in the
CoapEndpoint.InboxImpl.  This helps applications to inspect the scheme
just as source address, port or principle.  New method
RawData.isSecure() inspects the CorrelationContext to determine if it is
from a secure context.

Bug: eclipse-californium#38
Signed-off-by: Vinod Mukkamala <vmukkamala@hotmail.com>
Also-by: Kai Hudalla <kai.hudalla@bosch-si.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants