aws / aws-iot-device-sdk-java-v2

Next generation AWS IoT Client SDK for Java using the AWS Common Runtime
Apache License 2.0
111 stars 75 forks source link

Custom Authorization with tokenKeyName/tokenValue but without authorizerSignature -> RuntimeException #579

Closed schliesserams closed 2 months ago

schliesserams commented 2 months ago

Describe the bug

Please see AwsIotMqttConnectionBuilder.java line 631 and the JavaDoc for the "withCustomAuthorizer" function. The parameter "authorizerSignature" is nullable but in line 633 its checked for "null" if tokenKeyName or tokenValue is set.

But its totally possible and legal to create a custom authorization in the IoT Core without the signature

image

Edit: it´s also checked again in line 717 the AwsIotMqttConnectionBuilder if set manually in the username parameter, so even a "workaround" is currently not possible :(

Expected Behavior

authorizerSignature should be nullable even if tokenValue or tokenKeyName are set

Current Behavior

throws a RuntimeException if authorizerSignature is null but tokenValue or tokenKeyName are set

Reproduction Steps

Based on : https://github.com/aws/aws-iot-device-sdk-java-v2/blob/main/samples/CustomAuthorizerConnect/src/main/java/customauthorizerconnect/CustomAuthorizerConnect.java

AwsIotMqttConnectionBuilder builder = AwsIotMqttConnectionBuilder.newDefaultBuilder(); builder.withConnectionEventCallbacks(callbacks) .withClientId(cmdData.input_clientId) .withEndpoint(cmdData.input_endpoint) .withCleanSession(true) .withProtocolOperationTimeoutMs(60000); builder.withCustomAuthorizer( cmdData.input_customAuthUsername, cmdData.input_customAuthorizerName, null, cmdData.input_customAuthPassword, cmdData.input_customAuthorizerTokenKeyName, cmdData.input_customAuthorizerTokenValue); MqttClientConnection connection = builder.build(); builder.close();

-> throws RuntimeException in https://github.com/aws/aws-iot-device-sdk-java-v2/blob/main/sdk/src/main/java/software/amazon/awssdk/iot/AwsIotMqttConnectionBuilder.java line 634

Possible Solution

don´t check if signature is null (it is checked later in line 655 anyhow)

Additional Information/Context

No response

SDK version used

1.20.5

Environment details (OS name and version, etc.)

OSX Sunoma 14.4.1

bretambrose commented 2 months ago

This is by design. All three parameters work together to send the signed token value to the authorizer. Backwards compatibility prevents us from making changes here (to clean up the interface), so we just validate that "if at least one is set then all must be set."

A better solution would have been to have two separate APIs, one for unsigned (taking three parameters) and one for signed (taking 6), but that ship sailed before we realized that. For example (different repo and language, but illustrating the idea): https://github.com/gneiss-mqtt/gneiss-mqtt/blob/main/gneiss-mqtt-aws/src/lib.rs#L352-L393

Unsigned authorizers should not use any of those three fields.