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

Digest authentication not working for REPORT #1244

Open
Yannik opened this issue Feb 5, 2020 · 10 comments
Open

Digest authentication not working for REPORT #1244

Yannik opened this issue Feb 5, 2020 · 10 comments

Comments

@Yannik
Copy link
Contributor

Yannik commented Feb 5, 2020

When doing a REPORT request with digest-authentication like this:

curl -v --request REPORT
--header "Content-Type: application/xml; charset=utf-8"
--digest --user "user@domain.org:password"
--data-binary "@request.txt"
"https://domain.org/sabredav/addressbooks/user@domain.org/testing/"

The first request should return 401 Not Authorized with a WWW-Authenticate header containing the realm, nonce and qop.

Instead, sabredav fails with 500 Internal Server Error and returns the following body:

<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
<s:sabredav-version>4.0.3</s:sabredav-version>
<s:exception>ErrorException</s:exception>
<s:message>XMLReader::XML(): Empty string supplied as input</s:message>
</d:error>

I'm using the PDO backend.

@DeepDiver1975
Copy link
Member

A report request requires a body which you are not sending. This is why you get the error message.

Never the less we should not fail with a 500 ...

@Yannik
Copy link
Contributor Author

Yannik commented Feb 5, 2020

@DeepDiver1975
I'm specifying a body using the --data-binary @"request.txt" option. request.txt contains my request body.

As curl knows that it will need to request the WWW-Authenticate header first for digest auth, it does not send the body on the first request. I think this should be supported.

Also, IMO authorization should be checked first before any body parsing is done...

@DeepDiver1975
Copy link
Member

What is the behavior with basic auth? Can you quickly recheck?

@Yannik
Copy link
Contributor Author

Yannik commented Feb 5, 2020

@DeepDiver1975 Can you clarify: Do you want me to issue a request with basic auth to the server as is, or do you want me to reconfigure the auth-backend from PDO to a basic auth backend and subsequently do a basic auth request?

@Yannik
Copy link
Contributor Author

Yannik commented Feb 5, 2020

@DeepDiver1975 In the meantime, this is the stacktrace of the exception:

#1 /var/www/domain.org/htdocs/sabredav/vendor/sabre/event/lib/WildcardEmitterTrait.php(89): Sabre\DAV\CorePlugin->httpReport(Object(Sabre\HTTP\Request), Object(Sabre\HTTP\Response))
#2 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/Server.php(474): Sabre\DAV\Server->emit('method:REPORT', Array)
#3 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/Server.php(251): Sabre\DAV\Server->invokeMethod(Object(Sabre\HTTP\Request), Object(Sabre\HTTP\Response))
#4 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/Server.php(319): Sabre\DAV\Server->start()
#5 /var/www/domain.org/htdocs/sabredav/groupwareserver.php(107): Sabre\DAV\Server->exec()
#6 {main}

I also checked the stacktrace generated by a PROPFIND request, which returns the corrent 401 Unauthorized response with WWW-Authenticate headers:

<s:message>Username or password was incorrect. Login was needed for privilege: {DAV:}read on addressbooks/user@domain.org/testing</s:message>
<s:file>/var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAVACL/Plugin.php</s:file>
<s:stacktrace>#0 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAVACL/Plugin.php(948): Sabre\DAVACL\Plugin->checkPrivileges('addressbooks/ya...', Array, 1, false)
#1 /var/www/domain.org/htdocs/sabredav/vendor/sabre/event/lib/WildcardEmitterTrait.php(89): Sabre\DAVACL\Plugin->propFind(Object(Sabre\DAV\PropFind), Object(Sabre\CardDAV\AddressBook))
#2 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/Server.php(1063): Sabre\DAV\Server->emit('propFind', Array)
#3 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/Server.php(989): Sabre\DAV\Server->getPropertiesByNode(Object(Sabre\DAV\PropFind), Object(Sabre\CardDAV\AddressBook))
#4 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/Server.php(1678): Sabre\DAV\Server->getPropertiesIteratorForPath('addressbooks/ya...', Array, 1)
#5 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/Server.php(1661): Sabre\DAV\Server->writeMultiStatus(Object(Sabre\Xml\Writer), Object(Generator), false)
#6 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/CorePlugin.php(363): Sabre\DAV\Server->generateMultiStatus(Object(Generator), false)
#7 /var/www/domain.org/htdocs/sabredav/vendor/sabre/event/lib/WildcardEmitterTrait.php(89): Sabre\DAV\CorePlugin->httpPropFind(Object(Sabre\HTTP\Request), Object(Sabre\HTTP\Response))
#8 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/Server.php(474): Sabre\DAV\Server->emit('method:PROPFIND', Array)
#9 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/Server.php(251): Sabre\DAV\Server->invokeMethod(Object(Sabre\HTTP\Request), Object(Sabre\HTTP\Response))
#10 /var/www/domain.org/htdocs/sabredav/vendor/sabre/dav/lib/DAV/Server.php(319): Sabre\DAV\Server->start()
#11 /var/www/domain.org/htdocs/sabredav/groupwareserver.php(107): Sabre\DAV\Server->exec()
#12 {main}</s:stacktrace>

It seems to me as if all the "work" (processing the actual request) is done before the ACL is checked.

Is there any reason for this design?

@Yannik
Copy link
Contributor Author

Yannik commented Feb 5, 2020

FYI: I discovered this bug as it is the reason why the roundcube carddave plugin currently fails to sync with sabredav servers.

In principal, DAV\Auth\Plugin adds a beforeMethod:* trigger, which is executed before the respective method handler is executed.

Ordinarily, this should trigger a challenge, as long as autoRequireLogin is set, which it is by default.
However, the DAVACL\Plugin disables this by default.

A workaround to this is to set allowUnauthenticatedAccess property of DAVACL\Plugin to false.

$DAVACL = new \Sabre\DAVACL\Plugin();
$DAVACL->allowUnauthenticatedAccess = false;
$server->addPlugin($DAVACL);

However, the issue I reported here should still be fixed, as it cannot be expected that everyone uses this workaround.

Additionally, it should IMO be considered to check ACL first and then do the work of the request in any cases that is possible.

@Yannik
Copy link
Contributor Author

Yannik commented Apr 30, 2020

@DeepDiver1975 Could you possibly give an update on this? :)

@ByteHamster
Copy link
Member

I believe that this problem is caused by sabre-io/xml@6accd59#diff-de76feda74829916defbb14d089c7c08. With that commit, sabre/dav no longer silently ignores empty content.

Cross-post from sabre-io/Baikal#964:
I have a pretty limited understanding of sabre/dav. My guess would be that this happens because digest auth needs to send 2 requests. The first request is probably executed without data because curl knows that the reply will only contain the nonce. Apparently, sabre/dav tries to parse the content anyway, even if it should just reject and send the nonce. Parsing the empty content then makes the whole request fail even if it should have never looked at the payload in the first place.

@DeepDiver1975 does this sound reasonable to you?

@mstilkerich
Copy link
Contributor

I think the issue is older, please see #932

@ByteHamster
Copy link
Member

Thanks @mstilkerich. Sounds like this issue can be closed then, to keep the discussion in one place.

larseggert added a commit to larseggert/Baikal that referenced this issue Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants