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

Deploy new Oracle with SHA256 key to all validators #115

Closed
5 of 7 tasks
ZaradarBH opened this issue Jan 9, 2023 · 15 comments · Fixed by classic-terra/oracle-feeder#6
Closed
5 of 7 tasks

Deploy new Oracle with SHA256 key to all validators #115

ZaradarBH opened this issue Jan 9, 2023 · 15 comments · Fixed by classic-terra/oracle-feeder#6
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request in scope Work approved by the community security Security concerns

Comments

@ZaradarBH
Copy link
Contributor

ZaradarBH commented Jan 9, 2023

Problem definition
We need to ensure that the Oracle SHA256 patch we implemented to secure the oracle-feeder is pushed to the validators

Feature specification

  • Patch oracle-feeder with feather.js changes from TFL
  • Write up proposal to seek permission to update Oracle feeder with v1.0.0 release.
  • Write upgrade documentation
  • Prepare pre-release
  • Test on mainnet
  • Reach out to validators and notify them of required timing

Additional context
.

Acceptance criteria

  • Validators confirm upgrade
@ZaradarBH
Copy link
Contributor Author

I updated the key itself to SHA256 @ e3258df a while back because using a SHA1 based key would theoretically have enabled us to hack validators mnenomics via Terrad and DNS exfiltration. I also fixed a CVE related to Axios @ 0745b84. But in general we have tons of things we can address when it comes to security, I wrote up this postmortem doc a while back based on SNYK scans I did on the TR repos: https://github.com/terra-rebels/postmortems/blob/main/06272022-SecOps-Review-Postmortem.md

Also in general I would like to re-write apps like the oracle feeder & price server, fcd, etc to .NET core as I dont really consider NodeJS safe due to the many supply chain attack vectors waiting in NPM to make us cry and I have some legacy code from 2016 which we can use to write our own price server feeds talking directly to the CEXs which will also remove a potential MITM vector where people would possibly just buy up the firm providing the current price feeds to our oracle and tamper with the data feed once our oracle becomes "valueable" again, so all in all I think its something we should look into but its not super high priority :)

@ZaradarBH ZaradarBH self-assigned this Jan 9, 2023
@ZaradarBH ZaradarBH added enhancement New feature or request in scope Work approved by the community labels Jan 9, 2023
@ZaradarBH ZaradarBH added this to the Proposal 11168 milestone Jan 9, 2023
@ZaradarBH ZaradarBH transferred this issue from classic-terra/oracle-feeder Feb 13, 2023
@ZaradarBH ZaradarBH added security Security concerns documentation Improvements or additions to documentation labels Feb 15, 2023
@ZaradarBH ZaradarBH modified the milestones: Proposal 11168, Oracle Feeder Update Feb 16, 2023
@ZaradarBH
Copy link
Contributor Author

@ZaradarBH
Copy link
Contributor Author

@ZaradarBH ZaradarBH linked a pull request Feb 20, 2023 that will close this issue
@ZaradarBH
Copy link
Contributor Author

@ZaradarBH
Copy link
Contributor Author

I had to revert the usage of feather.js until the v1.1.0 PR for terra-money is approved. Once its in place we can quickly re-implement the bits we need to align the OracleAPI & LCDClient interfaces

@LuncBurner
Copy link
Collaborator

Oracle feeder is running in mainnet, following a one-week test period with no issues, all other validators will be encouraged to update their Oracle feeder installations.

@nghuyenthevinh2000
Copy link
Member

oracle is running so far on a validator node

we will wait until the end of this month before concluding that it is stable
Screenshot 2023-04-20 at 12 24 00

@LuncBurner
Copy link
Collaborator

There was a bug discovered in the Oracle feeder, which has since been resolved. The new Oracle feeder has been available for download since late April, and has been running on a mainnet validator at that time to test. We are going to continue to run this in production for another week prior to putting up a vote to mandate all validators to use the Sha256 version.

@aeuser999
Copy link

aeuser999 commented May 6, 2023

As I look at the screenshots posted above, the pricing is not correct.

  • it gives a number of feeds at 0.0 (abstain), including uusd, and usdr appears to be the valuation against the USD (right now $1.35), or possibly against Terra v2 (right now $1.21)(although looks more like against USD), rather than against LUNA v1. You will notice that all the prices in the example below are 1 LUNA v1 = Denomination (even though it incorrectly uses the microunit label), so 1 LUNA v1 = $0.000102772990741634 USD for instance.
  • Here is what one of the validators at block 12652310 produced as a vote (for comparison):
0.000151947194713746 uaud
0.000138470777983777 ucad
0.000092237320368746 uchf
0.000709878123662206 ucny
0.000694637814092138 udkk
0.000091634042913093 ueur
0.000081341122344336 ugbp
0.000806324614639766 uhkd
1.507330294624751582 uidr
0.00839495214039573 uinr
0.013849648870287123 ujpy
0.13537453033641671 ukrw
0.361456046297639532 umnt
0.000455756282558924 umyr
0.001085712065013981 unok
0.00568487462238468 uphp
0.000075981716423142 usdr
0.001042301884227613 usek
0.000136300829057259 usgd
0.003445047461816845 uthb
0.003142443181742132 utwd
0.000102772990741634 uusd

Also, has it been adjusted to use more than 1 source for SDR (the other two sources do not include SDR)?

  • SDR is what values everything in the market module (which still includes swaps between "stables")(here and here and here-although with changes between col-3 and col-5). It really does need more than 1 source for SDR to help provide variety in order to prevent pricing feed issues (and particularly one that can affect all the "stables").

Thank you so much for all your work on this :)

@LuncBurner
Copy link
Collaborator

@nghuyenthevinh2000 Would you be able to reply to @aeuser999 re: his post above. I'll reopen the issue back to in-review until we've confirmed that these items are addressed.

@nghuyenthevinh2000
Copy link
Member

As I look at the screenshots posted above, the pricing is not correct.

  • it gives a number of feeds at 0.0 (abstain), including uusd, and usdr appears to be the valuation against the USD (right now $1.35), or possibly against Terra v2 (right now $1.21)(although looks more like against USD), rather than against LUNA v1. You will notice that all the prices in the example below are 1 LUNA v1 = Denomination (even though it incorrectly uses the microunit label), so 1 LUNA v1 = $0.000102772990741634 USD for instance.
  • Here is what one of the validators at block 12652310 produced as a vote (for comparison):
0.000151947194713746 uaud
0.000138470777983777 ucad
0.000092237320368746 uchf
0.000709878123662206 ucny
0.000694637814092138 udkk
0.000091634042913093 ueur
0.000081341122344336 ugbp
0.000806324614639766 uhkd
1.507330294624751582 uidr
0.00839495214039573 uinr
0.013849648870287123 ujpy
0.13537453033641671 ukrw
0.361456046297639532 umnt
0.000455756282558924 umyr
0.001085712065013981 unok
0.00568487462238468 uphp
0.000075981716423142 usdr
0.001042301884227613 usek
0.000136300829057259 usgd
0.003445047461816845 uthb
0.003142443181742132 utwd
0.000102772990741634 uusd

Also, has it been adjusted to use more than 1 source for SDR (the other two sources do not include SDR)?

  • SDR is what values everything in the market module (which still includes swaps between "stables")(here and here and here-although with changes between col-3 and col-5). It really does need more than 1 source for SDR to help provide variety in order to prevent pricing feed issues (and particularly one that can affect all the "stables").

Thank you so much for all your work on this :)

Hi, it has been fixed in this, you can find proof here: #213

@aeuser999
Copy link

Hi @nghuyenthevinh2000 ,

Thank you. That does handle the pricing issue.

What about the point about SDR though:

  • Also, has it been adjusted to use more than 1 source for SDR (the other two sources do not include SDR)?
    • SDR is what values everything in the market module (which still includes swaps between "stables")(here and here and here-although with changes between col-3 and col-5). It really does need more than 1 source for SDR to help provide variety in order to prevent pricing feed issues (and particularly one that can affect all the "stables").

Thank you again so much for all your work on this :)

@LuncBurner
Copy link
Collaborator

@aeuser999 The primary purpose of this Oracle upgrade is one of security. Future enhancements, such as including additional SDR sources may be considered at a future date. The Oracle Feeder is currently intended to function exactly the same as the previous version (current for many validators), with the exception of the introduction of Sha256 encryption.

@LuncBurner
Copy link
Collaborator

Added a new issue for Oracle Feeder SDR which focuses specifically on that component. Anticipate scheduling network-wide upgrade for all validators a couple of weeks after parity upgrade.

@inon-man inon-man removed this from the Oracle Feeder - v1.0.0 milestone Jun 17, 2023
@inon-man
Copy link
Collaborator

Closing outdate issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request in scope Work approved by the community security Security concerns
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants