eclipse-californium / californium

CoAP/DTLS Java Implementation
https://www.eclipse.org/californium/
Other
724 stars 363 forks source link

RawData.isSecure() depends on DTLS #156

Closed boaks closed 6 years ago

boaks commented 7 years ago
     RawData

public boolean isSecure() {
    return (correlationContext != null &&
            correlationContext.get(DtlsCorrelationContext.KEY_SESSION_ID) != null);
}

Remove DTLS dependeny by introducing a "isSecure()" method into CorrelationContext?

boaks commented 7 years ago

If wanted, into PR #148 or new PR?

sophokles73 commented 7 years ago

There is no dependency on DTLS. There is just a dependency on a costant declared in class DtlsCorrelationContext. Since both classes are in the same package I do not see a problem here ...

boaks commented 7 years ago

Sure, there is no dependency to scandium, but to DTLS! If TLS (I hope its also secure :-) uses TlsCorrelationContext, this test is only true, because TlsCC uses the same key. But I would prefer to have a method ...

sophokles73 commented 7 years ago

The idea is to use the keys defined by DtlsCorrelationContext. Otherwise, a Connector may implement whatever type of CorrelationContext it wants. So, TlsCorrelationContext (if you want to create it), needs to use this key by definition instead of by coincidence.

sbernard31 commented 6 years ago

I continue the discussion here, because it's more appropriate.

Looking more deeper to the code. It seems that isSecure is just used to set request scheme for incoming request on a specific endpoint.

Currently : The code allow connector supporting several schemes. (coap, coaps, coap-tcp, ...) CoapEndpoint allow to support only 2 schemes (secure and unsecure). RawData.isSecure() depends on DTLS.

This seems not so clean to me. 1) I don't think that connector should express coap scheme, it is too coap dependent. => we should replace it by supported protocol (UDP, DTLS,TCP, TLS, ...). CoapEndpoint should deduce the supported scheme (coaps/coap/coap-tcp ...) from supported protocol. 2) I don't know if we want to keep connector/endpoint supporting several scheme/protocol. Currently our connector/endpoint support only one and I don't know how this could be different. => If we chose that connector/endpoint support only one protocol then we can remove the isSecure on RawData. => if we still want to support several protocol for connector/endpoint, I think this is up to the connector to say if this is a secure rawData or not, adding a boolean to RawData.inbound

boaks commented 6 years ago

FMPOV: a Endpoint has one Connector. Therefore the used scheme is defined by that connector and isSecureseems to be not longer relevant. I would therefore vote for expose the scheme of the connector (String getScheme()) rather the improving isSecure(). If successful, isSecure may even be removed.

boaks commented 6 years ago

If really multiple scheme on one Connectoror one Endpointshould be supported, then I would go for addding the scheme to the EndpointContext.

sbernard31 commented 6 years ago

I do not understand why you don't agree with this proposition :

I don't think that connector should express coap scheme, it is too coap dependent. => we should replace it by supported protocol (UDP, DTLS,TCP, TLS, ...). CoapEndpoint should deduce the supported scheme (coaps/coap/coap-tcp ...) from supported protocol.

boaks commented 6 years ago

I do not understand why you don't agree with this proposition :

Because I had not enough time to read it ;-) As long, as we stay to keep it simple (one endpoint one connector) it seems for me ok to expose the base protocol in the connector and determine the coap flavour with that.

sbernard31 commented 6 years ago

So I suppose we need to agree about how many protocol a connector can support.

Personally, I will go for 1 connector 1 protocol.

It's seems to match to our current implementation. and I don't really see why we need to support more than that. (We can imagine connector which support TCP+TLS with something like STARTTLS but AFAIK it's only for TEXT protocol and I don't even know if something like that exist for binary protocol...)

sophokles73 commented 6 years ago

+1 for supporting just one transport protocol per Connector and deriving the CoAP flavor from it in the Endpoint.

boaks commented 6 years ago

Can we close this again, #428 is merged now?

sbernard31 commented 6 years ago

👍