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

Uploading certificates causes failure in TrustManager/KeyManager because of bad MIME-type #22626

Closed
flamber opened this issue May 11, 2022 · 2 comments · Fixed by #23176
Closed
Assignees
Labels
Administration/Databases .Backend Database/Postgres Drainable:Yes Candidate for ramping up in a product area Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Type:Bug Product defects
Milestone

Comments

@flamber
Copy link
Contributor

flamber commented May 11, 2022

Describe the bug
Followup on #20319, which might have set the certificate validation too strict.

This is getting close to a P1, since there has been multiple fixes across multiple versions.

To Reproduce

  1. Setup a connection to Postgres, which requires verify-ca or verify-full
  2. Upload Root certificate and Save - fails with:
    Loading the SSL root certificate /tmp/metabase-secret_18327666861552528118.tmp into a TrustManager failed.
  3. Looking in the browser request, my browser sends the following ...,"ssl-root-cert-value":"data:application/pkix-cert;base64,...
  4. If I manually change the browser PUT request data to ...,"ssl-root-cert-value":"data:application/x-x509-ca-cert;base64,... and resend, then it save successfully.

Expected behavior

  1. Handle any MIME-type
  2. Provide a better error / better validation to help understanding what is going on
  3. Make sure this also applies to client certificates (KeyManager)

Information about your Metabase Installation:
Tested 0.42.4, 0.43.0

@flamber flamber added Type:Bug Product defects Priority:P2 Average run of the mill bug Database/Postgres .Backend .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Administration/Databases labels May 11, 2022
@adriancb
Copy link

adriancb commented May 11, 2022

We're seeing:

{"errors":{"dbname":"Loading the SSL certificate /tmp/metabase-secret_9584458119932934733.tmp into a KeyManager failed."}}

when using verify-full and the following parameters:

ssl-client-cert-value: "data:application/x-x509-ca-cert;base64...
ssl-key-value: "data:application/octet-stream;base64...
ssl-root-cert-value: "data:application/x-x509-ca-cert;base64...

which makes me think that the same mime-type issue exists for the client certificate.

We're seeing:

{"errors":{"dbname":"Connection requires a valid client certificate."}}

when using verify-ca and the following parameters:

ssl-root-cert-value: "data:application/x-x509-ca-cert;base64...

which makes me believe the root cert is being saved correctly.

@paoliniluis
Copy link
Contributor

paoliniluis commented May 16, 2022

I built a repro environment that uses Postgres 14 but only accepts certs to authenticate (via pg_hba.conf):
Steps:

  1. install docker and docker compose (last one is included in the latest versions of docker)
  2. save this docker compose file in a folder and do "docker-compose up"
version: '3.9'
services:
  metabase:
    image: metabase/metabase:v0.43.0
    container_name: metabase
    hostname: metabase
    volumes: 
    - /dev/urandom:/dev/random:ro
    ports:
      - 3031:3000
    environment:
      - "MB_DB_TYPE=postgres"
      - "MB_DB_DBNAME=metabase"
      - "MB_DB_PORT=5432"
      - "MB_DB_USER=metabase"
      - "MB_DB_PASS=mysecretpassword"
      - "MB_DB_HOST=postgres-app-db"
    cpus: 1
    mem_limit: 2048m
    networks: 
      - metanet1
  postgres-app-db:
    image: postgres:14.2-alpine
    container_name: postgres-app-db
    hostname: postgres-app-db
    ports:
      - 5432:5432
    environment:
      - "POSTGRES_USER=metabase"
      - "POSTGRES_DB=metabase"
      - "POSTGRES_PASSWORD=mysecretpassword"
    volumes:
      - $PWD/postgres_origin:/var/lib/postgresql/data
    networks: 
      - metanet1
    cpus: 1
    mem_limit: 128m
  postgres_with_certs:
    image: paoliniluis/qa-databases:postgres-sample-14-certs
    container_name: postgres_with_certs
    ports:
      - 5433:5432
    hostname: postgres_with_certs
    networks: 
      - metanet1
    cpus: 4
    mem_limit: 128m
    command: -c ssl=on -c ssl_cert_file=/var/lib/postgresql/pgconf/server.crt -c ssl_key_file=/var/lib/postgresql/pgconf/server.key -c ssl_ca_file=/var/lib/postgresql/pgconf/ca.crt -c hba_file=/var/lib/postgresql/pg_hba.conf
networks: 
  metanet1:
    driver: bridge
3) once the stack spin up, do the following to extract the certificates from the sample-data container
docker cp postgres_with_certs:/var/lib/postgresql/certs/ca.crt .
docker cp postgres_with_certs:/var/lib/postgresql/certs/client.crt .
docker cp postgres_with_certs:/var/lib/postgresql/keys/client.key .
  1. now go to localhost:3031, set up Metabase and then go to admin->settings to add a new database. The data you need to use is:
  • display name: postgres_with_certs
  • host: postgres_with_certs
  • port 5432
  • database name: sample
  • username: metabase
  • password: metasample123
  • enable "use a secure connection"
  • ssl mode: verify-full
  • ssl root certificate: uploaded file path (upload ca.crt)
  • authenticate client certificate: uploaded file path (upload client.crt)
  • ssl client key: uploaded file path (upload client.key)

click Save and see the error

@luizarakaki luizarakaki added the Drainable:Yes Candidate for ramping up in a product area label May 26, 2022
@flamber flamber changed the title Uploading certificates causes failure in TrustManager because of bad MIME-type Uploading certificates causes failure in TrustManager/KeyManager because of bad MIME-type May 31, 2022
@metamben metamben self-assigned this Jun 2, 2022
@flamber flamber added Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness and removed Priority:P2 Average run of the mill bug labels Jun 6, 2022
metamben added a commit that referenced this issue Jun 6, 2022
@flamber flamber added this to the 0.43.3 milestone Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Administration/Databases .Backend Database/Postgres Drainable:Yes Candidate for ramping up in a product area Priority:P1 Security holes w/o exploit, crashing, setup/upgrade, login, broken common features, correctness .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Type:Bug Product defects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants