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

Classed TLS configuration #120

Closed
wlandau opened this issue Sep 15, 2023 · 4 comments
Closed

Classed TLS configuration #120

wlandau opened this issue Sep 15, 2023 · 4 comments
Assignees

Comments

@wlandau
Copy link
Owner

wlandau commented Sep 15, 2023

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
Copy link
Owner Author

wlandau commented Sep 15, 2023

@shikokuchuo, I almost have this implemented, except for the issues at shikokuchuo/mirai#76 (reply in thread). 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:

crew/R/crew_client.R

Lines 220 to 229 in 0bd39b1

mirai::daemons(
n = self$workers,
url = url,
dispatcher = TRUE,
seed = NULL,
tls = self$tls$client(),
pass = get_password(),
token = TRUE,
.compute = self$name
)

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

self$tls$worker(name = self$name)

@shikokuchuo
Copy link
Contributor

shikokuchuo commented Sep 15, 2023

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
Copy link
Owner Author

wlandau commented Sep 15, 2023

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
Copy link
Contributor

shikokuchuo commented Sep 15, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants