TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
983 stars 310 forks source link

CUPS update-info returning 403 #1313

Closed mattdean-digicatapult closed 5 years ago

mattdean-digicatapult commented 5 years ago

Summary

Submitted as a result of a slack conversation. I've been trying to configure A Kerlink Wirnet Station using basicstation to connect to a v3 stack. When the station tries to call update-info it errors with 403.

Steps to Reproduce

  1. Add a new gateway
  2. Add a gateway api-key with rights RIGHT_GATEWAY_ALL
  3. Update the gateway setting gateway_server_address, and the attributes cups-credentials, lns-credentials and cups-uri. Not setting these either leads to a different cause of the 403 on the backend or a 500 error.
  4. Configure basicstation on the gateway setting the authorization header in cups-boot.key, the stack uri in cups-boot.uri and the stack cert in cups-boot.trust
  5. Start basicstation

What do you see now?

The backend logs for the request (these repeat as the gateway retries):

INFO Finding gateway... gateway_eui=024B0BFFFE030465 method=POST namespace=web remote_addr=10.15.18.77:13480 request_id=01DKYFYC486KNB1N5QG2KAZT5A url=/update-info
DEBUG Run database query duration=3ms grpc_method=GetIdentifiersForEUI grpc_service=ttn.lorawan.v3.GatewayRegistry namespace=db query=SELECT id, created_at, updated_at, gateway_id, gateway_eui FROM "gateways" WHERE "gateways"."deleted_at" IS NULL AND ((gateway_eui = $1)) ORDER BY "gateways"."id" ASC LIMIT 1 request_id=01DKYFYC49S6RVY3TYXY5R4TSZ rows=1 source=gateway_store.go:117 values=[024B0BFFFE030465]
INFO Finished unary call duration=5.640867ms grpc_method=GetIdentifiersForEUI grpc_service=ttn.lorawan.v3.GatewayRegistry namespace=grpc request_id=01DKYFYC49S6RVY3TYXY5R4TSZ
DEBUG Getting gateway with request credentials gateway_eui=024B0BFFFE030465 gateway_id=gtw-0465 method=POST namespace=web remote_addr=10.15.18.77:13480 request_id=01DKYFYC486KNB1N5QG2KAZT5A url=/update-info
DEBUG Run database query duration=2ms grpc_method=Get grpc_service=ttn.lorawan.v3.GatewayRegistry namespace=db query=SELECT * FROM "api_keys" WHERE ("api_keys"."api_key_id" = $1) ORDER BY "api_keys"."id" ASC LIMIT 1 request_id=01DKYFYC4FMA3WQWDS5S18R5A2 rows=1 source=api_key_store.go:84 values=[AGXRPYACMA7LIPMJQKQ53NQDAC5PNT5HFAJEGXA]
DEBUG Run database query duration=2ms grpc_method=Get grpc_service=ttn.lorawan.v3.GatewayRegistry namespace=db query=SELECT id as uuid, gateway_id as friendly_id FROM "gateways" WHERE (id in ($1)) request_id=01DKYFYC4FMA3WQWDS5S18R5A2 rows=1 source=membership.go:73 values=[1176ce5f-7cca-4a7c-b87d-351f9b83efee]
DEBUG Run database query duration=3ms grpc_method=Get grpc_service=ttn.lorawan.v3.GatewayRegistry namespace=db query=SELECT id, created_at, updated_at, gateway_id, gateway_eui, auto_update, frequency_plan_id, gateway_server_address, update_channel, brand_id, model_id, hardware_version, firmware_version FROM "gateways" WHERE "gateways"."deleted_at" IS NULL AND ((gateway_id = $1) AND (gateway_eui = $2)) ORDER BY "gateways"."id" ASC LIMIT 1 request_id=01DKYFYC4FMA3WQWDS5S18R5A2 rows=1 source=gateway_store.go:117 values=[gtw-0465 024B0BFFFE030465]
DEBUG Run database query duration=3ms grpc_method=Get grpc_service=ttn.lorawan.v3.GatewayRegistry namespace=db query=SELECT * FROM "attributes" WHERE ("entity_id" IN ($1) AND "entity_type" = $2) ORDER BY "attributes"."id" ASC request_id=01DKYFYC4FMA3WQWDS5S18R5A2 rows=3 source=gateway_store.go:117 values=[1176ce5f-7cca-4a7c-b87d-351f9b83efee gateway]
INFO Finished unary call duration=41.106178ms grpc_method=Get grpc_service=ttn.lorawan.v3.GatewayRegistry namespace=grpc request_id=01DKYFYC4FMA3WQWDS5S18R5A2
DEBUG Run database query duration=3ms grpc_method=Update grpc_service=ttn.lorawan.v3.GatewayRegistry namespace=db query=SELECT id, created_at, updated_at, gateway_id, gateway_eui FROM "gateways" WHERE "gateways"."deleted_at" IS NULL AND ((gateway_id = $1) AND (gateway_eui = $2)) ORDER BY "gateways"."id" ASC LIMIT 1 request_id=01DKYFYC6C6FJ5X7DSM8GMXHBM rows=1 source=gateway_store.go:117 values=[gtw-0465 024B0BFFFE030465]
WARN Finished unary call duration=5.596499ms error=error:pkg/auth/rights:insufficient_gateway_rights (insufficient rights for gateway `gtw-0465`) error_correlation_id=1caf5682cad840429e4d0738e3ebfb95 error_name=insufficient_gateway_rights error_namespace=pkg/auth/rights grpc_code=PermissionDenied grpc_method=Update grpc_service=ttn.lorawan.v3.GatewayRegistry namespace=grpc request_id=01DKYFYC6C6FJ5X7DSM8GMXHBM
INFO Request handled duration=74.123482ms error=error:pkg/auth/rights:insufficient_gateway_rights (insufficient rights for gateway `gtw-0465`) method=POST namespace=web remote_addr=10.15.18.77:13480 request_id=01DKYFYC486KNB1N5QG2KAZT5A status=403 url=/update-info

What do you want to see instead?

update-info method should in these circumstances return OK

Environment

v3.1.1 of the stack. basicstation is running on a Kerlink Wirnet Station. I can provide more info if relevant.

Can you do this yourself and submit a Pull Request?

Maybe, but I would need help to work out what actually is going on first.

johanstokking commented 5 years ago

Thanks @mattdean-digicatapult for reporting. Happy to see Basic Station support on Kerlink Wirnet Stations also. Please upvote #1140 as adding documentation for this gateway is on our radar. I'll need to check with Kerlink what their status is with Basic Station, or did you compile this yourself?

Assigning this to @KrishnaIyer to look into this. This may take a while as he won't be back from holidays before mid next week.

johanstokking commented 5 years ago

BTW, what exactly is the format of your Authorization header?

It should be: Authorization: Bearer <API Key>

mattdean-digicatapult commented 5 years ago

@johanstokking, yeah that's what I'm doing. Also If I deliberately mangle the token to make it an invalid apikey I get different logs on the backend, the notable one being:

INFO Finished unary call duration=32.621167ms error=error:pkg/identityserver:invalid_authorization (invalid authorization) error_correlation_id=ee375627239e4b3c831b7e6adc02330b error_name=invalid_authorization error_namespace=pkg/identityserver grpc_code=Unauthenticated grpc_method=Get grpc_service=ttn.lorawan.v3.GatewayRegistry namespace=grpc request_id=01DM061EJM87C4GK7JNZW81QMX

So I think the token is being parsed correctly. What seems to be happening is there's an error whenever the first internal RPC call is made within the UpdateInfo function in update_info.go. Setting all the attributes as described above actually lets the call get all the way to the gateway Update call near the end. Changing the attributes so that a different RPC call takes place (for example CreateAPIKey by un-setting lns-credentials) causes the 403 to be generated at that point instead.

That's as far as I've traced it so far.

mattdean-digicatapult commented 5 years ago

So I've done a bit of research and I think I know what's kind of going on. It seems that the authentication passed in the update-info is never used for any of the internal RPC call. If RegisterUnknown is set then the configured APIKey will be used. If it's not things fall back to the default cluster auth?? I have no idea if this is really correct but the logical thing looking at the code is that the auth from the header should be used.

I've created a PR (#1344) which demonstrates a bit of a hacky solution. Feel free to discard if not useful, but I'm happy to spruce it up to standard if desired

mattdean-digicatapult commented 5 years ago

Not that it's necessarily related, but I should point out this still doesn't solve all my issues in getting basicstation working. update-info now returns 200, but on the basicstation side I now get the error:

[AIO:ERRO] [3] Received more data than expected HTTP content size: 2187 extra bytes

If I find it's an unrelated issue I'll open another ticket, but I thought I'd just mention it.

mattdean-digicatapult commented 5 years ago

Following up from a slack conversation with @adriansmares there seems to be a bit of uncertainty as to how and why auth works the way it does in this case.

There seems to be some agreement that the call that's failing originates from the Update rpc call: https://github.com/TheThingsNetwork/lorawan-stack/blob/9262f5df0927ab437a10ca64fdcee008690444fd/pkg/basicstation/cups/update_info.go#L340

My query was then based around this call: https://github.com/TheThingsNetwork/lorawan-stack/blob/9262f5df0927ab437a10ca64fdcee008690444fd/pkg/basicstation/cups/update_info.go#L182

Following through the logic of this I've deduced the following. The call to getAuth can do the following. If RegisterUnknown has an apikey set in config this will be used to generate authorization as referenced above. If RegisterUnknown is not set the method will use s.component.WithClusterAuth(). In no case does the value from the auth header get used in the getAuth method despite it being passed. This just seems odd to me.

The impression from @adriansmares is that functionally speaking this behaviour may be correct, but to me this does seem odd. For one the getAuth method has a signature that takes a parameter that's never used. Secondly if the s.component.WithClusterAuth() allows for elevated privilege calls then that would seem to allow an apikey with the permissions to get a gateway to create a new apikey with higher privileges.

@htdvisser, do you have any insight into what the desired behaviour is here?

htdvisser commented 5 years ago

Let's try to have all discussions around this issue here on Github (and not on Slack) so that everyone has access to the full picture.

I set up a local test environment with the latest https://github.com/lorabasics/basicstation code and tried to reproduce the errors (which I could).

It looks like we need to make a bunch of changes to the way CUPS works. I'll take this issue over from @KrishnaIyer.

htdvisser commented 5 years ago

This is actually broken, so this is a bug for the current milestone.