Security design should be improved

[obilodeau]

I like the idea of this project. However, with a quick glance, some flaws can be identified.

Using TCP on port 5999 is not the best way to pass secrets around, even locally.

Anyone with non-admin access to the machine might be able to crash or race access to that port and then would have access to all credentials.

Admins would be able to sniff. Which breaks some security requirements / standards because a sniffing admin could then impersonate other users (which he couldn't do if he only have access to hashes).

There are several ways it could be hardened but I'm not the best to give this advice (I'm not that familiar with Windows' internals).

However, here are some things to consider:

• running on a privileged port (anything below 1024), at least non-admin users can't start a daemon on these ports
• consider using a shared secret confirmation before giving credentials to the other service (defense in depth)
• using an IPC mechanism that is considered secure for Windows instead of TCP (file handles would be an example)

p.s.: Some credits go to François Renaud for his identification of the flaw.

[brockrob]

I don't disagree with any of this, and I'd rather lose the C# service altogether and do it all in the dll because you can't delete the strings from memory after handling them as Microsoft advise in their docs on doing this kind of thing. But, by way of counterpoint, if an attacker is in a position to read memory on a domain controller (or otherwise spawn listeners on local ports), one has much bigger problems than getting a few passwords pwned from time to time.

[jephthai]

I wanted a user-space DB to make the decisions because the dictionary could be arbitrarily large. What if someone wants a gigabyte of forbidden passwords, or implements really complicated logic for mutating a password for decision making? I don't want that kind of complexity in the DLL -- it should be as small and as simple as possible, with absolutely minimal overhead.

There might be an elegant way to do that all in the DLL, but I feel like it's bloating the wrong space if we do it that way. As you point out, a user who is in position to read process memory is also in a position to modify the DLL, install a new filter DLL, or otherwise manipulate the system, so I don't think much is at risk.

On Fri, Apr 27, 2018 at 9:42 AM, Robert Brock notifications@github.com wrote:

I don't disagree with any of this, and I'd rather lose the C# service altogether and do it all in the dll because you can't delete the strings from memory after handling them as Microsoft advise in their docs on doing this kind of thing. But, by way of counterpoint, if an attacker is in a position to read memory on a domain controller (or otherwise spawn listeners on local ports), one has much bigger problems than getting a few passwords pwned from time to time.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jephthai/OpenPasswordFilter/issues/21#issuecomment-384991137, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYlQaZWypechdf4zoz12gDMvYk4U_L1ks5tsy45gaJpZM4TeSrV .

[jephthai]

On the network / IPC mechanism point -- I wanted to avoid having to mess with named pipes and the potential for network accessibility. Someone who can sniff traffic on the domain controller can have an awful lot of fun besides waiting for users to change passwords. Binding to localhost seemed a simple, convenient way to ensure that someone's got to have control of the DC to get to it. I'd be open to another mechanism, as I started off with the localhost service primarily for simplicity and convenience.

On Mon, Apr 30, 2018 at 10:25 AM, Josh Stone yakovdk@gmail.com wrote:

I wanted a user-space DB to make the decisions because the dictionary could be arbitrarily large. What if someone wants a gigabyte of forbidden passwords, or implements really complicated logic for mutating a password for decision making? I don't want that kind of complexity in the DLL -- it should be as small and as simple as possible, with absolutely minimal overhead.

There might be an elegant way to do that all in the DLL, but I feel like it's bloating the wrong space if we do it that way. As you point out, a user who is in position to read process memory is also in a position to modify the DLL, install a new filter DLL, or otherwise manipulate the system, so I don't think much is at risk.

On Fri, Apr 27, 2018 at 9:42 AM, Robert Brock notifications@github.com wrote:

I don't disagree with any of this, and I'd rather lose the C# service altogether and do it all in the dll because you can't delete the strings from memory after handling them as Microsoft advise in their docs on doing this kind of thing. But, by way of counterpoint, if an attacker is in a position to read memory on a domain controller (or otherwise spawn listeners on local ports), one has much bigger problems than getting a few passwords pwned from time to time.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jephthai/OpenPasswordFilter/issues/21#issuecomment-384991137, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYlQaZWypechdf4zoz12gDMvYk4U_L1ks5tsy45gaJpZM4TeSrV .

[Bi0force1]

If i ALT+CTRL+DEL from my local workstation to reset my password, is the password shared to the DC in a plain text format? Is there anyway to encrypt this or share via SSL?

[jephthai]

It won't be any different from a password change without a password filter configured. I can't find a good reference at the moment, but the RPC calls that negotiate a password change request with the domain controller do, ultimately, have to communicate the plaintext password. It is protected in transit, however, as there are appropriate secrets for the client system to use for encrypting it before transmission. This has nothing to do with password filters, as those are executed locally on the domain controller and are not involved in the network communication to process the password change.

On Tue, Sep 25, 2018 at 12:03 PM Bi0force1 notifications@github.com wrote:

If i ALT+CTRL+DEL from my local workstation to reset my password, is the password shared to the DC in a plain text format? Is there anyway to encrypt this or share via SSL?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jephthai/OpenPasswordFilter/issues/21#issuecomment-424421522, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYlQTE7iywHk3IPa-Mk5qb7R04W30nwks5uemH1gaJpZM4TeSrV .

[Bi0force1]

Thank you for the response. My organization requires the password to remain encrypted at all times, so i'll need to find a way to implement this for our needs.

[jephthai]

Do remember that the domain controller processes the password in plaintext once received from the user before it is hashed for storage in NTDS.DIT. The plaintext password is handed off to all filter DLLs that are configured, including the default one that implements the built-in password strength requirements from Microsoft.

On Wed, Sep 26, 2018 at 10:57 AM Bi0force1 notifications@github.com wrote:

Thank you for the response. My organization requires the password to remain encrypted at all times, so i'll need to find a way to implement this for our needs.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jephthai/OpenPasswordFilter/issues/21#issuecomment-424766386, or mute the thread https://github.com/notifications/unsubscribe-auth/AEYlQTDWZDAKz4396_addYNjVUT07NZEks5ue6PNgaJpZM4TeSrV .

[ForumSchlampe]

      Thank you for the response. My organization requires the password to remain encrypted at all times, so i'll need to find a way to implement this for our needs.

We have a work in progress to hash the password in the dll before passing to the service through tcp (we had more problems to implment an tls connection in the c dll than the hasing way), if you are interessted we can upload our current implementation of this to our fork….but the hashing implementation isnt well tested yet

The drawback is, your filterlist (in our fork plaintext, mysql, mssql) must contain the password in the hashing algorithm (sha256) instead of plaintext.

We have done this cause we mostly agree to obilodeau that tcp on high ports isnt the best way to pass a plain password, on the other hand we disagree that we should use low ports which are all reserved and have to disagree that sniffing/capture local connections on high tcp ports are the same vulnerability than reading the memory, if an attacker can read the memory this is instead an unhandleable scenario and you should dump your dc, but the ability to have a local standard user session which is able to hijack local high ports on an dc is an handleable situation (not a nice scenario)