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

vaultwarden ignores 404.html page #2767

Closed
tessus opened this issue Sep 24, 2022 · 11 comments · Fixed by #2768
Closed

vaultwarden ignores 404.html page #2767

tessus opened this issue Sep 24, 2022 · 11 comments · Fixed by #2768
Labels
enhancement New feature or request low priority Won't fix anytime soon, but will accept PR if provided

Comments

@tessus
Copy link
Contributor

tessus commented Sep 24, 2022

Subject of the issue

When I enter a non existing path https://vaultwarden.server.com/blabla, the following is logged:

[2022-09-24 19:37:20.223][_][WARN] Response was `None`.
[2022-09-24 19:37:20.223][_][WARN] No 404 catcher registered. Using Rocket default.

and I see the following:

image

instead of the 404.html page:

image

Deployment environment

Your environment (Generated via diagnostics page)

  • Vaultwarden version: v1.25.2
  • Web-vault version: v2022.6.2
  • Running within Docker: false (Base: Unknown)
  • Environment settings overridden: false
  • Uses a reverse proxy: true
  • IP Header check: true (X-Real-IP)
  • Internet access: true
  • Internet access via a proxy: false
  • DNS Check: true
  • Time Check: true
  • Domain Configuration Check: true
  • HTTPS Check: true
  • Database type: MySQL
  • Database version: 8.0.30
  • Clients used:
  • Reverse proxy and version:
  • Other relevant information:

Config (Generated via diagnostics page)

Show Running Config

Environment settings which are overridden:

{
  "_duo_akey": null,
  "_enable_duo": false,
  "_enable_email_2fa": true,
  "_enable_smtp": true,
  "_enable_yubico": true,
  "_icon_service_csp": "",
  "_icon_service_url": "",
  "_ip_header_enabled": true,
  "admin_ratelimit_max_burst": 3,
  "admin_ratelimit_seconds": 300,
  "admin_token": "***",
  "allowed_iframe_ancestors": "",
  "attachments_folder": "/data/system/vaultwarden/attachments",
  "authenticator_disable_time_drift": false,
  "data_folder": "/data/system/vaultwarden",
  "database_conn_init": "",
  "database_max_conns": 10,
  "database_timeout": 30,
  "database_url": "*****://*:*@*/*",
  "db_connection_retries": 15,
  "disable_2fa_remember": false,
  "disable_admin_token": false,
  "disable_icon_download": false,
  "domain": "*****://*****.********.**",
  "domain_origin": "*****://*****.********.**",
  "domain_path": "",
  "domain_set": true,
  "duo_host": null,
  "duo_ikey": null,
  "duo_skey": null,
  "email_attempts_limit": 3,
  "email_expiration_time": 600,
  "email_token_size": 6,
  "emergency_access_allowed": true,
  "emergency_notification_reminder_schedule": "0 5 * * * *",
  "emergency_request_timeout_schedule": "0 5 * * * *",
  "enable_db_wal": true,
  "extended_logging": true,
  "helo_name": null,
  "hibp_api_key": null,
  "icon_blacklist_non_global_ips": true,
  "icon_blacklist_regex": null,
  "icon_cache_folder": "/data/system/vaultwarden/icon_cache",
  "icon_cache_negttl": 259200,
  "icon_cache_ttl": 2592000,
  "icon_download_timeout": 10,
  "icon_redirect_code": 302,
  "icon_service": "internal",
  "incomplete_2fa_schedule": "30 * * * * *",
  "incomplete_2fa_time_limit": 3,
  "invitation_org_name": "Vaultwarden",
  "invitations_allowed": true,
  "ip_header": "X-Real-IP",
  "job_poll_interval_ms": 30000,
  "log_file": "/var/log/vaultwarden/vaultwarden.log",
  "log_level": "Info",
  "log_timestamp_format": "%Y-%m-%d %H:%M:%S.%3f",
  "login_ratelimit_max_burst": 10,
  "login_ratelimit_seconds": 60,
  "org_attachment_limit": null,
  "org_creation_users": "",
  "password_hints_allowed": true,
  "password_iterations": 100000,
  "reload_templates": false,
  "require_device_email": false,
  "rsa_key_filename": "/data/system/vaultwarden/rsa_key",
  "send_purge_schedule": "0 5 * * * *",
  "sends_allowed": true,
  "sends_folder": "/data/system/vaultwarden/sends",
  "show_password_hint": false,
  "signups_allowed": false,
  "signups_domains_whitelist": "",
  "signups_verify": false,
  "signups_verify_resend_limit": 6,
  "signups_verify_resend_time": 3600,
  "smtp_accept_invalid_certs": false,
  "smtp_accept_invalid_hostnames": false,
  "smtp_auth_mechanism": null,
  "smtp_debug": false,
  "smtp_explicit_tls": null,
  "smtp_from": "***********@********.**",
  "smtp_from_name": "Vaultwarden",
  "smtp_host": "********.**",
  "smtp_password": "***",
  "smtp_port": 465,
  "smtp_security": "force_tls",
  "smtp_ssl": null,
  "smtp_timeout": 15,
  "smtp_username": "*******",
  "templates_folder": "/data/system/vaultwarden/templates",
  "tmp_folder": "/data/system/vaultwarden/tmp",
  "trash_auto_delete_days": null,
  "trash_purge_schedule": "0 5 0 * * *",
  "use_syslog": false,
  "user_attachment_limit": null,
  "web_vault_enabled": true,
  "web_vault_folder": "/usr/share/vaultwarden/web-vault",
  "websocket_address": "0.0.0.0",
  "websocket_enabled": true,
  "websocket_port": 3012,
  "yubico_client_id": null,
  "yubico_secret_key": null,
  "yubico_server": null
}

Steps to reproduce

  • install vaultwarden as a precompiled binary (not docker)
  • enter a non-existing path: https://vaultwarden.server.com/blabla

Expected behaviour

image

Actual behaviour

image

Troubleshooting data

My httpd proxy config:

<VirtualHost aa.bb.cc.dd:443>
    SSLEngine On
    Include /etc/httpd/extra/certinfo-vault.conf

    Protocols h2 http/1.1
    ServerName vaultwarden.server.com

    RewriteEngine on
    RewriteCond %{HTTP:Upgrade} =websocket [NC]
    RewriteRule /notifications/hub(.*) ws://localhost:3012/$1 [P,L]
    ProxyPass / http://localhost:8888/

    ProxyPreserveHost On
    ProxyRequests Off
    RequestHeader set X-Real-IP %{REMOTE_ADDR}s

    Header always set Strict-Transport-Security "max-age=31536000"

    CustomLog "|/usr/local/apache/bin/rotatelogs -ft /var/log/vault.access 50M" standard
    ErrorLog /var/log/vault.error
</VirtualHost>

I use ROCKET_PORT=8888 instead of the default port.

@BlackDex BlackDex added enhancement New feature or request low priority Won't fix anytime soon, but will accept PR if provided labels Sep 24, 2022
@tessus
Copy link
Contributor Author

tessus commented Sep 24, 2022

I also tried to add ErrorDocument 404 /404.html to the VirtualHost stanza, but it is royally ignored.

@stefan0xC
Copy link
Contributor

I also tried to add ErrorDocument 404 /404.html to the VirtualHost stanza, but it is royally ignored.

I think you also need to enable ProxyErrorOverride to tell apache to load the 404 page.

@tessus
Copy link
Contributor Author

tessus commented Sep 25, 2022

Yep, you are correct, @stefan0xC

However, I would have hoped that the web service would redirect it. I haven't had the chance to look into the architecture between the binary and the web UI in more detail.
I only came across this project 2 days ago and I am not familiar with Rust.

@BlackDex
Copy link
Collaborator

I also tried to add ErrorDocument 404 /404.html to the VirtualHost stanza, but it is royally ignored.

I think you also need to enable ProxyErrorOverride to tell apache to load the 404 page.

That option will break the functionality of the API, since it does return error messages when sending 4xx or 5xx. So that's not the right option.

@stefan0xC
Copy link
Contributor

You are right. Sorry, I have not checked for side effects. My pull request should fix the issue without breaking the API but I have not tested it beyond creating a send and deleting it. Is there a way to automatically test the API for regressions?

@BlackDex
Copy link
Collaborator

@stefan0xC currently there isn't.
To test the 404 if that isn't breaking something in your current codebase you could test that with Send and upload a file, since it doesn't have the v2 file upload feature yet, that should generate a 404 and fall back to the old upload function.

Other then clicking through the interface and using other clients there isn't any way for testing. I was thinking about some options, but wasn't able to start on anything yet

@stefan0xC
Copy link
Contributor

stefan0xC commented Sep 25, 2022

Okay, I've added another 404 catcher for /api calls to return the same message as Rockets default json. Not sure if this is the best solution (we will probably have to wait on rwf2/Rocket#453) or if the other routes also need a json result for 404 errors but checking the path as suggested in rwf2/Rocket#694 seemed a bit hackish to me.

Rockets default handler is created by this macro https://github.com/SergioBenitez/Rocket/blob/6778089c129ee15dfd524413f067c9c572226159/core/lib/src/catcher/catcher.rs#L331

@tessus
Copy link
Contributor Author

tessus commented Sep 25, 2022

@BlackDex

That option will break the functionality of the API, since it does return error messages when sending 4xx or 5xx. So that's not the right option.

Hmm, in this case something is wrong. The API should not return http error codes. The API itself should return 200 with a status code in the response. At least this is how API development usually works.
Unless I misundestood your statement.

e.g. so how does my Apache (reverse proxy) return a proper error message when the vaultwarden service is not running? That is for status code 503? The service can't return anything when it is not running.

You can also specify specific error codes:

ProxyErrorOverride On 503

Update:

This is the config that seems to work for me. Please note that without the ProxyErrorOverride, I receive a 404 when trying to access /admin (unless I remove the Location stanza).

virtual host apache config
<VirtualHost aa.bb.cc.dd:443>
    SSLEngine On
    Include /etc/httpd/extra/certinfo-vault.conf

    Protocols h2 http/1.1
    ServerName vaultwarden.server.com

    ProxyPass "/error/" "!"

    RewriteEngine on
    RewriteCond %{HTTP:Upgrade} =websocket [NC]
    RewriteRule /notifications/hub(.*) ws://localhost:3012/$1 [P,L]
    ProxyPass / http://localhost:8888/

    ProxyPreserveHost On
    ProxyRequests Off
    RequestHeader set X-Real-IP %{REMOTE_ADDR}s
    #RequestHeader add X-Forwarded-Proto https

    <Directory "/data/xxxx/error/">
        Require all granted
    </Directory>
    Alias /error/ "/data/xxxx/error/"

    ProxyErrorOverride On 401 404 503
    ErrorDocument 404 /404.html
    ErrorDocument 503 /error/503.html

    Header always set Strict-Transport-Security "max-age=31536000"

    CustomLog "|/usr/local/apache/bin/rotatelogs -ft /var/log/vault.access 50M" standard
    ErrorLog /var/log/vault.error

    <Location "/admin">
        AuthType Basic
        AuthName "Vaultwarden Administration"
        AuthUserFile /xxxx/xxxx

        Require user xxxx
    </Location>
</VirtualHost>

@BlackDex
Copy link
Collaborator

@tessus I'm not sure why you think that is bad. 4xx errors are errors which can be returned by any web application. Normally a 5xx error is returned by a proxy server if it can't reach the endpoint, or the actual application when it has an internal server error, so that is all fine.

Only returning 200 and provide an error message is not really a standard way, those error codes are there for a reason.

@tessus
Copy link
Contributor Author

tessus commented Sep 25, 2022

Only returning 200 and provide an error message is not really a standard way, those error codes are there for a reason.

I think you misunderstood. I am talking about the API.

Let's say an API endpoint returns a list of items as the response. The search term was specified in the request payload. If the search term does not yield any results, the API must never, ever return the status code 404. The API must return status code 200 (because the API call succeeded), with the reponse that nothing was found and a status code (where you can e.g. also use 404).

@BlackDex
Copy link
Collaborator

Only returning 200 and provide an error message is not really a standard way, those error codes are there for a reason.

I think you misunderstood. I am talking about the API.

Let's say an API endpoint returns a list of items as the response. The search term was specified in the request payload. If the search term does not yield any results, the API must never, ever return the status code 404. The API must return status code 200 (because the API call succeeded), with the reponse that nothing was found and a status code (where you can e.g. also use 404).

I'm not talking about 404 specific, there is also 401 for example.
And if you call an API endpoint that doesn't exist, you should receive a 404. So no, it's definitely not only 200.

Also, a developer can decide to return a 404 for a non existing resources with a json body containing the error message, thats all up to the developer. And I'm not saying either is good or bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request low priority Won't fix anytime soon, but will accept PR if provided
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants