{"action":"created","issue":{"url":"https://api.github.com/repos/balabit/syslog-ng/issues/1888","repository_url":"https://api.github.com/repos/balabit/syslog-ng","labels_url":"https://api.github.com/repos/balabit/syslog-ng/issues/1888/labels{/name}","comments_url":"https://api.github.com/repos/balabit/syslog-ng/issues/1888/comments","events_url":"https://api.github.com/repos/balabit/syslog-ng/issues/1888/events","html_url":"https://github.com/balabit/syslog-ng/pull/1888","id":297737827,"number":1888,"title":"Password protected ssl keys","user":{"login":"lbudai","id":5908786,"avatar_url":"https://avatars1.githubusercontent.com/u/5908786?v=4","gravatar_id":"","url":"https://api.github.com/users/lbudai","html_url":"https://github.com/lbudai","followers_url":"https://api.github.com/users/lbudai/followers","following_url":"https://api.github.com/users/lbudai/following{/other_user}","gists_url":"https://api.github.com/users/lbudai/gists{/gist_id}","starred_url":"https://api.github.com/users/lbudai/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lbudai/subscriptions","organizations_url":"https://api.github.com/users/lbudai/orgs","repos_url":"https://api.github.com/users/lbudai/repos","events_url":"https://api.github.com/users/lbudai/events{/privacy}","received_events_url":"https://api.github.com/users/lbudai/received_events","type":"User","site_admin":false},"labels":[{"id":104723147,"url":"https://api.github.com/repos/balabit/syslog-ng/labels/in%20progress","name":"in progress","color":"207de5","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-02-16T10:21:49Z","updated_at":"2018-02-16T10:36:30Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/balabit/syslog-ng/pulls/1888","html_url":"https://github.com/balabit/syslog-ng/pull/1888","diff_url":"https://github.com/balabit/syslog-ng/pull/1888.diff","patch_url":"https://github.com/balabit/syslog-ng/pull/1888.patch"},"body":"# Support password protected SSL keys\r\n\r\n## Concept\r\nThe solution has three key parts\r\n * need to store the secret keys somewhere, possible in a non-dumpable memory area (credential store)\r\n * inject secret into syslog-ng with syslog-ng-ctl (syslog-ng-ctl)\r\n * afsocket:\r\n   * network source\r\n     * make it possible to disable client connections until a valid password is not provided by the user, and open connections when the password is available\r\n   * network destination\r\n     * try to reconnect to destination until a valid password is not available\r\n\r\n\r\n### Technical details\r\n\r\n#### Network source & destination (afsocket)\r\nIn case of TCP network source, we call a `TransportMapper` init as part of the main init. This `TransportMapper::init` is responsible for setting up the `TLSContext`.\r\nIn the original implementation, when someone uses a password protected SSL key, OpenSSL will ask for the password (from stdin as we did not set the default password callback) and entire syslog-ng blocks until the password is not set.\r\nOne of our goal was to avoid blocking the entire syslog-ng instance in such case.\r\nWe can do this by splitting the init logic:\r\n * first part is responsible for allocating resources (socket and bind system calls)\r\n * second part is to start accepting client connections(listen & accept system calls)\r\n\r\nAs `TransportMapper` initializes the `TLSContext`, it has the knowledge whether we can finalize the Source driver init or not, so we added an asynchronous TransportMapper init method.\r\nThe async init requires a callback which is responsible for finishing the Source initialization (it calls the finalize init method which then calls the listen&accept).\r\n\r\n#### Callback-hell\r\nFinalize callback (which is passed to `TransportMapper::async_init`, `_finalize_init` in `afsocket-source.c`) is not the only one callback we have to use.\r\nOpenSSL uses a password callback on its own: this callback is responsible for filling up a buffer with the password.\r\nThis callback (`_pem_passwd_callback` in `tlscontext.c`) has a user data argument which is used for passing the key parameter for secret store in our implementation.\r\nBased on the key we can get the stored password.\r\n\r\nThe third callback is used with secret store. Secret store handles a kind of hashtable which stores the keys and callbacks. Absolute path of the PEM files are used as keys, and\r\nwhen the user injects a password for a specific key, the registered callback will be called.\r\n\r\n#### How it works together?\r\n* When a tcp source is configured, asynchronous transportmapper init method will be called. When tls is not used, it just call the finalize callback and start accepting client connections.\r\n* When tls is used and the pem file is not protected by password, tls context setup will be created successfully, callback is called immediately, source driver is accepting client connections.\r\n* When tls is used and the pem file is protected by password, tls context setup will fail, another callback(secret-store-callback) is registered(`secret_storage_subscribe_for_key`) in secret-storage with\r\nkey = pem file name. Source driver init returns successfully but won't be able to receive client connections as it is not listening.\r\nWhen user inputs the password for the key, the registered function is called: it will try to setup tls context. The tls setup context method will trigger OpenSSL to call its own password callback(with\r\nuserdata=pem file), which will get the password from the secret store. If the password is not valid, we re-register the callback in secret-store (and display error message and change status in secret-storage).\r\nWhen the password is fine, and the tls context setup is succeeded, we call the callback passed to `TransportMapper::async_init` which then calls listen and accept, so the source driver is ready\r\nfor accepting client connections.\r\n\r\n#### Sequence diagram: source\r\n![network-source](https://user-images.githubusercontent.com/5908786/36303088-288f1396-130b-11e8-929c-3bbc1db70c11.png)\r\n\r\n#### Destination\r\nWe use the same `TransportMapper::async_init`, the only difference is that the finalize init callback calls the `afsocket_dd_try_connect`.\r\nSo we handle invalid/unavailable password as it would be a connection error.\r\n\r\n#### syslog-ng-ctl\r\nOperators can use `syslog-ng-ctl` to inject secrets to the secret storage.\r\n\r\n* The already stored secrets together with their status can be queried by `syslog-ng-ctl credentials status`.\r\n\r\n* Secrets can be added to the secret storage by `syslog-ng-ctl credentials add id secret` command.\r\n\r\nFrom ssl passhprase point of view the `id` in `syslog-ng-ctl` must be the exact string written in the configuration for the location of the encrypted key. If not the exact string is used, the unlock will not work. If full path is used inside the configuration, the same full path must be used in `syslog-ng-ctl`. If relative path is used, the same relative path must be used in `syslog-ng-ctl`. The reason is technical: the subscription happens with the string used inside the configuration, and the same key is used by `syslog-ng-ctl` to store the secret.\r\n\r\nThis behaviour is a counter-intuitive, but there are plans to resolve it in the future.\r\n\r\nFor sake of convenience, users can either add credentials with `-i/--id, -s/--secret` options, but also with positional parameters. In case an option is missing, `syslog-ng-ctl` tries to fill the data from the positional parameter. If the options could not be filled either way, error will be reported, and a help text is emitted about the options. For sake of the latter: a little refactor was necessary in syslog-ng-ctl: the `GOptionContext` needs to be passed to the callbacks as well.\r\n\r\n`syslog-ng-ctl credentials` is a command that has two subcommands: `add` and `status`. So to support this feature, the description of subcommands is extended: now subcommands can also have subcommands. The command descriptor table therefore is now represented as a tree. To find the appropriate command and callback, the tree needs to be traversed from now on, collecting the nodes in the meantime: `find_active_mode`. Once the correct leaf is found, the callback is invoked with the appropriate arguments.\r\n\r\n#### secret-storage\r\nThe secret-storage module is responsible for storing the credentials - provided by syslog-ng-ctl -, and callbacks - registered by drivers.\r\n\r\nSecret-storage is a hashtable with a special memory allocator. The hashtable is indexed by the keys provided by either `syslog-ng-ctl` or `TransportMapper`. An entry is a secret together with some callbacks and a secret status. The values are stored on a specially allocated memory area, that is locked into memory (not written into swap), and not written to a core file.\r\n\r\nThere is an ulimit for `RLIMIT_MEMLOCK` (total size of locked memory). This restricts the number of secrets handled by syslog-ng in parallel. The memory allocator can allocate only larger chunks than pagesize, which is typically 4k. The default ulimit for `RLIMIT_MEMOLOCK` is 64k, meaning only 16 secrets can be handled only at a time, depending the size of the secrets, including the secrets cloned internally.\r\n\r\nSecret storage is a core functionality, but due to a technical limitation, it is shipped as a separate shared library. The reason is that the symbols of the secret storage is set to hidden visibility by default (which we did not want to change in libsyslog-ng.so), meaning a specially crafted syslog-ng module cannot use secret-storage symbols, unless secret-storage explicitely allows it. Specially: the hash table of secrets is also hidden. As a side effect, users of the secret-storage need to explicitely link to libsecret-storage.so. This feature alone does not add much to security, because for now any driver can query any secrets using `secret_storage_get_secret_by_name`, though there are plans to add some level of authentication for drivers in the future.\r\n\r\nAny time a driver works with a secret, secret-storage first clones the secret, and provides only a copy for the driver. As a result, the different drivers cannot accidentally modify the secret for each other.\r\n\r\nThe status of a secret is initialized to `PENDING` when the first subscription comes from `TransportMapper`. After that only the drivers update the status of the secrets.\r\n\r\nThere is a function added to loop through the secret status: `secret_store_status_foreach`. This is used by `syslog-ng-ctl` to list the stored secrets with status.\r\n\r\n#### nondumpable-allocator\r\nA special memory allocator added to store the secrets in memory. The difference between a malloc-ed memory and a nondumpable-memory is the latter is not written to swap, and not written to core file. During deallocation, the area is zeroed out as well.\r\n\r\nThe allocator works on mapped anonymous pages, calling `mlock` and `madvise` on the pages. Each allocated memory contains a buffer provided for the user, and some metadata about the allocation: the allocated and available size. The available size is used for zeroing during deallocation.\r\n\r\nThe layout of buffer and metadata tries to imitate the malloc implementation in glibc: the metadata is stored right before the user buffer in memory: i. e. the allocated anonymous map starts with the metadata, however the pointer returned by the allocator is the location of the user buffer inside the anonymous memory. Hence the deallocator can easily find the metadata by a simple substraction from the location provided by the user.\r\n\r\nThere is an ulimit for locking memory: `RLIMIT_MEMLOCK`, which is usually 64k. As the allocation happens in pages, usually 4k, the number of parallel allocations is very limited by default.\r\n\r\n### Future improvements\r\n * Use crendential store for SQL , mongodb, and amqp destinations (->any other sources, destinations where credential can be set by users).\r\n"},"comment":{"url":"https://api.github.com/repos/balabit/syslog-ng/issues/comments/366200307","html_url":"https://github.com/balabit/syslog-ng/pull/1888#issuecomment-366200307","issue_url":"https://api.github.com/repos/balabit/syslog-ng/issues/1888","id":366200307,"user":{"login":"kira-syslogng","id":25661985,"avatar_url":"https://avatars0.githubusercontent.com/u/25661985?v=4","gravatar_id":"","url":"https://api.github.com/users/kira-syslogng","html_url":"https://github.com/kira-syslogng","followers_url":"https://api.github.com/users/kira-syslogng/followers","following_url":"https://api.github.com/users/kira-syslogng/following{/other_user}","gists_url":"https://api.github.com/users/kira-syslogng/gists{/gist_id}","starred_url":"https://api.github.com/users/kira-syslogng/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kira-syslogng/subscriptions","organizations_url":"https://api.github.com/users/kira-syslogng/orgs","repos_url":"https://api.github.com/users/kira-syslogng/repos","events_url":"https://api.github.com/users/kira-syslogng/events{/privacy}","received_events_url":"https://api.github.com/users/kira-syslogng/received_events","type":"User","site_admin":false},"created_at":"2018-02-16T10:36:30Z","updated_at":"2018-02-16T10:36:30Z","author_association":"CONTRIBUTOR","body":"Build FAILURE, the tests were executed on test branch: master and test suite: functions\n"}}
{"actor":{"display_login":"kira-syslogng"}}