Skip to content

Conversation

@wfurt
Copy link
Member

@wfurt wfurt commented Aug 9, 2022

This will use 2.1 bits published by MsQuic.

@wfurt wfurt requested review from MattGal, mthalman and rzikm August 9, 2022 22:57
@wfurt wfurt self-assigned this Aug 9, 2022
@ghost ghost added the area-dockerfiles label Aug 9, 2022
Comment on lines +46 to +52
COPY microsoft.asc /tmp
RUN apt-key add /tmp/microsoft.asc \
&& rm /tmp/microsoft.asc \
&& apt-add-repository https://packages.microsoft.com/debian/10/prod \
&& apt-get update \
&& apt-get install -y libmsquic \
&& rm -rf /var/lib/apt/lists/*
Copy link
Member

Choose a reason for hiding this comment

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

Instead of needing to store the key locally, wouldn't it be better to follow the pattern that is documented here.

Suggested change
COPY microsoft.asc /tmp
RUN apt-key add /tmp/microsoft.asc \
&& rm /tmp/microsoft.asc \
&& apt-add-repository https://packages.microsoft.com/debian/10/prod \
&& apt-get update \
&& apt-get install -y libmsquic \
&& rm -rf /var/lib/apt/lists/*
RUN curl https://packages.microsoft.com/config/debian/10/packages-microsoft-prod.deb -o packages-microsoft-prod.deb \
&& dpkg -i packages-microsoft-prod.deb \
&& rm packages-microsoft-prod.deb \
&& apt-get update \
&& apt-get install -y libmsquic \
&& rm -rf /var/lib/apt/lists/*

Copy link
Member Author

Choose a reason for hiding this comment

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

This problem with that is that it is vulnerable to chain attacks since there is no verification if the downloaded key is the correct one. To do that correctly, we would need to verify key fingerprint before using as trusted. That can be done but it takes extra steps. I can do that if you really want to avoid establishing trust via local key file @mthalman

My preference would be to make singe file for all containers but I did not figure out how to do that.
Since this is ASCII, we can possibly create it via echo/printf as well.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, this is fine. But if there's a vulnerable aspect to the official installation instructions to customers, we should probably get that addressed, yes?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, there were also some errors for some distributions and we should perhaps update notes to include current/latest OS versions as well. I will take a look.

@wfurt wfurt merged commit bf1f2be into dotnet:main Aug 10, 2022
@wfurt wfurt deleted the msquicDeb branch August 10, 2022 20:09
@wfurt wfurt mentioned this pull request Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants