Re: pg_hba_lookup function to get all matching pg_hba.conf entries

On Wed, Dec 23, 2015 at 8:54 PM, Shulgin, Oleksandr
<oleksandr(dot)shulgin(at)zalando(dot)de> wrote:
> On Wed, Dec 16, 2015 at 9:33 AM, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
> wrote:
>>
>>
>> Function is changed to accept default values.
>>
>> Apart from the above, added a local memory context to allocate the memory
>> required for forming tuple for each line. This context resets for every
>> hba line
>> to avoid consuming unnecessary memory for scenarios of huge pg_hba.conf
>> files.
>>
>> In the revert_hba_context_release_in_backend patch, apart from reverting
>> the commit - 1e24cf64. In tokenize_file function, changed the new context
>> allocation from CurrentMemoryContext instead of TopMemoryContext.
>>
>> Patch apply process:
>> 1. revert_hba_context_release_in_backend_2.patch
>> 2. pg_hba_lookup_poc_v7.patch
>
>
> Hello,
>
> 1. Have you considered re-loading the HBA file upon call to this function in
> a local context instead of keeping it in the backends memory? I do not
> expect that the revert of 1e24cf645d24aab3ea39a9d259897fd0cae4e4b6 would be
> accepted, as the commit message refers to potential security problems with
> keeping this data in backend memory:
>
> ... This saves a
> probably-usually-negligible amount of space per running backend. It
> also
> avoids leaving potentially-security-sensitive data lying around in
> memory
> in processes that don't need it. You'd have to be unusually paranoid to
> think that that amounts to a live security bug, so I've not gone so far
> as
> to forcibly zero the memory; but there surely isn't a good reason to
> keep
> this data around.

Yes, it is possible to load the file locally whenever the lookup
function is called.
Only thing i am considering is performance impact because of huge file load
whenever the function is called.

> 2. I also wonder why JSONB arrays for database/user instead of TEXT[]?

When I first tried this functionality as a view, it became very
difficult to deal with
keyword database and user names, so at that time, it was thought to use jsonb
instead of text[], thinking of easy to handle the keywords. But later decided to
drop the view approach itself. I can change it if others also feel the same.

> 3. What happens with special keywords for database column like
> sameuser/samerole/samegroup and for special values in the user column?

There is no special handling for the keywords in this approach. Based on the
inputs to the function, it checks for the matched line in all hba lines.

For example, if a configuration line contains 'all' for database and user names,
then if user gives any database name and user name this line will be matched
and returned.

> 4. Would it be possible to also include the raw unparsed line from the HBA
> file? Just the line number is probably enough when you have access to the
> host, but to show the results to someone else you might need to copy the raw
> line manually. Not a big deal anyway.

IMO as we are already showing all line information in columns
separately, it may not
be looks good.

> 5. Some tests demonstrating possible output would be really nice to have.

Do you mean regression tests? In case of install check case, the results are
based on the server configuration that is running. It may be difficult to write
tests to pass in all scenarios. Because of this reason i didn't add them.

Regards,
Hari Babu
Fujitsu Australia