Voyz / ibeam

IBeam is an authentication and maintenance tool used for the Interactive Brokers Client Portal Web API Gateway.
Apache License 2.0
581 stars 115 forks source link

Use IB CP API latest binaries for Docker builds #209

Closed teodorkostov closed 3 weeks ago

teodorkostov commented 3 months ago
teodorkostov commented 3 months ago

Should fix #208. Maybe provide a workaround for #207.

Voyz commented 3 months ago

Hey @teodorkostov thanks for submitting this PR.

I've spent a while reviewing the idea behind your proposed changes - one of having the dependency as latest each time we build.

I find good reasons behind doing both pinned versions and latest:

pinned:

latest:

However, I don't have time resources to manually verify the integrity of shipped image each time we build should the Gateway version change. I'd also feel uncomfortable publishing an image which people use in production environments with latest dependencies that haven't been at all tested. I feel that going with latest would only make sense if we could lower the maintenance cost of having latest tested every time we build by introducing a testing suite with CI/CD. This is currently not present in IBeam - would you be interested in introducing it?

An opposite solution would be to keep things as they are. IBKR integrates meaningful changes to the gateway infrequently enough that manually updating it every year or two has been sufficient. Do you feel that this has changed now?

Voyz commented 3 months ago

As for:

Add 172.* addresses to whitelist

172.* addresses include both private and public IP addresses, hence hard-coding them as allowed seems to be a security risk and is not advisable:

teodorkostov commented 3 months ago

Hey, @Voyz.

Now, let's decide on how to deal with the IBKR code versioning.

The problem with IBKR is that you can get only the latest version from them. Logically, at some point ibeam should get the latest IBKR version. The question is when? I would argue always.

The other solution would be to create the build image that holds the IBKR binaries separately. Version that image properly. And use that versioned image in the build stage. It requires more effort. Should that effort be invested now? I would argue no. Go with with the simpler solution now. Do a couple of ibeam versions like that. If it becomes an issue then invest the effort in proper versioning of the IBKR code. And who knows, maybe when the need to properly version the IBKR code they could have solved that problem themselves.

bfoz commented 3 months ago

For what little it's worth, I mostly agree with @teodorkostov. Occasional breaking changes are irritating, but to be expected. I doubt anyone expects you (@Voyz) to be providing mission-critical levels of reliability for free.

As long as the images are properly tagged, we can always roll back to a previous tag when something breaks.

Can we tag the auto-fetched image as "nightly" or "latest", and only tag tested images with a version?

How does this affect people who are running standalone instead of using the docker image?

demircancelebi commented 3 months ago

I like the direction this PR introduces. I think I can help with this:

The other solution would be to create the build image that holds the IBKR binaries separately. Version that image properly.

I already created https://github.com/demircancelebi/ibkr-clientportal-archive which should archive the versions and the checksums daily automatically going forward. If you like it you can fork it / change it to your needs.

Voyz commented 3 months ago

Guys, firstly, I wanna say that I value your input and I'm open to this discussion. Nevertheless, I feel I'm missing a crucial point here, and I'd appreciate if you could try to see this perspective:

  1. Unless I'm missing something - there has been zero updates to the Gateway since April 2023. If what I see is true, then nothing will change directly by introducing this PR right now. I've just downloaded the latest stable release of the Gateway and it is the same exact code IBeam already ships with, zero changes. I went through their changelog, no changes either. We're talking about versioning like if there were any meaningful versions, but there essentially aren't.
  2. What's more, IBKR indicates they're looking to introduce the OAuth 2.0 API which will most likely mean the Client Portal Web API - and as such the Gateway - will become obsolete.
  3. What these two points indicate, is that if there are to be any changes to the Gateway, it's very likely these will be sparse and possibly only few before deprecation - if any.

Why are we trying to do this? Just for sake of being hypothetically up to date? I think I'm not making an unreasonable statement when I'd say that automation only makes sense if it really automates something, otherwise it's another redundant moving part. Downloading, unzipping and pushing the latest Gateway once it changes takes me less than 5 minutes a year. I totally have time for that; I sometimes spend hours trying to debug various Issues users submit, hence 5 minutes is nothing and not worth automating at all. Arguably, having this automation maintained is going to be a similar level of negligible effort, yeah, but if it fixes nothing and changes nothing - why would we introduce it?

Or am I missing something crucial here, and this PR does change something important for somebody, other than not keeping binaries in the source code? Honestly, if there's something that this change will fix and help for anyone here I'm totally happy to go ahead - please help me see your perspective. But at this stage I don't understand why would we do it and why are we investing time into this.

demircancelebi commented 3 months ago

I was thinking about security. In case your account is compromised and the gateway is updated, a malicious actor may introduce unwanted code to the clientportal section. Now the burden to check nothing suspicious going on is on the user, I thought removing that burden and relying on IB would be a good idea.

I also checked the diff and asked Claude to summarize them. I guess these were all intentional: CleanShot 2024-08-20 at 14 03 56@2x

You are right, clientportal is not updated frequently so I'd say it may not be worth the effort if this was not implemented. Since the PR is already here though, I think it'd be better to merge it.

Voyz commented 3 months ago

@demircancelebi thanks for pointing the security out 👍

Correct me if I'm wrong though: if my account gets compromised, a bad actor could introduce malicious code irrespectively of which method we use to support the Docker image with the Gateway files. To my understanding, providing the Gateway automatically from IBKR doesn't mitigate that risk, does it? If it does, how? A bad actor that has compromised my computer or account can introduce suspicious code after the Gateway is pulled from the IBKR servers, or redirect the download request to their own version of the Gateway. If this is a real concern, in either version - manual or automated - the burden of checking whether the files are not suspicious stays with the user.

Even more, one could make an argument that I could be the malicious actor, in which case it would be unadvisable that one uses this image without checking whether the files are not suspicious. I think that could be a fair concern for some, however it would stay true irrespectively of whether my account is compromised or not, and as such irrespectively of which method is used to provide the Gateway files.

Seeing that you've mentioned this though, is there any way we could mitigate this security risk? If there is, I'd suggest we open a new issue and discuss it there.


As for your diff - yes, all the changes are intentional:

Given above, I feel that my previous question remains - I don't understand why we would introduce the automatic pull seeing that it introduces no practical changes or improvements, and to my understanding it doesn't mitigate the security risk of having my account compromised. As always, please let me know if I misunderstood something here 👍

teodorkostov commented 3 months ago

Thanks for the info @Voyz.

  1. What's more, IBKR indicates they're looking to introduce the OAuth 2.0 API which will most likely mean the Client Portal Web API - and as such the Gateway - will become obsolete.

I was not aware that the Client Portal would become obsolete soon. This is certainly a good thing.

The only value this PR has:

  1. Remove the manual step of downloading the CPAPI sources.
  2. Remove the CPAPI sources from the code base.

I am perfectly fine if you would like keep things as is. If you have made up your mind just cancel this PR.

Voyz commented 3 weeks ago

Hey @teodorkostov thanks for your reply. I wanted to give some time for other users to participate and reply to the points I brought up. However, seeing that there hasn't been any, I will indeed proceed and close this PR. I appreciate your initiative and contribution, and I'm sorry I concluded with not wanting to merge it at this time - I don't mean to discourage you from participating in this project. Thanks!