eclipse-leshan / leshan

Java Library for LWM2M
https://www.eclipse.org/leshan/
BSD 3-Clause "New" or "Revised" License
652 stars 407 forks source link

Custom Authorizer for Bootstrap server #1359

Closed Warmek closed 1 year ago

Warmek commented 1 year ago

Hi, my name is Bartosz and I will be working as contributor from Orange Polska.

There wasn't an Authorizer for Bootstrap Server. We need customizable Authorizers in Boostrap Server, similarly to Leshan Server.

I've made my first commit on opl/bootstrap_authorizer

sbernard31 commented 1 year ago

Hi and welcome,

The current way to manage if a device is authorize to start a bootstrap session is handled by BootstrapSessionManager.

The javadoc says :

/**
 * Manages life cycle of a bootstrap process.
 * <p>
 * This class is responsible to accept or refuse to start new {@link BootstrapSession}, then to provide request to send.
 * It also decide when session should continue, finished or failed.
 *
 * @see DefaultBootstrapSessionManager
 * @see BootstrapSession
 */
public interface BootstrapSessionManager {
   ....

    /**
     * Starts a bootstrapping session for an endpoint. In particular, this is responsible for authorizing the endpoint
     * if applicable.
     *
     * @param request the bootstrap request which initiates the session.
     * @param clientIdentity the {@link Identity} of the client.
     *
     * @return a BootstrapSession, possibly authorized.
     */
    public BootstrapSession begin(BootstrapRequest request, Identity clientIdentity);

See default implementation DefaultBootstrapSessionManager :

@Override
public BootstrapSession begin(BootstrapRequest request, Identity clientIdentity) {
    boolean authorized;
    if (bsSecurityStore != null && securityChecker != null) {
        Iterator<SecurityInfo> securityInfos = bsSecurityStore.getAllByEndpoint(request.getEndpointName());
        authorized = securityChecker.checkSecurityInfos(request.getEndpointName(), clientIdentity, securityInfos);
    } else {
        authorized = true;
    }
    DefaultBootstrapSession session = new DefaultBootstrapSession(request, clientIdentity, authorized);
    LOG.trace("Bootstrap session started : {}", session);

    return session;
}

Could it match your use case ? Reading your code, I guess you already see that so maybe I missed something ? :thinking:

The rational behind this is that often the class responsible to authorize the device will maybe need to store custom data in BootstrapSession. (Note this is pretty much the same needs for Registration See : https://github.com/eclipse/leshan/issues/1293 and for now I don't know what is the best approach)

Warmek commented 1 year ago

My colleagues from project have asked me to implement interface BootstrapAuthorizer, so we could write custom authorizers, similarly to how it was done here:

public interface Authorizer {

    /**
     * Return the registration if this request should be handled by the LWM2M Server. When <code>null</code> is returned
     * the LWM2M server will stop to handle this request and will respond with a {@link ResponseCode#FORBIDDEN} or
     * {@link ResponseCode#BAD_REQUEST}.
     * <p>
     * Some Application Data could be attached to the Registration using :
     *
     * <pre>
     * return new Registration.Builder(registration).applicationData(myAppData).build();
     * </pre>
     *
     * @param request the request received
     * @param registration the registration linked to the received request.<br>
     *        For register request this is the registration which will be created<br>
     *        For update request this is the registration before the update was done.
     * @param senderIdentity the {@link Identity} used to send the request.
     *
     * @return the registration if this request is authorized or <code>null</code> it is not authorized.
     */
    Registration isAuthorized(UplinkRequest<?> request, Registration registration, Identity senderIdentity);
}

and then

/**
 * A default {@link Authorizer} implementation
 *
 * It checks in {@link SecurityStore} if there is a corresponding {@link SecurityInfo} for this registration endpoint.
 * If there is a {@link SecurityInfo} it check the identity is correct, else it checks if the LWM2M client use an
 * unsecure connection.
 */
public class DefaultAuthorizer implements Authorizer {

    private SecurityStore securityStore;
    private SecurityChecker securityChecker;

    public DefaultAuthorizer(SecurityStore store) {
        this(store, new SecurityChecker());
    }

    public DefaultAuthorizer(SecurityStore store, SecurityChecker checker) {
        securityStore = store;
        securityChecker = checker;
    }

    @Override
    public Registration isAuthorized(UplinkRequest<?> request, Registration registration, Identity senderIdentity) {

        // do we have security information for this client?
        SecurityInfo expectedSecurityInfo = null;
        if (securityStore != null)
            expectedSecurityInfo = securityStore.getByEndpoint(registration.getEndpoint());
        if (securityChecker.checkSecurityInfo(registration.getEndpoint(), senderIdentity, expectedSecurityInfo)) {
            return registration;
        } else {
            return null;
        }
    }
}

So instead of hard coding your authorizer in BootstrapSession begin() we could make our own authorizers.

We would be gratefull if you could aprove this change

sbernard31 commented 1 year ago

My colleagues from project have asked me to implement interface BootstrapAuthorizer, We would be gratefull if you could aprove this change

I understand you but I try to base my choices on technical reason, not just making your colleagues happy :grin:
So no offense but probably better to come with some argument which drive the modification.
But don't worry, I digged a bit into this and I agree that current API is not so good. :point_down:

I tried to achieve this without code modification.
This looks to something like :

LeshanBootstrapServerBuilder builder = new LeshanBootstrapServerBuilder();

final BootstrapSecurityStore securityStore =  new YourSecurityStoreIfNeeded();
final SecurityChecker securityChecker =  new SecurityChecker();
builder.setSessionManager(new DefaultBootstrapSessionManager(securityStore, new InMemoryBootstrapConfigStore()) {

    @Override
    public BootstrapSession begin(BootstrapRequest request, Identity clientIdentity) {
              boolean authorized =  isAuthorized(request, clientIdentity);
              DefaultBootstrapSession session = new DefaultBootstrapSession(request, clientIdentity, authorized);
              return session;
    }

    private boolean isAuthorized(BootstrapRequest request, Identity clientIdentity) {
        // put your custom code here
        Iterator<SecurityInfo> securityInfos = securityStore.getAllByEndpoint(request.getEndpointName());
              return  securityChecker.checkSecurityInfos(request.getEndpointName(), clientIdentity, securityInfos);
    }
});

This is not too much code but I agree this is not so elegant... Especially that we need to create final variable as attribute in DefaultBootstrapSessionManager are private. And we pass a securityStore to DefaultBootstrapSessionManager which will never be used.

So maybe you're right and it would be better to create an Authorizer dedicated to this role.

As I explained at https://github.com/eclipse/leshan/issues/1359#issuecomment-1326403251, your solution doesn't fit the use case where someone want to store data extracted by Authorizer in the BootstrapSession (like for registration at https://github.com/eclipse/leshan/issues/1293) But maybe we can do that in a second time. So let's try with your modification in a first time. (You can create a PR)

sbernard31 commented 1 year ago

About :

As I explained at https://github.com/eclipse/leshan/issues/1359#issuecomment-1326403251, your solution doesn't fit the use case where someone want to store data extracted by Authorizer in the BootstrapSession (like for registration at https://github.com/eclipse/leshan/issues/1293)

I'm working on implementing #1293 at #1361.

sbernard31 commented 1 year ago

@Warmek (or anybody who could be interested by this at Orange), PR #1361 is done so you can review it if wanted.

sbernard31 commented 1 year ago

PR #1361 is now integrated in master

Before to close this issue, I will prepare a PR to adapt BootstrapAuthorizer like in #1361.

sbernard31 commented 1 year ago

Before to close this issue, I will prepare a PR to adapt BootstrapAuthorizer like in https://github.com/eclipse/leshan/pull/1361.

PR is available at #1366.

@Warmek (or anybody from Orange) let me know if you want to review it ?

sbernard31 commented 1 year ago

@Warmek I guess nobody will review it, or ?

sbernard31 commented 1 year ago

with #1361 and #1366 integrated in master, I think we can close this now. (feel free to reopen if I was wrong)

This should be available in 2.0.0-M10.

@Warmek thx for contribution :pray: