search

OctopusDeploy / Halibut

| Public | A secure communication stack for .NET using JSON-RPC over SSL.
Other
12 stars 44 forks source link

Treat thumbprints as case-insensitive #606

Closed khellang closed 4 months ago

khellang commented 4 months ago

Background

See #605

Results

Fixes #605

Pre-requisites

sburmanoctopus commented 4 months ago

Thank you for you submission.

We were wondering about the reasons behind doing this change. Could you please elaborate on the motivation?

khellang commented 4 months ago

Could you please elaborate on the motivation?

We encountered the error described in #605, where the server had the thumbprint in uppercase and the client had the thumbprint in lowercase (or vice versa).

sburmanoctopus commented 4 months ago

Hi @khellang,

Thank you for taking the time to prepare this contribution, we appreciate and value your time, unfortunately this time we won't be able to accept this contribution.

Halibut is a very important part of Octopus Deploy, and technically this is a breaking change. While we agree this is a positive change, however the fact that consumers of the library should be able to account for it's shortcoming easily means we aren't able to justify the cost of making this breaking change right now.

khellang commented 4 months ago

Hi @sburmanoctopus ! 👋

Can you explain why this is a breaking change? I'm falling to see how this could break anything 😅

sburmanoctopus commented 4 months ago

Hi @khellang,

To make this change we would also want to update the trust accepting side of halibut to be case insensitive, that way we don't have some checks as case sensitive and other checks as case insensitive. Users of Halibut can supply their own ITrustProvider, which won't know we have switched from case sensitive to case in-sensitive. In a sense that is a breaking change, although in theory it should never be a problem.

Since this is in the security area of Halibut, we would prefer to not make any change unless absolutely necessary. As we want to avoid inadvertently introducing a security vulnerability.

In theory a workaround to the issue your facing is to always upper case the thumbprint to the ServiceEndpoint`.

Since this issue has a workaround, and would require changes in the security areas of Halibut and with our general hesitation to add unnecessary changes to Halibut (we have Halibut services in very remote locations we don't want to break) let's see if holding off on this change is feasible.