Asterisk
  1. Asterisk
  2. ASTERISK-6629

[branch] SSL encryption for Asterisk Manager Interface (AMI)

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Severity: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Target Release Version/s: None
    • Component/s: Core/ManagerInterface
    • Labels:
      None
    • Mantis ID:
      6812
    • Regression:
      No

      Description

      It is our humble opinion that everything going over an IP network should be encrypted from end to end, or at least from an end to an intermediate trusted point. The Manager Interface certainly fails that motto currently, so we've developed a solution.

      Several files are enclosed which should work on SVN TRUNK as of today (2006-03-27) for enabling the encryption of AMI socket connections via OpenSSL. This is completely backwards-compatible with existing AMI clients, since the system will just wait on the socket for a configurable number of milliseconds before failing over to non-encrypted versions (if desired.)

      The new Makefile will automatically generate a cert, since it is expected that nobody will ever actually do this without it happening as a matter of course. Of course, normal certs can be utilized.

      Requires OpenSSL, but that is not a big deal since we already require OpenSSL for other tasks. Can easily be commented out (one line) during compile.

      These patches are being contributed by Tello Inc. I have a disclaimer on file, and I will submit a disclaimer for Tello shortly, as I assume that is required.

      Concept/some docs: John Todd (jtodd@tello.com)
      Programmers: Mahesh Karoshi (mkaroshi@tello.com) and Remco Treffkorn (rtreffkorn@tello.com)

      1. ami-ssl-patch2
        16 kB
      2. ssl.txt
        6 kB
      3. ssl-patch.txt
        36 kB
      4. ttb.diff
        0.8 kB
      5. ttb2.diff
        1.0 kB

        Activity

        Hide
        jmls added a comment -

        work is being done on this by rizzo. stay tuned ..

        Show
        jmls added a comment - work is being done on this by rizzo. stay tuned ..
        Hide
        John Todd added a comment -

        I am actually unclear on the status of this patch. It was created long ago, and patched against TRUNK. It worked well. In fact, it was tested enough that even astmanproxy supports it. However, it was not evaluated for inclusion into trunk for reasons opaque to me. Now, there is an HTTP-based manager interface, which can have SSL layered onto it. (Why a duplicate effort? Unclear to me.) That is now in TRUNK as far as I know. So what of this patch? Should it be marked as closed or rejected?

        Show
        John Todd added a comment - I am actually unclear on the status of this patch. It was created long ago, and patched against TRUNK. It worked well. In fact, it was tested enough that even astmanproxy supports it. However, it was not evaluated for inclusion into trunk for reasons opaque to me. Now, there is an HTTP-based manager interface, which can have SSL layered onto it. (Why a duplicate effort? Unclear to me.) That is now in TRUNK as far as I know. So what of this patch? Should it be marked as closed or rejected?
        Hide
        Luigi Rizzo added a comment -

        jtodd: first-sorry for not coordinating with you when i worked on ssl support.
        i was not aware of the patch when i started, and when i found it the
        work was already done.
        From a purely technical point of view:
        1. obviously i am perfectly fine in centralizing in one file the routines
        to handle the ssl initialization. The reason why i put them in http.c
        was that in trunk there was no other user yet and we needed to start
        somewhere, but moving it is trivial. So, in this respect, there is no
        real difference between this code and what is in trunk.

        2. The above (as probably discussed by others) should definitely be taken
        a step further, by centralizing also the basic socket/bind/listen/accept
        calls to avoid replicating it. This is something that neither this patch
        nor the code in trunk does, and requires a bit of restructuring of the
        client code (http.c, manager.c etc.) which does its own "polling" of the
        socket within a loop.

        3. what i don't like in this specific patch is the way
        "encrypted descriptors" are handled, because
        it is not transparent to applications, which need to be aware of that
        by calling the s_ version of read() and write().

        It's not just that "i don't like the approach", the problem is that it
        makes debugging the code a nightmare, because you have to be sure,
        when you pass a descriptor around, that all interested pieces of code
        do the proper changes. This is easy when you have a small number of
        such places, it is a lot harder when you want to bring SSL support to
        e.g. the CLI or other third party code where you don't have a lot of
        control on how descriptors are used.

        4. This said, the code in trunk (http.c) handles a slightly different
        case, because http.c uses FILE * for I/O, and for those there is a
        completely transparent solution (the one that is in trunk) that uses
        fuopen() or fopencookie() to provide the client with a FILE * which
        can handle ssl in a totally transparent way.

        5. Unfortunately the solution above doesn't work with low-level file
        descriptors. Here we need some good idea e.g. possibly playing with
        the linker to replace the standard IO library calls with our own,
        ssl-aware, in a transparent way, or some smart trick to hide
        the ssl hooks underneath.
        At the moment i don't have good ideas here (other than using a separate
        thread to provide the encryption between the ssl descriptor and
        the standard one).

        Show
        Luigi Rizzo added a comment - jtodd: first-sorry for not coordinating with you when i worked on ssl support. i was not aware of the patch when i started, and when i found it the work was already done. From a purely technical point of view: 1. obviously i am perfectly fine in centralizing in one file the routines to handle the ssl initialization. The reason why i put them in http.c was that in trunk there was no other user yet and we needed to start somewhere, but moving it is trivial. So, in this respect, there is no real difference between this code and what is in trunk. 2. The above (as probably discussed by others) should definitely be taken a step further, by centralizing also the basic socket/bind/listen/accept calls to avoid replicating it. This is something that neither this patch nor the code in trunk does, and requires a bit of restructuring of the client code (http.c, manager.c etc.) which does its own "polling" of the socket within a loop. 3. what i don't like in this specific patch is the way "encrypted descriptors" are handled, because it is not transparent to applications, which need to be aware of that by calling the s_ version of read() and write(). It's not just that "i don't like the approach", the problem is that it makes debugging the code a nightmare, because you have to be sure, when you pass a descriptor around, that all interested pieces of code do the proper changes. This is easy when you have a small number of such places, it is a lot harder when you want to bring SSL support to e.g. the CLI or other third party code where you don't have a lot of control on how descriptors are used. 4. This said, the code in trunk (http.c) handles a slightly different case, because http.c uses FILE * for I/O, and for those there is a completely transparent solution (the one that is in trunk) that uses fuopen() or fopencookie() to provide the client with a FILE * which can handle ssl in a totally transparent way. 5. Unfortunately the solution above doesn't work with low-level file descriptors. Here we need some good idea e.g. possibly playing with the linker to replace the standard IO library calls with our own, ssl-aware, in a transparent way, or some smart trick to hide the ssl hooks underneath. At the moment i don't have good ideas here (other than using a separate thread to provide the encryption between the ssl descriptor and the standard one).
        Hide
        Luigi Rizzo added a comment -

        Basically, the feature has been imported in trunk, although
        not with the patch provided.
        Yesterday's commits provides AMI-over-TLS support using the same
        mechanism (and code) used for HTTPS.

        Show
        Luigi Rizzo added a comment - Basically, the feature has been imported in trunk, although not with the patch provided. Yesterday's commits provides AMI-over-TLS support using the same mechanism (and code) used for HTTPS.
        Hide
        Serge Vecher added a comment -

        scroll keeper adds "as of r.48351." Thanks everyone for good work ...

        Show
        Serge Vecher added a comment - scroll keeper adds "as of r.48351." Thanks everyone for good work ...

          People

          • Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development