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

BeegoMaxMemoryBytes default set 128GB can be used to crash all Harbor instances. #21056

Open
Vad1mo opened this issue Oct 17, 2024 · 4 comments
Assignees
Labels
more-info-needed The issue author need to provide more details and context to the issue

Comments

@Vad1mo
Copy link
Member

Vad1mo commented Oct 17, 2024

Expected behavior and actual behavior:

The PR #19578 increased Beego Max memory to 128 GiB allowing, so uploads up to 128 GiB will be stored in memory.

This can lead to instance crashes,

Usually an uploaded file is stored in the system memory, but if the file size is larger than the memory size limitation in the configuration file, the file will be stored in a temporary file. The default memory size is 64M but can be changed using (bit shift):

Steps to reproduce the problem:

Versions:
Please specify the versions of following systems.

@chlins
Copy link
Member

chlins commented Oct 18, 2024

This value is a default provided by Harbor, suitable for various scenarios and hardware configurations. However, due to different user requirements and environments, it is recommended to customize the two values according to your specific scenarios and settings, rather than relying on the default. It is impractical to find a default value that satisfies everyone's needs, particularly considering security concerns, although 32GB and 128GB may not differ significantly in principle, reducing this value can still compromise the security of some machines with weaker configurations of machine.

@chlins
Copy link
Member

chlins commented Oct 18, 2024

Especially in the current AI scenario, some users will package the model into an OCI artifact and store it in harbor, gather the new feature of k8s OCI volume, and mount the model as a volume in the workload. Due to the specificity of the model, many business models on the application side are often very large, dozens of gigabytes (G) or even hundreds of gigabytes (G). If the default value here is too small, it may cause the convenience of harbor in AI scenarios to be not so strong.

@reasonerjt reasonerjt added the more-info-needed The issue author need to provide more details and context to the issue label Oct 21, 2024
@reasonerjt
Copy link
Contributor

reasonerjt commented Oct 21, 2024

@Vad1mo Please clarify after the comments by @chlins
If there's concern IMO you can update the setting in the deployment of Harbor.

@Vad1mo
Copy link
Member Author

Vad1mo commented Oct 22, 2024

I have a few observations about this setting and the conclusions we are silently assuming the Harbor users must comply with:

  1. The Harbor Installation Prerequisites don't mention the requirement of 128 GiB per Harbor core process. This is what we have in the default config. So an upload can cause memory exhaustion because we expect an upload layer to fit into memory.

  2. There are existing and outlined alternatives, and it is not mentioned in the PR fix: increase beego max memory and upload size #19578 why those alternatives have been ignored: The Beego config documentation clearly says:

    if the file size is larger than the memory size limitation in the configuration file, the file will be stored in a temporary file

    So it is not clear to me why we are taking this risky approach of putting everything into memory. Therefore, risking system instability and opening up an attack vector, instead of a temp file?

  3. Reading the Beego docs, it seems that MaxMemory only applies to enctype="multipart/form-data.
    So at best, this option has no effect to the system. ( I am not sure if layer uploads are done with multipart/form-data)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-info-needed The issue author need to provide more details and context to the issue
Projects
None yet
Development

No branches or pull requests

4 participants