Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(api): remove insecure APIs #5096

Merged

Conversation

halibobo1205
Copy link
Contributor

What does this PR do?
remove some APIs for security concerns
Why are these changes required?
possible exposure of sensitive information
This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

@halibobo1205 halibobo1205 merged commit 8a45628 into tronprotocol:release_v4.7.2 Apr 3, 2023
@Crypto2
Copy link

Crypto2 commented Apr 17, 2023

Why have these been removed? They are widely used and going to break a ton of integrations. It's the job of the network/firewall configuration to prevent any leaking not the coin node.

@StevenProhashing
Copy link

Why have these been removed? They are widely used and going to break a ton of integrations. It's the job of the network/firewall configuration to prevent any leaking not the coin node.

Agreed. We upgraded to the latest Tron version until we discovered these changes break our integrations👎. We use the API on our local network.

@smartynovych
Copy link

What alternative options do developers have after the removal of certain APIs?

@Crypto2
Copy link

Crypto2 commented Jul 11, 2023

Using one of the SDKs or rolling your own, we used them because back in the day there was no SDK out. To get the key from your password it is just a sha256 hash of your password I believe.

@FrankWooster
Copy link

FrankWooster commented Jul 11, 2023

Are you familiar with the concept of backward compatibility? Apparently not. if you use java, read about the @deprecated annotation.
Great approach.
👎

@ExOCeo
Copy link

ExOCeo commented Jul 12, 2023

@Crypto2, it seems all the removed are about address and PK. There used to be some warnings in TRON docs for the APIs, it's not safe to get such transmitted through the network indeed. It is impractical for normal developers to build a node, configure a firewall and DMZ to make a safe box. So now, we have to use the SDK.

@Crypto2
Copy link

Crypto2 commented Jul 12, 2023

That takes all of 15 minutes and a DMZ wouldn't be involved in the first place and they would have to install and configure the node either way. Also the integrations most people are talking about probably were built years ago when Tron came out and that's all there was to use unless you were using one of the like 1-2 languages with an SDK.

@ExOCeo
Copy link

ExOCeo commented Jul 12, 2023

@Crypto2, it is not just a matter of time, check how many users are rolling with third-party providers, there are many reasons to not maintain a node while acquiring the service to access TRON. The integrations are impacted, and the notice to adapt to it is rather short. TRON should make a much earlier advance notice before eliminating those APIs.

@Crypto2
Copy link

Crypto2 commented Jul 12, 2023

Any developer/user who can't be trusted to do that simple task can't be trusted to implement any part of their system securely, including private key management. There would be 100% no reason to remove existing working APIs but only to mark them deprecated and put a note they they shouldn't be used for new development and/or if they are using 3rd party node hosting.

@FrankWooster
Copy link

FrankWooster commented Jul 12, 2023

That takes all of 15 minutes and a DMZ wouldn't be involved in the first place and they would have to install and configure the node either way. Also the integrations most people are talking about probably were built years ago when Tron came out and that's all there was to use unless you were using one of the like 1-2 languages with an SDK.

The TRON team needs to work seriously on the key storage and security architecture. And not delete methods. DMZ closes these problems with the proper alignment of the DMZ.
DMZ, isolated VLANs, as well as IPS\IDS tools strictly control traffic to the node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants