Redesign peer's Identity class.

[sbernard31]

At #1421, we discover that we have maybe an issue with current Identity class/concept in Leshan.

For CoAP, Identity contains only "socket address" (I think that makes sense, because we have nothing more to use)
For CoAPs, Identity contains "TLS Identifier" + "socket address" and maybe this is not so good, "socket peer address" should not be used to identify the peer.
If that was the case when we search by identity for CoAPs even if socket address changed we will be able to find the profile.

(from : #1421 (comment))

This issue aims to talk about a new design.

This will probably lead to some Redis Serialization break again 😬 (See #1417)

[sbernard31]

Some thoughts about this :

Current Design:

Currently we have 1 class called Identity with these attributes :

public class Identity {
  InetSocketAddress peerAddress;
  String pskIdentity;
  PublicKey rawPublicKey;
  String x509CommonName;
  OscoreIdentity oscoreIdentity;
  ...
}

This idea of that class was to identified a Foreign Peer (Client or Server)

Current Drawbacks :

But there is maybe several issues with this design :

1. we see just above that's not alway a good idea to consider peerAddress as part of identity.
2. Adding new kind of identity means create new fields in the only 1 existing class ...

Some Concepts :

Here some similar concept in Java :

• https://docs.oracle.com/javase/7/docs/technotes/guides/security/jgss/tutorials/glossary.html
• https://stackoverflow.com/a/5025140/5088764

I'm not sure we should reuse those classes but we should maybe create a new class hierarchy to separate foreign peer and identity concepts ? About naming I don't know ?

• For Foreign Peer : Peer, LwM2mPeer ?
• For Identity : Identity LwM2mIdentity ?

Proposed Design :

I guess classes could looks like

public interface Peer {
  Identity getIdentity()
}

public class IpPeer implements Peer {  // This class is to anticipate Non-IP protocol
  InetSocketAddress getSocketAddress()
  Identity getIdentity()
}


public interface Identity {
  String getKeyIdentifier() // TODO I don't know if we really need this. 
  String toString();
  boolean equals(Object obj)
  int hashCode();
}

public class SocketIdentity implements Identity {
  InetSocketAddress getSocketAddress();
}

public class PskIdentity implements Identity {
  String getIdentity();
}
public class RpkIdentity implements Identity {
  PublicKey getPublicKey();
}
public class X509Identity implements Identity {
  String getX509CommonName(); // when we will go futher we will probably see that current design is too limited. 
}

public class OscoreIdentity implements Identity {
   byte[] getRecipientId();
}

OSCORE + TLS case :

Now there is one problem with this design ... OSCORE can be used togeter with (D)TLS protocol.
So to support that either :

• we go with 1 Peer could have several Identities.
• OR we create and an OscoreOverTlsIdentity which looks like this

public class OscoreOverTlsIdentity implements Identity {
   OscoreIdentity getOscoreIdentity();
   Identity getTlsIdentity(); // we can create a TlsIdentity interface. 
}

@JaroslawLegierski, @jvermillard, @rikard-sics if you have any opinions / ideas about this, do not hesitate to share.

[sbernard31]

Just see, we maybe also need to think about Identity of Gateway (#1237)
https://github.com/eclipse/leshan/blob/a9f5ab469d22a44af68713217ba8a292bb66706f/leshan-server-core/src/main/java/org/eclipse/leshan/server/registration/RegistrationHandler.java#L139-L142

[JaroslawLegierski]

Maybe the name LwM2mIdentity would be better to differentiate new identity design from the current solution ?

for non IP devices we can extend IpPeer class by adding in the future:

e.g. for LoRaWAN
DevEUI getDevEUI()
or
devId getdevId() for NIDD (where devId is: IMEI, IMSI, MSISDN, ICCID, ... etc)

for OSCORE with (D)TLS proposal of usage OscoreOverTlsIdentity class is in my opinion very good idea.

[sbernard31]

@JaroslawLegierski thx for feedback.

Maybe the name LwM2mIdentity would be better to differentiate new identity design from the current solution ?

You think this will help for migration ?

[JaroslawLegierski]

@JaroslawLegierski thx for feedback.

Maybe the name LwM2mIdentity would be better to differentiate new identity design from the current solution ?

You think this will help for migration ?

Yes mainly for migration

For better understanding your concept I started preparing PoC

From what I understood Identity will be replaced by IpPeer from API user point of view e.g: here ?

I'm not sure what the getKeyIdentifier() method should return something like this "EP#IDENTITY#" (for Redis)?

[sbernard31]

For better understanding your concept I started preparing PoC

👍

From what I understood Identity will be replaced by IpPeer from API user point of view e.g: here ?

Yep.

I'm not sure what the getKeyIdentifier() method should return something like this "EP#IDENTITY#" (for Redis)?

Nope.

I was thinking that maybe this getter could help store implementation as this string could be used by store as index to search by Identity. E.G. a PskIdentity will maybe returns something like psk#my_device_identity, RpkIdentity maybe rpk#my_public_key_as_hex_or_base64

But I'm not sure this is a good idea to do that in that class.
Maybe better to let store to that ?
We need to decide which class is responsible to do that.

[JaroslawLegierski]

I created first working version IpPeer implementation:

• (old) Identity was eliminated and replaced by IpPeer
• Maybe we need replace getSocketAddress() by getPeerAddress() in IpPeer class ? ( this change would reduce the number of changes to the existing code)
• I focused on running RedisRegistrationStore (In MemoryRegistrationStore has not been tested and is not correctly implemented) however for rpk I still have an JSON serialization error (psk, X509 and unsecure clients are registering without errors)
• I didn't modify javadoc (for new added classes javadocs don't exist yet)

[sbernard31]

👍 I will start to look at this on 3rd May.

[sbernard31]

I created PR #1445 with your work (2 commit squashed in one) rebased on master + some additions.

About,

(old) Identity was eliminated and replaced by IpPeer

I think this not just about replacing Identity by IpPeer, some Identity must be replaced by Peer, some other by IpPeer and other one by LwM2mIdentity. I will try to move forward on this this afternoon. 🙂

[sbernard31]

Since #1436 is integrated in master, we can close this one.