BIND 10 #2641: Disable default account, require authentication setup during initialization

[BIND 10 Development]

#2641: Disable default account, require authentication setup during initialization
-------------------------------------+-------------------------------------
            Reporter:  shane         |                        Owner:
                Type:  enhancement   |  jinmei
            Priority:  very high     |                       Status:
           Component:  bind-ctl      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130319
         Sub-Project:  Core          |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => jinmei


Comment:

 Hi Jinmei

 Replying to [comment:30 jinmei]:
 > Maybe I wasn't clear enough...first off, I didn't understand whether
 > or not SSL or socket error can still happen in the scenario described
 > in #2595.  If it can still happen, we should still also prevent that
 > in the sense we discussed in this (#2641) ticket.  If it can't, at
 > least it doesn't make sense any more to be this specific in bindcmd.py:
 >
 > {{{#!python
 >         except ssl.SSLError as err:
 >             self._print("SSL error while sending login information: ",
 err)
 >             if err.errno == ssl.SSL_ERROR_EOF:
 >                 self.__print_check_ssl_msg()
 >         except socket.error as err:
 >             self._print("Socket error while sending login information:
 ", err)
 >             # An SSL setup error can also bubble up as a plain
 CONNRESET...
 >             # (on some systems it usually does)
 >             if err.errno == errno.ECONNRESET:
 >                 self.__print_check_ssl_msg()
 >             pass
 > }}}
 >
 > At least it doesn't make sense to catch specific errno's like
 > SSL_ERROR_EOF or ECONNRESET.  `__print_check_ssl_msg` would also not
 > be needed any more either.  And, since ssl.SSLError is derived from
 > socket.error (but maybe Python 3.3 has changed that; don't remember
 > it), it also does not even make sense to catch SSLError separately.
 > And, further, if socket.error is sufficiently unusual (again, it's
 > just an 'if'; I didn't check how often it can happen in the latest
 > code), explicitly catching socket.error may also not even make sense.
 >
 > What I wanted is to clarify these things and make the code as simple
 > (while still providing necessary logs/outputs) as possible based on
 > that clarification.

 I have unified the exceptions to check them in a single `except` clause,
 and have also updated the messages. Is this a reasonable solution?
 I ask this because it's still possible for a socket error to occur due
 to the environment (not just some condition at `b10-cmdctl`) and we may
 want to handle such an exception.

 > BTW, bindctl unit tests now fail.  I suspect it was because of the
 > latest changes.
 >
 > (and I couldn't confirm whether distcheck now passes).

 If it fails again, please can you include how this failed for you? I do
 run `make check` (and in this bug, `make distcheck` after your recent
 comment) before putting a bug to review. It did not fail for me, so the
 failure message would be helpful.

 From code updates done now, it could be because messages printed after
 an exception in bindctl changed, but `distcheck` did not fail when the
 bug was put to review last time.

 > As for the change to b10-certgen_test, (although maybe not directly
 > related to this ticket) I wonder why we have this 'if-else':
 >
 > {{{#!python
 >         if ('CMDCTL_TESTDATA_PATH' in os.environ):
 >             path = os.environ['CMDCTL_TESTDATA_PATH'] +
 "/tests/testdata/"
 >         else:
 >             path = "testdata/"
 > }}}
 >
 > Also, setting CMDCTL_TESTDATA_PATH' to the srcdir seems to be awkward
 > (why it's named "TESTDATA_PATH" then?).  Some clarification and
 > cleanup seem to be needed here.

 This environment variable has been renamed to `CMDCTL_SRC_PATH` and the
 code has been adjusted for it.

 Before putting this bug to review, I ran `make distcheck` and it passes
 on my workstation.

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