FIPS mode

[spunkedy]

https://github.com/coryodaniel/bonny/blob/3a8bf8f99280aa1ddf12c1c75bedb8797b2a36c3/lib/bonny/operator/leader_elector.ex#L290

** (stop) {:error, {'hash.c', 125}, 'Low-level call failed'}
    (crypto 5.2) crypto.erl:2200: :crypto.hash(:md5, "Elixir.Devotools.Operator")
    (bonny 1.2.0) lib/bonny/operator/leader_elector.ex:290: Bonny.Operator.LeaderElector.lease_name/1
    (bonny 1.2.0) lib/bonny/operator/leader_elector.ex:309: Bonny.Operator.LeaderElector.lease/3
    (bonny 1.2.0) lib/bonny/operator/leader_elector.ex:198: Bonny.Operator.LeaderElector.acquire_or_renew/2

When FIPS mode is enabled anything md5 is considered insecure.

I can create a MR to check for FIPS mode and then do a different hash algorithm, or could we potentially switch this to a different hash algo?

[mruoss]

well... it is not supposed to be secure... it is just used to create a safe name for a lease. Potentially we could use a different hash algorithm. But we need to make sure the lease name is the same on all replicas (i.e. don't use salt or similar).

[spunkedy]

Yeah. I agree haha. This only on more secured deployments where the underlying infrastructure throws a hard error when trying to use an unsupported algorithm.

I can get PR working against this, want me to just use a new algorithm or want a FIPS mode check ?

On Mon, May 27, 2024 at 2:56 PM Michael Ruoss @.***> wrote:

well... it is not supposed to be secure... it is just used to create a safe name for a lease. Potentially we could use a different hash algorithm. But we need to make sure the lease name is the same on all replicas (i.e. don't use salt or similar).

— Reply to this email directly, view it on GitHub https://github.com/coryodaniel/bonny/issues/275#issuecomment-2133428241, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFIULZN775BI35KEQ2XX6TZEMUPDAVCNFSM6AAAAABILDY6DOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZTGQZDQMRUGE . You are receiving this because you authored the thread.Message ID: @.***>

[mruoss]

That would be great. No, just use a new algorithm.

[spunkedy]

:+1: will get a PR ready later for this

[mruoss]

Sha-1 should do the trick, right?

[spunkedy]

I will test that today. I’m about 40 minutes from my desk and a fips feedback loop

On Tue, May 28, 2024 at 9:19 AM Michael Ruoss @.***> wrote:

Sha-1 should do the trick, right?

— Reply to this email directly, view it on GitHub https://github.com/coryodaniel/bonny/issues/275#issuecomment-2134512165, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFIUL3CCJY3GL4BKE2SN2DZEQVZDAVCNFSM6AAAAABILDY6DOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZUGUYTEMJWGU . You are receiving this because you authored the thread.Message ID: @.***>

[spunkedy]

[mruoss]

I am worried about the length...

[spunkedy]

Maybe just grab the first 16 to keep the same length since this is only for leader election?

 :crypto.hash(:sha256, "asdf") |> String.slice(0..15) |> Base.encode16() |> String.downcase() |> IO.inspect
 :crypto.hash(:sha256, "qwer") |> String.slice(0..15) |> Base.encode16() |> String.downcase() |> IO.inspect

:crypto.hash(:sha256, "asdf") |> String.slice(0..15) |> Base.encode16() |> String.downcase() |> String.length()

[mruoss]

:sha doesn't work?

[spunkedy]

:facepalm: :sha does :sha1 doesn't .... trying to get an integration test going with the :sha

[spunkedy]

debugging the integration test now:

15:40:05.174 [debug] {Operator=REDACTED.Operator} - Starting leadership evaluation
Erlang/OTP 26 [erts-14.2.2] [source] [64-bit] [smp:16:2] [ds:16:2:10] [async-threads:1] [jit:ns]

Interactive Elixir (1.16.1) - press Ctrl+C to exit (type h() ENTER for help)
15:40:05.189 [debug] K8s.Client.Mint.ConnectionRegistry other message received: {:ssl, {:sslsocket, {:gen_tcp, #Port<0.8>, :tls_connection, :undefined}, [#PID<0.2094.0>, #PID<0.2093.0>]}, <<0, 0, 30, 4, 0, 0, 0, 0, 0, 0, 5, 0, 4, 0, 0, 0, 3, 0, 0, 0, 100, 0, 6, 0, 16, 1, 64, 0, 1, 0, 0, 16, 0, 0, 4, 0, 4, 0, 0>>}
15:40:05.239 [debug] {Operator=REDACTED.Operator} - I'm holding the lock. Trying to renew it
15:40:05.288 [debug] {Operator=REDACTED.Operator} - Lock successfully acquired/renewed.
15:40:05.288 [debug] {Operator=REDACTED.Operator} - I am the new leader. Starting the operator.
15:40:05.334 [debug] {Operator=REDACTED.Operator} - Starting leadership evaluation
15:40:05.423 [debug] {Operator=REDACTED.Operator} - I'm holding the lock. Trying to renew it
15:40:05.475 [debug] {Operator=REDACTED.Operator} - Lock successfully acquired/renewed.
15:40:05.475 [debug] {Operator=REDACTED.Operator} - I am the new leader. Starting the operator.
15:40:05.518 [debug] {Operator=REDACTED.Operator} - Starting leadership evaluation
15:40:05.611 [debug] {Operator=REDACTED.Operator} - I'm holding the lock. Trying to renew it
15:40:05.666 [debug] {Operator=REDACTED.Operator} - Lock successfully acquired/renewed.
15:40:05.666 [debug] {Operator=REDACTED.Operator} - I am the new leader. Starting the operator.
15:40:05.710 [debug] {Operator=REDACTED.Operator} - Starting leadership evaluation
15:40:05.800 [debug] {Operator=REDACTED.Operator} - I'm holding the lock. Trying to renew it
15:40:05.852 [debug] {Operator=REDACTED.Operator} - Lock successfully acquired/renewed.
15:40:05.852 [debug] {Operator=REDACTED.Operator} - I am the new leader. Starting the operator.
15:40:05.899 [notice] Application REDACTED exited: shutdown
15:40:05.905 [debug] K8s.Client.Mint.ConnectionRegistry DOWN of process #PID<0.2095.0> received.
iex(REDACTED@REDACTED-56d96b7c59-z7xr4)1> Kernel pid terminated (application_controller) ("{application_terminated,REDACTED,shutdown}")

Crash dump is being written to: erl_crash.dump...done

[spunkedy]

 kubectl get lease -A | grep -i bon
default              default-bonny-7c0953f1db9bc080301cdace36d39749                                   bonny                                                                                       339d
default              default-bonny-816c1063dfe66e38f5dd7915d09df8ee30315d3ef91fc69320f2feac733a4267   bonny                                                                                       38m

I deleted the leases by hand just in case, but I changed it to limit to the first 16

[irvinbonilla]

Hi everybone and @spunkedy, is possible the use of sha256 to be more standard?

Take a note k8s oficially uses this algorith, according to the docs:

https://kubernetes.io/docs/concepts/architecture/leases/

[sleipnir]

Hi everybone and @spunkedy, is possible the use of sha256 to be more standard?

Take a note k8s oficially uses this algorith, according to the docs:

https://kubernetes.io/docs/concepts/architecture/leases/

I agree that using sha256 would be desirable

[mruoss]

Guys, can you elaborate a bit more on your suggestions? What makes sha256 more desirable? Remember: this is not a hash used for security...

[spunkedy]

Guys, can you elaborate a bit more on your suggestions? What makes sha256 more desirable? Remember: this is not a hash used for security...

This is just to make a unique lease, I have 0 preference which algorithm we do since it doesn't have to do with security at all.

[sleipnir]

https://kubernetes.io/docs/concepts/architecture/leases/

"The SHA256 hash used in the lease name is based on the OS hostname as seen by that API server. Each kube-apiserver should be configured to use a hostname that is unique within the cluster. New instances of kube-apiserver that use the same hostname will take over existing Leases using a new holder identity, as opposed to instantiating new Lease objects. You can check the hostname used by kube-apisever by checking the value of the kubernetes.io/hostname label: "

Although it does not explain why the documentation indicates the use of sha256 under some rules. I think it would be desirable to follow the example of the official documentation if possible, even if no other reason or impediment acts. Just for consistency with the documentation.

[sleipnir]

@mruoss I know what we are discussing is more relating to the https://kubernetes.io/docs/concepts/architecture/leases/#custom-workload section that doesn't ask for sha256. But I think if it's possible to use sha256 it would be better. Otherwise I'm not really opposed to any other algorithm, I just gave my 2 cents in the discussion :D

[spunkedy]

I don't mind refactoring this to work off of the hostname for the lease. I don't see it being a problem for statefulsets vs deployments.

Would we prefer that?

[mruoss]

Using the hostname for our lease would defeat the purpose. While in the linked example the lease is used to communicate the API Server's identity, we use the lease for leader election. We explicitly want one single leader over all hosts :)

[spunkedy]

Using the hostname for our lease would defeat the purpose. While in the linked example the lease is used to communicate the API Server's identity, we use the lease for leader election. We explicitly want one single leader over all hosts :)

Agreed.

I think we might be overcomplicating this all. Just updated #276