BIND 10 #2595: cmdctl with chusr crashes due to permission issue

[BIND 10 Development]

#2595: cmdctl with chusr crashes due to permission issue
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |  jinmei
            Priority:  medium        |                       Status:
           Component:  cmd-ctl       |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130122
         Sub-Project:  Core          |                   Resolution:
Estimated Difficulty:  3             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => jinmei


Comment:

 Replying to [comment:6 jinmei]:
 > '''general'''
 >
 > - I think we need a changelog for this ticket.
 >

 [bug]
 b10-cmdctl no longer aborts on basic file issues with its https
 certificate or private key file. It performs additional checks, and
 provides better error if these fail. Additionally, bindctl provides a
 better error report if it is unable to connect over https connection.

 >   of treating it as fatal with no clear benefit.  Is there any real
 >   advantage of handling it as a fatal event?
 >

 No, perhaps just that it is slightly more annoying and hence gets looked
 at sooner, but you're right, unfatalizing now. (by raising socket.error
 like the other exception clause).

 > '''bindcmd.py'''
 >
 > - Is it okay to not have a test for it?

 added a separate test for !__try_login

 > - This branch slightly changes failure mode if the following after
 >   `__try_login` fails with socket.error:
 > {{{#!python
 >             data = response.read().decode()
 > }}}
 >   Previously it's immediately caught and converted to `FailToLogin`.
 >   Now it's propagated to the upper layer and I guess caught here:
 > {{{#!python
 >         except socket.error as err:
 >             print('Failed to send request, the connection is closed')
 >             return 1
 > }}}

 oh right, updated (it now performs the read() as well, and returns a tuple
 on success)

 > - `_config_data_check`: This comment seems to have to be updated:
 > {{{#!python
 >                 #TODO, only check whether the file exist,
 >                 # further check need to be done: eg. whether
 >                 # the private/certificate is valid.
 > }}}
 >  (it at least does readability check too)
 >

 right :) updated

 > '''cmdctl_messages.mes'''
 >
 > - missing period?
 > {{{
 > While running, b10-cmdctl encountered a fatal exception and it will shut
 down
 > }}}

 moot now (the message was removed un the unfataling)

 > - CMDCTL_SSL_SETUP_FAILURE_READING_CERT message: I'd note that
 >   depending on whether to treat this case fatal it may have to be
 >   updated.
 >

 ack, updated

 > '''cmdctl_test.py'''
 >
 > - test_check_file: it modifies (the attributes of) a file in the
 >   source tree:
 > {{{#!python
 >         file_name = SRC_FILE_PATH + 'cmdctl-keyfile.pem'
 >         check_file(file_name)
 >         with UnreadableFile(file_name):
 >             self.assertRaises(CmdctlException, check_file, file_name)
 > }}}
 >   Does it work with distcheck?
 >

 as 2631 also noted, it was wrong anyway. So I updated it slightly based on
 the patch proposed there (but with a BUILD_FILE_PATH instead of two
 separate values).

 > - test_wrap_sock_in_ssl_context: not really the subject of this task,
 >   but the test doesn't seem to be sufficient: its return value isn't
 >   checked; the ssl.SSLError case isn't tested.

 added two additional checks

-- 
Ticket URL: <http://bind10.isc.org/ticket/2595#comment:8>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development