[jira] [Commented] (MESOS-5405) Make fields in authorization::Request protobuf optional.

["Till Toenshoff (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MESOS-5405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15287889#comment-15287889 ] 

Till Toenshoff commented on MESOS-5405:
---------------------------------------

Seems the correct solution here is to make {{subject}} and {{object}} optional within the proto definition.

Keeping them mandatory would e.g. mandate a rather ugly fix for the first example:
{noformat}
        if (principal.isSome()) {
          authRequest.mutable_subject()->set_value(principal.get());
        } else {
          authRequest.mutable_subject();
        }
{noformat}
I firmly believe that this is not what we want for expressing {{ANY}} as used by our internal representation whenever an empty / missing {{subject}} or {{object}} got supplied.

> Make fields in authorization::Request protobuf optional.
> --------------------------------------------------------
>
>                 Key: MESOS-5405
>                 URL: https://issues.apache.org/jira/browse/MESOS-5405
>             Project: Mesos
>          Issue Type: Bug
>            Reporter: Alexander Rukletsov
>            Priority: Blocker
>              Labels: mesosphere, security
>             Fix For: 0.29.0
>
>
> Currently {{authorization::Request}} protobuf declares {{subject}} and {{object}} as required fields. However, in the codebase we not always set them, which renders the message in the uninitialized state, for example:
>  * https://github.com/apache/mesos/blob/0bfd6999ebb55ddd45e2c8566db17ab49bc1ffec/src/common/http.cpp#L603
>  * https://github.com/apache/mesos/blob/0bfd6999ebb55ddd45e2c8566db17ab49bc1ffec/src/master/http.cpp#L2057
> I believe that the reason why we don't see issues related to this is because we never send authz requests over the wire, i.e., never serialize/deserialize them. However, they are still invalid protobuf messages. Moreover, some external authorizers may serialize these messages.
> We can either ensure all required fields are set or make both {{subject}} and {{object}} fields optional. This will also require updating local authorizer, which should properly handle the situation when these fields are absent. We may also want to notify authors of external authorizers to update their code accordingly.
> It looks like no deprecation is necessary, mainly because we already—erroneously!—treat these fields as optional.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)