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

Update note on security policy #4480

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

beutlich
Copy link
Member

Maintenance updates are skipped and were never released actually.

See #3674.

@beutlich beutlich added this to the maintenance milestone Oct 16, 2024
@beutlich beutlich requested review from dietmarw and casella October 16, 2024 15:52
@beutlich beutlich enabled auto-merge (squash) October 16, 2024 15:53
Copy link
Contributor

@casella casella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't get this. Why do we remove the check marks?

@dietmarw
Copy link
Member

@casella because that is what we are actually doing. That file should reflect the actual implemented policy, not what you would have liked it to be. So rather than making false promises, just state the situation.

@casella
Copy link
Contributor

casella commented Oct 18, 2024

@casella because that is what we are actually doing. That file should reflect the actual implemented policy, not what you would have liked it to be. So rather than making false promises, just state the situation.

OK. And, we do we remove 3.2.3? We are still supporting it, as far as I understand.

@HansOlsson
Copy link
Contributor

@casella because that is what we are actually doing. That file should reflect the actual implemented policy, not what you would have liked it to be. So rather than making false promises, just state the situation.

OK. And, we do we remove 3.2.3? We are still supporting it, as far as I understand.

This is about security-notices, and whether we update 3.2.3 if there are any security issues detected (as far as I know we don't update it at all - but on the other hand one could argue that we don't expect any security issues at all, and thus it isn't really relevant - just something that is added as a tick-box by github policy).

@casella
Copy link
Contributor

casella commented Oct 18, 2024

This is about security-notices, and whether we update 3.2.3 if there are any security issues detected (as far as I know we don't update it at all - but on the other hand one could argue that we don't expect any security issues at all, and thus it isn't really relevant - just something that is added as a tick-box by github policy).

I'm not sure I understand.

If someone discovered that, say, the external C code for Tables is vulnerable to cyberattacks, we should of course fix that and release a new 3.2.4 version that is immune to that threat. Ditto for 4.x.y.

Shouldn't that be our security policy?

@beutlich
Copy link
Member Author

Shouldn't that be our security policy?

Yes, this should be the case in an ideal world where we stick to our own project rules and the recommended release schedule. But we failed to do so. Both MSL 3.2.3+build.4 and MSL 4.0.0 are no longer maintained - not only w.r.t. the security updates.

If someone discovered that, say, the external C code for Tables is vulnerable to cyberattacks

Let me give examples:

  • MSL 3.2.3+build.4 uses ModelicaMatIO 1.5.12 which is known to vulnerabilities (with CVEs assigned), i.e., reading crafted MAT files can lead to undefined behaviour or crash of the runtime environment (e.g., a FMU simulator). See also the release notes of the upstream dependency matio.
  • MSL 4.0.0 uses ModelicaMatIO 1.5.17 which is known to vulnerabilities (with CVEs assigned), i.e., reading crafted MAT files can lead to undefined behaviour or crash of the runtime environment (e.g., a FMU simulator). See also the release notes of the upstream dependency matio.

As such, this PR reflects the current state as is.

@beutlich beutlich requested a review from casella October 19, 2024 07:35
@casella
Copy link
Contributor

casella commented Oct 24, 2024

Yes, this should be the case in an ideal world where we stick to our own project rules and the recommended release schedule.

This turned out to be a bit hard given the current regression testing policies. We'll try to improve it after 4.1.0 with a proper CI, that should allow to follow those schedules.

Both MSL 3.2.3+build.4 and MSL 4.0.0 are no longer maintained - not only w.r.t. the security updates.

It depends what you mean by "maintained". We are not deprecating them, though of course we recommend upgrading to 4.1.0 when it will be available. If there were safety critical issues with them, we should of course either fix them or deprecate them explicitly.

If someone discovered that, say, the external C code for Tables is vulnerable to cyberattacks

Let me give examples:

* MSL 3.2.3+build.4 uses ModelicaMatIO 1.5.12 which is known to vulnerabilities (with CVEs assigned), i.e., reading crafted MAT files can lead to undefined behaviour or crash of the runtime environment (e.g., a FMU simulator). See also the [release notes](https://github.com/tbeu/matio/blob/eb937e755692c458112cacb4de9ded3930c138b6/NEWS#L1-L132) of the upstream dependency [matio](https://github.com/tbeu/matio).

Good to hear, I was not aware of this vulnerability. Was that fixed in MatIO 1.5.27?

* MSL 4.0.0 uses ModelicaMatIO 1.5.17 which is known to vulnerabilities (with CVEs assigned), i.e., reading crafted MAT files can lead to undefined behaviour or crash of the runtime environment (e.g., a FMU simulator). See also the [release notes](https://github.com/tbeu/matio/blob/eb937e755692c458112cacb4de9ded3930c138b6/NEWS#L1-L100) of the upstream dependency [matio](https://github.com/tbeu/matio).

Ditto.

As such, this PR reflects the current state as is.

My point is, I would rather change the current state, rather than merging this PR.

I think we should definitely released patched versions of 3.2.3 and 4.0.0 with the updated MatIO. Is that possible? Can we just do it with a new build or does this deserve a minor release, 3.2.4 and 4.0.1?

Are there other know vulnerability issues that we should take care of?

@beutlich
Copy link
Member Author

My point is, I would rather change the current state, rather than merging this PR.

Indeed, this is a sublime intention for the future. Once we are there, we can easily adapt what's supported and maintained. For now the PR as is reflects the present state.

I was not aware of this vulnerability. Was that fixed in MatIO 1.5.27?

Well, at least no more assigned CVEs in v1.5.27, but oss-fuzz reports a few new findings every month.

I think we should definitely released patched versions of 3.2.3 and 4.0.0

I am undecided. Yes, seems odd to leave known vulnerabilities open just because we do not manage to release maintenance versions often. No, it requires another regression test run we still have MSL v4.1.0 in the pipeline.

@casella
Copy link
Contributor

casella commented Oct 25, 2024

Indeed, this is a sublime intention for the future.

Not by any means, I would do that right away. I can't see any good reason not to do it.

The reason why I haven't taken this action previously is simply that I was not aware of this issue, I am sorry but taking responsibility for MAP-Lib takes some time to figure all things out.

I was not aware of this vulnerability. Was that fixed in MatIO 1.5.27?

Well, at least no more assigned CVEs in v1.5.27, but oss-fuzz reports a few new findings every month.

I'm talking about know past issues. Of course we can't fix future ones now 😄

I am undecided. Yes, seems odd to leave known vulnerabilities open just because we do not manage to release maintenance versions often.

Indeed

No, it requires another regression test run we still have MSL v4.1.0 in the pipeline.

Maybe that's not such a big deal. MatIO 1.5.26 is on master since March, I guess all tool vendors keep running the master version all the time to be prepared. If nobody complained, it is quite likely that there are none. The problem with regression testing is when it is likely to actually get regressions, because you need to fix them. Otherwise, it's just a quick check done by automated scripts.

@casella
Copy link
Contributor

casella commented Oct 25, 2024

First and foremost, we should update matio in MSL 4.1.0, given what wrote here I really think it doesn't make sense to wait for MSL 4.2.0 to upgrade 1.5.24 to 1.5.27. Of course we should ship the latest vulnerability fixes.

See #4489

@dietmarw
Copy link
Member

So could we please be honest and recognise the current state and stop making false promises? That's all what this PR is about. You can still update the security message at a later point when we are actually able to ship security updates to other than just the current version of the MSL.

So please merge this and hopefully, we can update this later again.

Maintenance updates are skipped and were never released actually.
@beutlich beutlich force-pushed the update-security-notice branch from 5dd7c71 to f11b5d0 Compare October 27, 2024 11:17
@casella
Copy link
Contributor

casella commented Nov 11, 2024

So could we please be honest and recognise the current state and stop making false promises? That's all what this PR is about. You can still update the security message at a later point when we are actually able to ship security updates to other than just the current version of the MSL.

OK

@beutlich beutlich merged commit 1166ac5 into modelica:master Nov 11, 2024
11 checks passed
@beutlich beutlich deleted the update-security-notice branch November 11, 2024 15:32
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.

4 participants