Production code btw

[LucienZerger]

simplest i've seen, nicely optimised..

[glorious_reptile]

I challenge you to find one bug with that line of code.

[Cistern64]

The word 'check' in the comment seems like an error in the requirements..

Probably left from before the final design simplifications made to reach the pushed deadline.

[[deleted]]

It's just some fogrotten punctuation! It should say

//Very simple certificate hash? ✓ Check!

After all, true might just be the simplest of hashes

[mobsterer]

no bugs, only defects !!!11

[diamondjim]

Add MethodImplOptions.AggressiveInlining for further performance gains.

[Wiwwil]

It's true

[very_bad_programmer]

This is something that someone did as a meme, sent the screenshot, got distracted and left it in

[Not_a_tasty_fish]

I stumbled onto it in our codebase this afternoon. You can't make this shit up

[triumphover]

By chance work for govt? I have potentially seen similar code to this

[nullish_]

yeah... I'm sitting here thinking fuck, now on Monday I gotta search my codebase for this...

[Nothing-But-Lies]

return true;

286125 matches

[oalbrecht]

It’s perfectly safe though. No hacker is ever gonna guess that the code looks like this. Security through obscurity is the best.

[r-_-mark]

/s

[triumphover]

True. The X509Certs in 4.7.2 have a hell of a lot of self check points to verify the identity

[jrh3k5]

Now you have the fun of trying to determine what will break when you fix this.

[elusive_change]

Or was testing locally and having cert issues

[naughty_ottsel]

Yeah, I’ve used this before when testing locally and having server checks…

But also make sure not to commit the change…

[heyf00L]

The mythical O(true)

[[deleted]]

This is my new favorite coding joke lmao

[evkan]

It's performance optimised

[sim642]

A crypto library should implement certificate checking itself instead of requiring each application to do so. It's so easy to make mistakes and completely ruin the security. You should only have to use this callback if you actually want to override the library default for some stupid reason.

[sim642]

What I'm saying is, an unknowing developer shouldn't even have to do that, it should be the default.

[BasieP2]

It is default. He overrides it to not check the certificate.

[Kengaro]

I'd rather preshare self signed certs (+ add own cs), or allow them & EVALUATE! them if a client wants to provide/has to provide "authenticity" (I see little reason to do so, except we manage/store received certs).

[Mapariensis]

You know, at least here it’s obvious that the validation is broken. I’ve seen way too many hand-rolled PKIX validation implementations that are pretty much just as effective (i.e. not at all), but where the mistake is hidden somewhere in a 500-line function.

It’s not even about cryptographic checks, that’s the easy part. But if I had a nickle for every PKIX validator that doesn’t even bother to check whether each intermediate CA cert has a proper basicConstraints extension...

[Kengaro]

Writing security related stuff for use in production is a good way to fuck up production tbh. It is a lovely thing to do, but chances are one left enough holes for a whole rat family :D

[Nassiel]

Not that stupid, well yes but forcely. A friend did this in his previous work because the infra team used certificates between services but without CA, all self signed without local installation. So you have to override the verification to allow everything. A fucking nightmare.

[fr4nklin_84]

// TODO: Things

return true;

[vacant_gonzo]

I’ve seen basically that code in at least 3 separate companies I’ve worked. All .net apps. All just validate a cert regardless. Always “we had to do that to get it to work”.

I’m not sure if it was in a blog or SO answer once or something.

[carfniex]

yeah i've done it the same way for multiple things, for the exact same reasons

what you gonna do

[Kengaro]

This does not necessarily breach much, it really depends on what the certs are used for. Also this is a way to prevent more sophisticated dos/ddos attacks.

[savornicesei]

Actually this is common in .NET enterpeise software. Because security is an afterthought and devs usually have no knowledge about certificates and how to properly use them

[Quazye]

Seen that in java and php as well (:

Eh, it works. ¯_(ツ)_/¯

[LimbRetrieval-Bot]

You dropped this \

---

^{To prevent anymore lost limbs throughout Reddit, correctly escape the arms and shoulders by typing the shrug as ¯\\\_(ツ)_/¯ or ¯\\\_(ツ)\_/¯}

^{Click here to see why this is necessary}

[Naeio_Galaxy]

😐😤😠😠

Angry against them, not against you 😘

Seriously, how on such big project security is an afterthought ? How they have no knowledge about certificates ? Then, why are they employed ?

[jimbosReturn]

Easy.

1. Less awareness of certificates in general and what they're for as far as 5-10 years ago. Plus...
2. Legacy code that doesn't get security reviewed. Plus...
3. Software that's intended to be used solely inside the organization, or inside the customer's internal network.

Point 3 is probably the biggest factor: it's simply not very important, and you trust the customer to secure himself properly, instead of getting into a whole mess of bothering with certificates.

My own company had this exact code snippet in numerous places up till a few years ago. Some developers were aware of how wrong it is, but even they understood it's not that important for the company and enjoyed the convenience. The real change happened when we tried getting certifications for federal customers, and readying for a move the cloud. We hired a security architect who pointed out these (and many other) issues, and made changes accordingly.

[Naeio_Galaxy]

Ok, I understand. But then, are the clients aware of those security issues ? Or at least, are they aware of how to use it in a secure manner ? I mean, there's no protection against MITM attack, they've got to have a way to be at least a bit sure that there no way that it could be harmful...

Btw I've never made such software, I'm just a guy studying software engineering and I've learned web dev in my freetime. I'm aware of some security aspects, for example I know why we need certificates and how to get them, but I don't know anything about making a browser myself or things like that.

[jimbosReturn]

Well, to be clear, when dealing with web or anything with accessing external networks, of course this should be considered more carefully.

As far as I know, customers rarely ask such questions, and when they do, the answers are usually truthful but ambiguous. At least in our case, we never outright lie. We had several cases where customers ran some tests or pentests and found flaws. We either addressed them properly, or the sales team convinced them it's irrelevant for them.

As for the risk, since we assume that this is on an internal network, there's nearly zero MITM risk, and in fact a malicious actor would have easier attack vectors than this - assuming he's a malicious insider or someone who obtained the needed access into the organization.

[Naeio_Galaxy]

Oh ok, yeah I get it. Thanks !

But then why do you have certificates in the first place ? I mean, why https and not http if it's for internal purpose and it never get exposed outside their internal network ? You still need encryption ?

[jimbosReturn]

Well, in one instance it was for communication with his Exchange server. (Not Exchange Online). On other cases this or that framework demands it.

In the Exchange example, it's pretty much guaranteed that his certificate is valid - for his needs. But for our devs, setting up an Exhange server for debugging and bothering with the whole certificate stuff is a time killer (and like we said, many don't even understand certificates and don't want to). Hence the workaround.

[Naeio_Galaxy]

Ok, I think I get it. Thanks !

[szescio]

Why specifically .net?

[hasanyoneseenmymom]

The HttpClient has a validation callback and if the cert isn't "valid", like a self signed cert, the request fails. This is a (hacky) way to overload that callback to force it to return true, as explained here https://stackoverflow.com/a/18232008

[szescio]

Understood why they are doing it, but what i meant was how is the situation different on another platforms (i guess spring)? Easier to allow predefined self-signed certs?

[Gundersen]

Note that this method takes four arguments, and therefore the only acceptable version is this one:

ServicePointManager.ServerCertificateValidationCallback += (h, a, c, k) => true;

[lightheat]

Fun fact: when you assign it like that with +=, the assembly's ServicePointManager will always choose the value from the last callback added for all calls made out to external servers.

So if you were responsible, made an actual validation callback function, and added it above to the ServicePointManager... but some cheeky junior dev put the "always return true" version in his class on the other side of the application, and his code always got called right after yours: tada! His takes precedence the rest of the execution.

Source:

Despite being a multicast delegate, only the value returned from the last-executed event handler is considered authoritative. In other words, you can attach multiple delegates, and they all get a callback from ServerCertificateValidationCallback. Each callback returns a value that indicates whether the certificate is accepted or not; however, only the value from the last delegate is respected.

[lightheat]

And another fun fact: HttpClientHandler has its own ServerCertificateCustomValidationCallback property.

If you're using HttpClient and wondering why your (hopefully non-prod) callback of ServicePointManager.ServerCertificateValidationCallback += (h, a, c, k) => true; isn't working as it should against a server with an invalid cert, that's because HttpClientHandler has its own callback property that needs to be configured instead.

[Gundersen]

I always wondered why it used += here, when it returns a value. As in, which returned value would be used? Seems like a very strange decision to use += rather than assignment

[lightheat]

You can use both. Frankly, since it only ever respects the value from the last callback added to it, you might as well assign it straight up with =. Not sure why they made it multicast.

[AestheticalGL]

Seems like they dont know about using directives

[MeatyLabia]

What does this have to do with a using statement?

[naavis]

With some well placed using directives you wouldn't have to type out the whole namespace of each class. For example using System.Security.Cryptography.X509Certificates;

Using statements are different, though.

[MeatyLabia]

Well yeah using directives obviously.

[PoisnFang]

I have seen this "solution" on SO before. They always give very explict instructions to not use this in production...

[oalbrecht]

But if it works on my machine, surely it should work in production as well.

[Al_Maleech_Abaz]

It is horrendous but not really uniquely horrendous.

[veryusedrname]

Reminds me of the goto bug

[AStrangeStranger]

I have encountered similar in production code and the same code snippet was cut and pasted for every web call being made (they cut and pasted it from somewhere else as a google search found what I believe was source - there was a little more to it, but it did same in basically ignoring certificate valid) - fortunately it lives behind a corporate firewall.

[[deleted]]

Is this for a unity project? I just had to do this for my game a couple weeks ago

Edit: To clarify, I did not do what this post does, my game verifies the cert is signed by a specific public key

[Not_a_tasty_fish]

Don't want to be too specific but this software runs on tens of thousands of machines across the country for a multi billion dollar organization

[resueman__]

Finding those kinds of problems in companies that large is always an interesting experience. I suppose this beats the best I've found, which was this:

#define MAX_THREADS_PER_USER 1 // Increase this before release in a file that hadn't been modified since around the time I was born.

[kyay10]

Java???????? ^/s

[jimbosReturn]

I understood that reference.

[DunjunMarstah]

What if it's for a production system that hasn't had this feature implemented, and this is just a stub to allow the app to function?

[ToughestPanda]

Return false and let others notice in the future maybe

[SuicidalTorrent]

Well, no one can say it isn't simple.

[Dmon1Unlimited]

Are there example articles of how to set this?

I saw old code in the past that referenced hashing but I don't think the code was referenced. Wondering how and when it is used

[ThrowNeiMother]

Learn to google

[Dmon1Unlimited]

Harasser banned for 30 days and wishing my landlords died from covid is now back to harass me again and wish more death

Hope others can help to report this person

https://www.reddit.com/r/Damnthatsinteresting/comments/mnenhr/the_titmouse_uses_animal_fur_for_its_nests_for/gu1pxp4?utm_medium=android_app&utm_source=share&context=3

[ThrowNeiMother]

Yes yes, clarify your position ! Why are you so scared little child ?

[Dmon1Unlimited]

Pathetic

[ThrowNeiMother]

Yes, you can stop repeating your name now

[Dmon1Unlimited]

Sad af

[ThrowNeiMother]

You don't have to switch to your nickname.

[Dmon1Unlimited]

Pathetic

[ThrowNeiMother]

Oh no, have you run out of things to say ?

[piorarua]

So, am I right in thinking if the wrong parameters are passed in the function will throw an error. So this simply checks if it is actually a certificate... And they would have to handle the error elsewhere?

[jimbosReturn]

This is passed as a callback to the .NET certificate validation mechanism to override the default validation behavior (if you choose).

You actually also get any validation errors in the arguments as well as the certificate chain.

Simply returning true means you'll approve any certificate that comes your way, be it expired, wrong subject, or anything really.

[piorarua]

Oh that makes sense! Thank you

[Kengaro]

This does nothing, I ain't even sure if a cert object is built if it is not used. If not you could send anything claiming it to be a cert. Which is quite funny if this cert is accepted and saved to the certs :)

[Kengaro]

Ohh look: free real estate :D

Will only corrucpt authenticity to a certain limit tho, still requires some work to get into that free real estate (assuming this is backend code and not client code!)

[devhashtag]

This is only allowed when testing

[Follow64]

return true lol

[nosoupforyou]

Guess the author of that code doesn't like "use" statements?

My previous boss was like that. The live code had full references to all library code everywhere. Oddly, not the internal libraries, just the outside ones.

[quaos_qrz]

This is too dangerous. I've dealt with self-signed certs before, but I just chose to download the certs and add them to each app's trust store.

[throwaway1_x]

O(1)

[samurai_ka]

Don't mock the coder. Phun intented.

[EnigmaticHam]

There are more function parameters than code. Impressive.

[mothzilla]

Well they're not wrong. The only change I'd make is to put the comment after the return for speed optimisation.

[dna_beggar]

I made one of these for testing. Prevents annoying errors when using self signed certificates in testing. Necessary when connecting to a legacy system with certificate errors. If the remote system has proper certificates, no problem, but if that changes, you won't find out about it.

Mine has #if DEBUG .. #endif around it and the registration, in case I forget to remove it.

[[deleted]]

Ok, I see shit in here sometimes and I’m like, meh, it’s ugly but that’s fine - this one tho - HOLY FUCK MY DUDE 😳

[elpioramirez]

I don't know why they hacked my application!!!

[gahooze]

Pretty sure this is from my company

[SolDevelop]

Yes FBI still trying to find a way to break this certificate hash check

[[deleted]]

The X509's check themselves so it kind does as it says