search

RITlug / teleirc

Go implementation of a Telegram <=> IRC bridge for use with any IRC channel and Telegram group
https://docs.teleirc.com/
GNU General Public License v3.0
140 stars 45 forks source link

Image url reveals bot's token #115

Closed alcir closed 5 years ago

alcir commented 5 years ago

Imgur is not configured. Sending an image on Telegram, the API URL appears on IRC. Such URL contains the token of the bot, that should be keep private.

'Untitled Image' uploaded by XXXXXX: https://api.telegram.org/file/botXXXXXXXXXXX:XXXXXXXXXXXXXXXXXX/photos/file_1.jpg

Is this the only way to use the Telegram API? There is a way to disable images on IRC, if Imgur is not configured?

Thanks.

xforever1313 commented 5 years ago

My guess is that files are also going to be impacted by this.

I think the best way to do this is add options to disable Photos and Files being sent via URL so we don't expose the bot API token. These config items should probably be set to false by default, and have a comment with a warning about what happens if they are set to true (maybe even go so far as hiding them so its developers only). I could see someone wanting to set this to true if they are testing the bot out, or if they are in a private IRC channel and trust everyone inside of it.

The good news is this should be straight forward to do since all of the TgHandlers take in an enabled boolean. We can just pass in the true/false from the config directly into the constructor.

jwflory commented 5 years ago

@alcir @xforever1313 We suggested handling this issue in two ways.

Option 1

  1. Enabling Imgur support by default (with the generic API key provided by upstream library)
  2. Supplementing docs to recommend getting your own API key to replace the default

This way, the Telegram API token is never exposed by accident.

Option 2

  1. Better documenting behavior of not using Telegram (i.e. API token will be exposed and this is a risk you are comfortable taking)

What do y'all think? @RITlug/teleirc-developers-commit-access

jwflory commented 5 years ago

Discussed in 2019-02-02 RITlug developers' meeting. This issue is targeted for the v1.3 milestone, estimated to release on March 2nd, 2019.

@Tjzabel is looking into this one. If @RITlug/teleirc-developers-commit-access have an opinion on previous options, please weigh in. :slightly_smiling_face: Otherwise, @Tjzabel is researching this more by the next developers' meeting on Saturday, Feb. 9th.

Edit: Personally I am in favor of Option 1. We accidentally were doing this before. I think we can strongly recommend someone to get their own API key instead of the generic default, but I don't think we have to write new code to police users' behavior.

Tjzabel commented 5 years ago

So as an update, I am looking to go with option 1.

This entails enabling Imgur by default, and writing documentation suggesting to get a personal Imgur API key. However, this does not fix the issue of when someone uploads an image as a file. When uploading as a file, the API key is still shown.

In order to fix this, I think we can either:

  1. Check to see if the photo is a file, and if so, don't make it go through to IRC side.
  2. Keep it how it is, only enabling Imgur by default
jwflory commented 5 years ago

When uploading as a file, the API key is still shown.

Ouch, I didn't know that.

I think we should add a configuration option to allow files to be sent, but it should be disabled by default (i.e. files will not be relayed over the bridge). We can provide documentation to warn a user about the risk of exposing the API key by enabling it. What do you think?

Tjzabel commented 5 years ago

@jwflory hmm, so essentially it would work like this(?):

  1. User sends file over Telegram
  2. Teleirc sees message is of file type, checks to see if IRC file sending is enabled
  3. If enabled, send over file IRC side, showing API key
  4. If disabled, display message similar to: <user> shared a file on Telegram with caption: "foo"

Then, write documentation to support this suggestion. Thoughts?

jwflory commented 5 years ago

Discussed in 2019-02-09 RITlug developers' meeting.

Agreed on Option 1 with Imgur enabled by default with docs to explain how to get an Imgur API key. @Tjzabel's PR to include a configurable feature to send an alert that a file was uploaded without exposing the sensitive URL over IRC (e.g. @Tim123 uploaded a file on Telegram with caption: "This is my white paper on why Teleirc is awesome."). The notification is enabled by default, and an admin can allow Telegram API URLs (with bot token) to go over to IRC if configured.

@Tjzabel aims to have this done by next developers' meeting on Saturday, Feb. 16th.

Tjzabel commented 5 years ago

Due to unforeseen circumstances, this issue will not be completed by 16 February.

By our next meeting, I hope to at least further my research into grabbing the document caption.

jwflory commented 5 years ago

@Tjzabel Thanks for the advance heads-up. :+1: We should still be on target if you can get this sometime next week.

Tjzabel commented 5 years ago

@jwflory I should most likely be able to still get this done (at lest the implementation, not documentation) sometime next week. I will re-evaluate my workload over this weekend to see what is realistically feasible.

Tjzabel commented 5 years ago

@alcir this issue has been resolved, and the newest changes have been pushed to master.

If you would like to have this new default behavior before our next milestone release, feel free to run teleirc from the master branch.