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

Add a downloader + verifier for static.rust-lang.org #179

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

emberian
Copy link
Contributor

This is pretty primitive and completely synchronous with no progress notifications.

First step towards #172

This is pretty primitive and completely synchronous with no progress notifications.

foreach (String fingerprint in Sha256Fingerprints)
{
if (fingerprint == received_fingerprint) { return true; }
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a security person by any stretch of the imagination, so this might be dumb, but what does checking for exact cert fingerprint gives us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It gives us the knowledge that the connection isn't being MITM'd by any other certificates that may be trusted by the root CA store - either added by malware, compromised certificate, or just plain malice (which has been documented)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's called certificate pinning, and browsers use it in the form of HSTS.

@vosen
Copy link
Member

vosen commented Aug 23, 2015

I don't enforce any particular style of coding in this project, but please use camelCase for variables and function arguments.
i don't know which gpg4win build did you download, but in "vanilla" package, gpg2.exe and gpgconf.exe don't depend on libassuan-0.dll, giving a modest of saving of 69KB.
And a heads up, I'll merge this after releasing 0.1.1 into the wild, which will happen once I get my identity verified for code signing certificate, so this PR might linger here few days longer than seemingly necessary.

@emberian
Copy link
Contributor Author

@vosen No problem! I'm glad we'll be getting a signing cert :) I'll clean up the style - C# isn't what I've used in any projects except minor "learn the language" programs. I'll poke around for something that doesn't depend on libassuan-0.dll

@emberian
Copy link
Contributor Author

I'm back to uni tomorrow so I might be a bit slower than usual in responding, as well.

@emberian
Copy link
Contributor Author

Also, as detailed a review as you can give in regards to structure etc is useful for me - I don't do much programming in the OO style, and C# is a language I'm very weak in. Any avoidance of useful features or handy library things is not a guided choice, but just ignorance!

@vosen
Copy link
Member

vosen commented Aug 27, 2015

Your style is OK, other than using a Gpg singleton. I know, we have some examples of this style in the codebase already, but it should be avoided. And I've noticed just now that the VS settings in ReinitializeGpgPaths are only read once, at the first use of Gpg. This is wrong, we won't apply any settings updates.

@Boddlnagg
Copy link
Contributor

I guess this is outdated and we should just wrap rustup instead, right? (Sorry for the wasted effort ...)

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.

None yet

3 participants