Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposed Security Patch: Replacement of use of SHA-1 in the SRP Client Proof with SHA-256 [CORE5788] #6051

Closed
firebird-automations opened this issue Apr 5, 2018 · 28 comments

Comments

@firebird-automations
Copy link
Collaborator

Submitted by: Tony Whyman (twhyman)

Block progress on JDBC536

Attachments:
srp_sha256.patch
srp_sha256_v2.patch

Votes: 1

This proposed patch results from a security review of the Firebird SRP-6a implementation taking into account current NIST guidance on the use of SHA-1 - see NIST Special Publication 800-131A, Revision 1, Transitions: Recommendation for Transitioning the Use of Cryptographic Algorithms and Key Lengths (http://dx.doi.org/10.6028/NIST.SP.800-131Ar1) chapter 9. This guidance disallows the general use of SHA-1 for "Digital Signature Generation" whilst permitting continued use for "Digital Signature Verification".

Review of the Firebird SRP implementation appears to indicate that most uses of SHA-1 continue to be permitted under NIST guidance except for its use in generating the client proof. The SRP client proof may be characterised as a "Poor Man's Digital Signature" in that it provides a two party proof of identity rather than the third party proof normally expected from a Digital Signature i.e. it is not a non-repudiable proof. Nevertheless, it is believed that generation of the client proof falls under the heading of "Digital Signature Generation" when considering the NIST Guidance.

Continued use of SHA-1 in order to generate the client proof appears to risk leakage of the shared session key used to encrypt "over-the-wire" encryption and which hence also provides peer entity authentication during the lifetime of the connection. This may result in an attacker being able to monitor confidential communication either during the connection or at some later date and this could include leakage of an encryption key used to encrypt the user database, if this is passed from client to server during the connection.

Such an attack is viable if weaknesses in SHA-1 can be exploited to allow a brute force attack on the client proof to be computationally feasible. All parts of the message on which the client proof is based may be known to an attacker with the exception of the shared session key and such an attack would concentrate on revealing this key. If it were possible to reveal the shared session key in real time then additionally a man-in-the-middle attack would be feasible.

The severity of this issue is viewed as Important but not Critical. This is because (a) users that comply with NIST Guidance as a matter of policy may feel unable to use Firebird/SRP and hence choose or migrate to a different database, and (b) users that rely on SRP/over the wire encryption to protect confidential communication have a long term risk that the confidentiality of their data may be compromised. The attack may also be mitigated through the use of other procedures to protect communications (e.g. a secure VPN).

The patch adds a new directory to the source code tree (src/common/sha2) containing an implementation of the SHA-2 family of message digests derived from the implementation published by Olivier Gay <mailto:olivier.gay@a3.epfl.ch> (see https://github.com/ouah/sha2). This has been adapted for Firebird as a set of classes that follow the model of the existing Firebird::Sha1 class. Classes are provided for SHA-224, SHA-256, SHA-384 and SHA-512. A SHA-2 compliancy confidence test is also included.

The SRP RemotePassword class is modified to additionally include a method for generating a client proof using SHA-256 as the message hash.

The SRP client class is modified to use only SHA-256 for generating the client proof.

The SRP server class is modified to use either SHA-1 or SHA-256 for verifying the client proof, with the verification method depending on the length of the client proof. This is believed to be compliant with NIST Guidance for legacy use of SHA-1 and permits backwards compatibility with older clients.

The patch also modifies the makefiles for posix builds in order to include the SHA-2 classes in the "common" library and has been tested on Linux. It may be necessary to modify the build procedures for other platforms in order to use the patch.

It is proposed that this patch is expedited into the next step release of Firebird 3 and which should be made available as soon as possible.

Commits: 54bf8a3 406b169 b9a93f5

@firebird-automations
Copy link
Collaborator Author

Commented by: Tony Whyman (twhyman)

Patch to Firebird 3.0.3 source code tree

@firebird-automations
Copy link
Collaborator Author

Modified by: Tony Whyman (twhyman)

Attachment: srp_sha256.patch [ 13234 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @AlexPeshkoff

assignee: Alexander Peshkov [ alexpeshkoff ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

> The SRP client class is modified to use only SHA-256 for generating the client proof.
Does it mean that with suggested change fbclient 3.0.4 will not be able to attach to server 3.0.3 ?

> The SRP server class is modified to use either SHA-1 or SHA-256 for verifying the client proof, with the verification method depending on the length of the client proof.
That's OK. But what modification of SHA is used to work with stored on the disk hashes? Will old passwords work for new server/client pair? For new server with old client?

I suggest you to leave current SRP plugin as is and add instead new one, with enhanced security.
We may have as many plugins as we wish.

@firebird-automations
Copy link
Collaborator Author

Commented by: Tony Whyman (twhyman)

> Does it mean that with suggested change fbclient 3.0.4 will not be able to attach to server 3.0.3 ?
That is true. It is designed to allow a roll out of the change first to servers and then to clients.

>That's OK. But what modification of SHA is used to work with stored on the disk hashes? Will old passwords work for new server/client pair? For new server with old client?
There is no change proposed to any use of SHA-1 other than to the client proof. The on disk hashes are unchanged and old passwords will continue to work.

>I suggest you to leave current SRP plugin as is and add instead new one, with enhanced security.
That is obviously an option. However, I did not propose this because I saw little value in leaving the old client in place. Note that there is no proposed change to the SRP User Manager and the proposed patch to the SRP server is backwards compatible with 3.0.3. As long as users upgrade their server software first and then their clients then they will not have an issue.

On the other hand, if you retain a legacy SRP client in place then it becomes all too easy for a client to be mis-configured to use the legacy client rather than the updated version and as there is no server logging mechanism for a DBA to monitor whether clients are using SHA-256, there is no easy means to ensure that clients are up-to-date.

@firebird-automations
Copy link
Collaborator Author

Commented by: Tony Whyman (twhyman)

I also thought it useful to pre-empt the question on why it is not necessary to replace SHA-1 in computation of the password verifiers with the following explanation:

The SRP password verifier is computed as:

<password verifier> = v = g^x mod N

where

x = H(<salt>, H( I, ':',<raw password>))

and H() is the hash algorithm.

as such, the password verifier is protected from attack by the intractability of the Discrete Logarithm Problem in that while the computation of v = g^x mod N is computationally feasible in a reasonable time, the inverse i.e. computing the logarithm (base g) of v mod N is not computationally feasible.

Therefore SHA-1 is not being relied upon to protect the password. It is simply used as a message digest and I see no reason to propose its replacement.

The double hash in the SRP password verifier appears to be a Firebird local choice and I not sure why this was done.

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

> As long as users upgrade their server software first and then their clients then they will not have an issue.
Imagine that in a week after server upgrade with 99% of clients upgraded one needs to return back to previous server version due to server bug (we try to avoid such cases in point releases but shit happens sometimes) found... Next some clients may need to attach to >1 server, and our recommendation for such cases has always been 'use freshmost client'.
I have to say that breaking (moreover - hard breaking) backward compatibility in point release is not an option.

> On the other hand, if you retain a legacy SRP client in place then it becomes all too easy for a client to be mis-configured to use the legacy client rather than the updated version and as there is no server logging mechanism for a DBA to monitor whether clients are using SHA-256, there is no easy means to ensure that clients are up-to-date.

I can't agree that there are absolutely no ways to check who is using old auth plugin, for example:
SQL> select MON$USER, MON$REMOTE_HOST, MON$AUTH_METHOD from mon$attachments;

MON$USER SYSDBA
MON$REMOTE_HOST fbs3
MON$AUTH_METHOD Srp

DBA who really needs to avoid old plugin can just remove it from firebird.conf.

> Note that there is no proposed change to the SRP User Manager ...
Very good - 2 different AuthServer plugins will share same security database and manager for it, no problems with this. Our code is 100% ready for it.

@firebird-automations
Copy link
Collaborator Author

Commented by: Tony Whyman (twhyman)

>Very good - 2 different AuthServer plugins will share same security database and manager for it, no problems with this. Our code is 100% ready for it.
Actually, there is no need for two AuthServer plugins. The patch creates a single AuthServer plugin that can verify either a SHA-256 client proof or a SHA-1 client proof, with the length of the client proof used to determine which it is. This is compliant with NIST guidelines that still permit use of SHA-1 for legacy verification use.

The issue is over whether there is one or two AuthClient plugins: one for SHA-1 and one for SHA-256. If you look at the patch, there is a single line change to the code between the original SHA-1 version and the SHA-256 version (+selecting a different header file). If would be easy enough to have common source code for both plugins with an ifdef used to select which client proof is generated.

As to having two AuthClient plugins - one for legacy SHA-1 and one for SHA-256 - I am not going to object that strongly. It does however have to be carefully documented so that DBA's can configure their clients properly. The one thing I would strongly object to is a scenario where a client first tried SHA-256 and then SHA-1 if that did not work. This is because this opens up a well known attack strategy where an attacker deliberately blocks the "good" SHA-256 exchange in order to solicit the "weak" SHA-1 exchange.

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

> The one thing I would strongly object to is a scenario where a client first tried SHA-256 and then SHA-1 if that did not work. This is because this opens up a well known attack strategy where an attacker deliberately blocks the "good" SHA-256 exchange in order to solicit the "weak" SHA-1 exchange.

That's one of the reasons why there should not be universal SRP on server (even compliant with NIST guidelines) - only with correct server configuration admin may be sure that such scenario is not taking place. Trying to ensure something on client is not a way to go.

@firebird-automations
Copy link
Collaborator Author

Commented by: Tony Whyman (twhyman)

OK, its easy enough to have two servers and I suppose that a DBA can always configure use of both. Again, the documentation needs to be very clear if you want to do this.

@firebird-automations
Copy link
Collaborator Author

Commented by: Tony Whyman (twhyman)

Updated patch attached with separate AuthClients and AuthServers for SHA-1 and SHA-256 client proofs

I have updated the proposed patch to use template classes to avoid code duplication between the different plugins. This made it trivial to add clients and servers for the other SHA-2 hashes and hence it defines clients/servers for Srp (SHA-1), Srp224 (SHA-224), Srp256, Srp384 and Srp512.

The default server set is Srp256, Srp and the default client is Srp256. The default result is the same as for the earlier patch. The difference is that the DBA can now configure the combination of Srp client and server plugins that they deem suitable.

The patch also includes a proposed README file and updated http://Firebird.conf.in.

@firebird-automations
Copy link
Collaborator Author

Modified by: Tony Whyman (twhyman)

Attachment: srp_sha256_v2.patch [ 13235 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

Tony, just to make sure - is proposed SHA* implementation OK for non-intel CPUs?

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

Patch brings authentication subsystem into non-working state, login is impossible, i.e. appears it was never tested by author.

@firebird-automations
Copy link
Collaborator Author

Modified by: @AlexPeshkoff

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Incomplete [ 4 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: Tony Whyman (twhyman)

It was tested. Have you remembered to update the "firebird.conf" file as described in the document update?

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

It was new clean build and firebird.conf was default. Srp256 did not start at all (though was present in plugins list). So client had only Legacy_Auth loaded but server - only Srp, that's why no handshake.

@firebird-automations
Copy link
Collaborator Author

Commented by: Tony Whyman (twhyman)

I tested the patch with 3.0.3. Which build did you apply it to? There may be a conflict with another patch.

@firebird-automations
Copy link
Collaborator Author

Modified by: @pcisar

status: Resolved [ 5 ] => Closed [ 6 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

Alex,

patch works for me (while it was a puzzle of how to apply it).

SQL> show version;
ISQL Version: WI-V3.0.4.32987 Firebird 3.0
Server version:
Firebird/Windows/AMD/Intel/x64 (access method), version "WI-V3.0.4.32987 Firebird 3.0"
Firebird/Windows/AMD/Intel/x64 (remote server), version "WI-V3.0.4.32987 Firebird 3.0/tcp (Win7x64)/P15:C"
Firebird/Windows/AMD/Intel/x64 (remote interface), version "WI-V3.0.4.32987 Firebird 3.0/tcp (Win7x64)/P15:C"
on disk structure version 12.0

SQL> select * from mon$attachments where mon$system_flag = 0;

MON$ATTACHMENT_ID 759
MON$SERVER_PID 13160
MON$STATE 1
MON$ATTACHMENT_NAME S:\TEMP\A.30.FDB
MON$USER SYSDBA
MON$ROLE NONE
MON$REMOTE_PROTOCOL TCPv6
MON$REMOTE_ADDRESS ::1/52551
MON$REMOTE_PID 9548
MON$CHARACTER_SET_ID 0
MON$TIMESTAMP 2018-06-14 20:49:56.8840
MON$GARBAGE_COLLECTION 1
MON$REMOTE_PROCESS F:\FB2\fb30\temp\x64\Release\firebird\isql.exe
MON$STAT_ID 12
MON$CLIENT_VERSION WI-V3.0.4.32987 Firebird 3.0
MON$REMOTE_VERSION P15
MON$REMOTE_HOST win7x64
MON$REMOTE_OS_USER vlad
MON$AUTH_METHOD Srp256
MON$SYSTEM_FLAG 0

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

OK, I will try it once again.

@firebird-automations
Copy link
Collaborator Author

Modified by: @AlexPeshkoff

status: Closed [ 6 ] => Reopened [ 4 ]

resolution: Incomplete [ 4 ] =>

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

The reason of a bug was badly written function
+const char* RemotePassword::pluginName(const char * bits)
+{
+ std::string plg(plugName);
+ plg += bits;
+ return plg.c_str();
+}
You returned a pointer to on-stack object that is destroyed by stack of later calls.
(Suppose that did not work with Firebird::string therefore a 'fix' was to use std::string.)

After fixing this code patch works for me, I apply it with modified according to FB backward compatibility rules set of plugins.

@firebird-automations
Copy link
Collaborator Author

Commented by: @AlexPeshkoff

Status of Srp256 plugin in FB3 & FB4.

In master branch Srp256 (with enhanced security) becomes single default authentication plugin. That means that with default configuration clients earlier than FB 3.0.4 will be not able to attach to FB4. This should not be severe problem - hopefully most of clients will be upgraded when FB4 is released.

In B3_0_Release default plugin is old Srp - I've decided not to break compatibility with existing clients in point release. Hope that fits requirements of most users. People who need enhanced security should upgrade all clients to at least 3.0.4 and set
AuthServer=Srp256
in firebird.conf.

In all cases hashes stored in security database are fully compatible between Srp & Srp256, i.e. security.db does not require any upgrade.

@firebird-automations
Copy link
Collaborator Author

Modified by: @AlexPeshkoff

status: Reopened [ 4 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

Fix Version: 4.0 Beta 1 [ 10750 ]

Fix Version: 3.0.4 [ 10863 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: Tony Whyman (twhyman)

>>Tony, just to make sure - is proposed SHA* implementation OK for non-intel CPUs?

It should be. The original author of the SHA2 code has claimed "The implementation has been tested on numerous UNIX systems and on both little endian and big endian platforms."

I have only be able to verify this on intel platforms and hopefully someone else will confirm the above is true.

@firebird-automations
Copy link
Collaborator Author

Commented by: Tony Whyman (twhyman)

>>The reason of a bug was badly written function

>>You returned a pointer to on-stack object that is destroyed by stack of later calls.
>>(Suppose that did not work with Firebird::string therefore a 'fix' was to use std::string.)

I always preferred Pascal reference counted strings anyway ;)

@firebird-automations
Copy link
Collaborator Author

Modified by: @mrotteveel

Link: This issue block progress on JDBC536 [ JDBC536 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants