wlandau / crew

A distributed worker launcher
https://wlandau.github.io/crew/
Other
129 stars 4 forks source link

Classed TLS configuration #120

Closed wlandau closed 1 year ago

wlandau commented 1 year ago

I think crew needs an R6 class to manage TLS configuration in mirai, given how complicated it is to configure TLS and how the interface might change in the future.

wlandau commented 1 year ago

@shikokuchuo, I almost have this implemented, except for the issues at https://github.com/shikokuchuo/mirai/discussions/76#discussioncomment-7006291. I still seem to have trouble supplying a password for an encrypted key in a way that functions. All the relevant tests I can think of are https://github.com/wlandau/crew/blob/120/tests/tls/test-crew_tls.R, and most are failing.

crew supplies tls and pass to mirai::daemons() here:

https://github.com/wlandau/crew/blob/0bd39b14e90edf6490babf167f11bb3335c1d830/R/crew_client.R#L220-L229

and builds the tls of mirai::daemon() here:

https://github.com/wlandau/crew/blob/0bd39b14e90edf6490babf167f11bb3335c1d830/R/crew_launcher.R#L331

shikokuchuo commented 1 year ago

I think the solution is to expose it as an argument - for user to supply a function - with the default value of NULL.

Unless you want to customise this behaviour, in which case you need to export a function from crew i.e. crew::get_password(). This function then should always return a character string or NULL.

wlandau commented 1 year ago

I was trying to generate this function inside crew automatically, and it's not working. Here is a mirai-only example:

system(
  paste(
    "openssl genpkey -out fd.key -algorithm RSA -outform PEM",
    "-pkeyopt rsa_keygen_bits:2048 -des3 -pass pass:crew"
  )
)
system(
  paste(
    "openssl req -new -key fd.key -out fd.csr",
    "-subj \"/CN=127.0.0.1\" -passin pass:crew"
  )
)
system(
  paste(
    "openssl x509 -req -days 365 -in fd.csr -signkey",
    "fd.key -out fd.crt -passin pass:crew"
  )
)
get_password <- function() {
  "crew"
}
mirai::daemons(
  n = 1L,
  url = "wss://127.0.0.1:0",
  dispatcher = TRUE,
  seed = NULL,
  tls = c(
    paste(readLines("fd.crt"), collapse = "\n"),
    paste(readLines("fd.key"), collapse = "\n")
  ),
  pass = get_password(),
  token = TRUE,
  .compute = "name"
)
shikokuchuo commented 1 year ago

In a nutshell, the function you supply to 'pass' in your daemons() call just gets spliced into a call to dispatcher.

So the above becomes dispatcher(...,pass=get_password().

It is only evaluated on dispatcher. As this is a new process, there is no get_password() function there hence it will error.

But I think in reality all you can do is expose the argument and document that they need to use something like keyring that will persist across sessions.