Uploaded image for project: 'Opencast'
  1. MH-13082

Reopened security vulnerability closed by MH-12840

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed and reviewed
    • Affects Version/s: 3.7, 4.5, 5.2, 6.0
    • Fix Version/s: 3.7, 4.5, 5.2, 6.0
    • Component/s: External API
    • Labels:
      None
    • Severity:
      Security
    • Steps to reproduce:
      Hide
      As Karen Dolan mentioned on MH-13022 my "fix" in MH-13022 actually brought back the security risk that was fixed by MH-12840.

      In MH-13022 I made the 3 parameter LtiLaunchAuthenticationHandler constructor work as it used to before MH-12840. The reason was that the 3 parameter constructor internally called the 4 parameter constructor but only passed its first argument to it discarding its other two arguments. I found that behavior unintuitive. The 3 parameter constructor did the same as the 1 parameter constructor.

      When we upgraded from Opencast 4.3 to 4.4 we had the problem that LTI authentication stopped working. And only after we exhausted many other possible causes did we find out that the list of highly trusted keys we passed as the third parameter now silently got discarded. So the new fourth parameter to the constructor was essentially not optional unlike what the different constructor argument lists might make you believe.

      So instead of making the 3 parameter constructor work like it used to before MH-12840 with the security issue again included (as I did with MH-13022) it would be better to remove it altogether. If there was no longer a 3 parameter constructor people upgrading from e.g. 4.3 to 4.4 would at least get some error if they didn't update their config files.
      Show
      As Karen Dolan mentioned on MH-13022 my "fix" in MH-13022 actually brought back the security risk that was fixed by MH-12840 . In MH-13022 I made the 3 parameter LtiLaunchAuthenticationHandler constructor work as it used to before MH-12840 . The reason was that the 3 parameter constructor internally called the 4 parameter constructor but only passed its first argument to it discarding its other two arguments. I found that behavior unintuitive. The 3 parameter constructor did the same as the 1 parameter constructor. When we upgraded from Opencast 4.3 to 4.4 we had the problem that LTI authentication stopped working. And only after we exhausted many other possible causes did we find out that the list of highly trusted keys we passed as the third parameter now silently got discarded. So the new fourth parameter to the constructor was essentially not optional unlike what the different constructor argument lists might make you believe. So instead of making the 3 parameter constructor work like it used to before MH-12840 with the security issue again included (as I did with MH-13022 ) it would be better to remove it altogether. If there was no longer a 3 parameter constructor people upgrading from e.g. 4.3 to 4.4 would at least get some error if they didn't update their config files.

      TestRail: Results

        Attachments

          Activity

            People

            • Assignee:
              timschroeder Tim Schroeder
              Reporter:
              timschroeder Tim Schroeder
            • Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                TestRail: Cases