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.

Sign up for GitHub

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

Jump to bottom

TLSNotary server wont start without private-key-pem-path defined even when its unused #565

Open
pirate opened this issue Aug 14, 2024 · 4 comments · May be fixed by #589
Open

TLSNotary server wont start without private-key-pem-path defined even when its unused #565

pirate opened this issue Aug 14, 2024 · 4 comments · May be fixed by #589
Assignees
@funkyenough
Labels
good first issue Good for newcomers

Comments

@pirate
Copy link

pirate commented Aug 14, 2024 •
edited
Loading

I just set up a TLSNotary server here: https://tlsnotary.zervice.io/ and wrote a guide for others in the process, but I encountered a minor issue in the process that I think is a bug.

TLSNotary fails to start when any of private-key-pem-path, certificate-pem-path, or whitelist-csv-path are missing from config.yaml, even when the tls and authorization sections are disabled with enabled: false.

According to the (excellent) docs, those files should not be needed when those features are disabled:
image

Error Output

tlsnotary-1  | Location:
tlsnotary-1  |     /usr/src/tlsn/notary/server/src/util.rs:7:21)
tlsnotary-1  | Error: Unexpected(tls: missing field `private-key-pem-path` at line 20 column 3
tlsnotary-1  | Location:
tlsnotary-1  |     /usr/src/tlsn/notary/server/src/util.rs:7:21)
tlsnotary-1  | Error: Unexpected(authorization: missing field `whitelist-csv-path` at line 30 column 3

Steps to reproduce

Full steps to reproduce here: https://docs.monadical.com/s/UrN0j1lpu

docker-compose.yml:

services:
    tlsnotary:
        # https://github.com/tlsnotary/tlsn/pkgs/container/tlsn%2Fnotary-server
        image: ghcr.io/tlsnotary/tlsn/notary-server:latest
        volumes:
          - ./tlsnotary:/root/.notary-server/config:ro

./tlsnotary/config.yaml:

# copied from: https://github.com/tlsnotary/tlsn/blob/main/notary/server/config/config.yaml

server:
  name: "notary-server"
  host: "0.0.0.0"
  port: 80
  html-info: |
    <h1>Notary Server {version}!</h1>
    <ul>
    <li>git commit hash: <a href="https://github.com/tlsnotary/tlsn/commit/{git_commit_hash}">{git_commit_hash}</a></li>
    <li>git commit timestamp: {git_commit_timestamp}</li>
    <li>public key: <pre>{public_key}</pre></li>
    </ul>
    <a href="/healthcheck">health check</a> - <a href="/info">info</a><br/>

notarization:
  max-transcript-size: 20480

tls:
  enabled: false
  # BUG: these are unused when enabled=false, but tlsnotary will fail to start without them:
  # private-key-pem-path: "/letsencrypt/live/$DOMAIN/privkey.pem"
  # certificate-pem-path: "/letsencrypt/live/$DOMAIN/fullchain.pem"

notary-key:
  private-key-pem-path: "/root/.notary-server/config/notary.key"
  public-key-pem-path: "/root/.notary-server/config/notary.pub"

authorization:
  enabled: false
  # BUG: this must still be set even if auth is not enabled or it will fail to start:
  # whitelist-csv-path: "/root/.notary-server/config/whitelist.csv"

logging:
  level: DEBUG

If I create empty files in those locations and uncomment those lines in config.yaml then it starts without issue.
@yuroitaki
Copy link
Member

@pirate thanks for reporting the issue! When this was first implemented, I assumed that notary-server operator will just leave those optional fields unchanged (default to the fixture paths), hence they are not made optional in code. But it seems to me now that it's a better idea to enforce that in code instead — will raise a PR to fix this :)

@sinui0
Copy link
Member

sinui0 commented Aug 14, 2024

it might be worthwhile to pull in a lib for config management, eg https://github.com/mehcode/config-rs

@funkyenough
Copy link

Hi, may I work on this issue? Would love to contribute!

@yuroitaki
Copy link
Member

Hi, may I work on this issue? Would love to contribute!

For sure! Let us know if you have any questions - you can post them here or in our discord!(the latter is probably easier for back and forth discussions)

@yuroitaki yuroitaki added the good first issue Good for newcomers label Sep 5, 2024
@yorozunouchu yorozunouchu linked a pull request Sep 18, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@funkyenough funkyenough

Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make TLS and Authorization Configuration Fields Optional yorozunouchu/tlsn
4 participants
@pirate @funkyenough @yuroitaki @sinui0