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

LNC Receiver (w/ microservice) #1763

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

riccardobl
Copy link
Member

@riccardobl riccardobl commented Dec 25, 2024

Description

A LNC receiver using a go microservice as discussed in #1342 .
The microservice proxies lnc rpcs through a rest-like api and handles the connection lifecycle internally, reusing connections when possible, and closing them if they become unused for a while.

This pr also adds the microservice to sndev.

Closes #1141

Checklist

Are your changes backwards compatible? Please answer below:
yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
8

Did you introduce any new environment variables? If so, call them out explicitly here:

  • LNCD_URL : http url to the microservice (eg. http://lncd:7167 )
  • LNCD_CERT : the certificate serialized with xxd -p -c0 (needed only if lncd uses TLS with a self signed certificate)
  • LNCD_AUTH_TOKEN : the auth token used to authenticate with lncd

env variables for the microservice

(we might need to tweak these for production)

Environment Variable Default Value Description
LNCD_TIMEOUT 5m Timeout duration for connections.
LNCD_LIMIT_ACTIVE_CONNECTIONS 210 Maximum number of active connections allowed.
LNCD_STATS_INTERVAL 1m Interval for logging connection pool statistics.
LNCD_DEBUG false Flag to enable or disable debug logging.
LNCD_PORT 7167 Port on which the server listens.
LNCD_HOST 0.0.0.0 Host address on which the server listens.
LNCD_TLS_CERT_PATH "" Path to the TLS certificate file (empty to disable TLS).
LNCD_TLS_KEY_PATH "" Path to the TLS key file (empty to disable TLS).
LNCD_AUTH_TOKEN "" Bearer token required to access the server (empty to disable authentication).
LNCD_DEV_UNSAFE_LOG false Enable or disable logging of sensitive data.
LNCD_HEALTHCHECK_SERVICE_PORT 7168 Additional healthcheck service port.
LNCD_HEALTHCHECK_SERVICE_HOST 127.0.0.1 Additional healthcheck service host.

If LNCD_HEALTHCHECK_SERVICE_PORT and LNCD_HEALTHCHECK_SERVICE_HOST are set, an additional unauthenticated and unencrypted healthcheck endpoint will be listening on the specified port and host.

@riccardobl riccardobl changed the title LNC receiver V2 LNC Receiver (w/ microservice) Dec 26, 2024
@@ -779,6 +784,7 @@ async function upsertWallet (
if (testCreateInvoice) {
try {
await testCreateInvoice(data)
await validate({ data, settings, skipGenerated: false })
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this file are not related to this pr specifically.
See #1769

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this means this PR is based on #1769?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

@@ -63,7 +63,7 @@ export function useWalletConfigurator (wallet) {
throw err
}
} else if (canReceive({ def: wallet.def, config: serverConfig })) {
const transformedConfig = await validateWallet(wallet.def, serverConfig)
const transformedConfig = await validateWallet(wallet.def, serverConfig, { skipGenerated: true })
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this file are not related to this pr specifically.
See #1769

@@ -72,7 +72,7 @@ function composeWalletSchema (walletDef, serverSide, skipGenerated) {

if (clientOnly && serverSide) {
// For server-side validation, accumulate clientOnly fields as vaultEntries
vaultEntrySchemas[optional ? 'optional' : 'required'].push(vaultEntrySchema(name))
vaultEntrySchemas[(optional || generated) ? 'optional' : 'required'].push(vaultEntrySchema(name))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this file are not related to this pr specifically.
See #1769

@riccardobl riccardobl marked this pull request as ready for review December 27, 2024 18:24
@huumn
Copy link
Member

huumn commented Dec 31, 2024

lncd doesn't appear authenticated. Is the expectation that it's run in a VPC or that authentication is handled in a reverse proxy?

@riccardobl
Copy link
Member Author

lncd doesn't appear authenticated. Is the expectation that it's run in a VPC or that authentication is handled in a reverse proxy?

it only needs to be accessible by the backend, so the expectation was to run it in a vpc

@huumn
Copy link
Member

huumn commented Dec 31, 2024

Everything we run in a vpc we also make authenticated, just in case, so we'll want to do that.

@riccardobl
Copy link
Member Author

Everything we run in a vpc we also make authenticated, just in case, so we'll want to do that.

do you have preferences for the authentication method? Does it need tls?

@huumn
Copy link
Member

huumn commented Dec 31, 2024

Either basic or bearer auth is fine. Builtin TLS would be nice so I don't have to run a reverse proxy.

Maybe we can gut lnd's gprc auth/tls? Or maybe that's more work than rolling our own.

@riccardobl
Copy link
Member Author

riccardobl commented Jan 1, 2025

Done, added bearer auth and TLS.
I've updated the env variable list in the pr description, but tl;dr is that i've added these vars for the lncd daemon

LNCD_TLS_CERT_PATH	Path to the TLS certificate file (empty to disable TLS).
LNCD_TLS_KEY_PATH		Path to the TLS key file (empty to disable TLS).
LNCD_AUTH_TOKEN		Bearer token required to access the server (empty to disable authentication).

and these for the sn backend:

LNCD_AUTH_TOKEN     same as LNCD_AUTH_TOKEN for lncd
LNCD_CERT                    optional cert hex string used only if lncd has a self-signed certificate

the health check endpoint is authenticated too:

curl -f --insecure  -H "Authorization: Bearer $LNCD_AUTH_TOKEN" https://localhost:7167/health

not sure if this is desired or not, lmk if you prefer this to be not authenticated.
It just prints the status and how many connections and queued actions there are.

Copy link

gitguardian bot commented Jan 1, 2025

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
- - Generic Private Key 04b959f docker/lncd/certs/key.pem View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@huumn
Copy link
Member

huumn commented Jan 1, 2025

the health check endpoint is authenticated too:

I'd prefer it not be. It'll be a load balancer checking it without tls.

@riccardobl
Copy link
Member Author

the health check endpoint is authenticated too:

I'd prefer it not be. It'll be a load balancer checking it without tls.

Ok, i've added an unauthenticated http- /health endpoint that is configurable in the daemon (v0.3.2) using the env variables:

LNCD_HEALTHCHECK_SERVICE_PORT	
LNCD_HEALTHCHECK_SERVICE_HOST

it defaults to http://127.0.0.1:7168/health

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

Successfully merging this pull request may close these issues.

LNC receives
3 participants