Ubuntu
openssl package

[fips] ntpq segfaults when attempting to use MD5 from FIPS-openssl library.

Bug #1884265 reported by Dariusz Gadomski
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ntp (Ubuntu)
Invalid
Undecided
Unassigned
Bionic
Fix Released
Undecided
Unassigned
openssl (Ubuntu)
Fix Released
Medium
Joy Latten
Bionic
Invalid
Medium
Unassigned

Bug Description

[Impact]
In FIPS mode on Bionic MD5 is semi-disabled causing some applications to segfault.

ntpq uses crypto hashes to authenticate its requests. By default it uses md5. However, when compiled with openssl it creates a lists of acceptable hashes from openssl that can be used.

This issue is only applicable in bionic and when using fips-openssl.

[Test Steps]
Test case:
sudo apt install ntp
ntpq -p
Segmentation fault (core dumped)

What happens there is ntpq wants to iterate all available digests (list_digest_names in ntpq.c). It uses EVP_MD_do_all_sorted for this task.

EVP_MD_do_all_sorted eventually runs openssl_add_all_digests_int in c_alld.c.
For FIPS mode it adds:
EVP_add_digest(EVP_md5());

What happens later in ntpq is (list_md_fn function inside ntpq.c):
ctx = EVP_MD_CTX_new();
EVP_DigestInit(ctx, EVP_get_digestbyname(name));
EVP_DigestFinal(ctx, digest, &digest_len);

First digest it gets is MD5, but while running EVP_DigestInit for it, it gets to this point (openssl/crypto/evp/digest.c EVP_DigestInit_ex):
#ifdef OPENSSL_FIPS
        if (FIPS_mode()) {
            if (!(type->flags & EVP_MD_FLAG_FIPS)
                && !(ctx->flags & EVP_MD_CTX_FLAG_NON_FIPS_ALLOW)) {
                EVPerr(EVP_F_EVP_DIGESTINIT_EX, EVP_R_DISABLED_FOR_FIPS);
                return 0;
            }
        }
#endif

Due to type->flags for MD5 being 0 there's an error set (EVP_R_DISABLED_FOR_FIPS).
After getting back to ntpq.c:
ctx->engine and ctx->digest are not set (due to the mentioned error), hence

inside EVP_DigestFinal_ex (openssl/crypto/evp/digest.c)
OPENSSL_assert(ctx->digest->md_size <= EVP_MAX_MD_SIZE);
causes a segfault (ctx->digest is NULL).

So either MD5 shouldn't be added in FIPS mode or it should have the EVP_MD_FLAG_FIPS to be properly initialized.

[Regression Potential]

I don't think this should regress ntpq + openssl from the Ubuntu archive.

Current archive ntpq + openssl behaviour:
openssl includes all message digests and hands ntpq a sorted digest-list.
ntpq doesn't check return from EVP_Digest(Init|Final) and assumes all is well and sticks all digests into its list regardless if it is working or not.

i.e.
ntpq> help keytype
function: set key type to use for authenticated requests, one of:
    MD4, MD5, RIPEMD160, SHA1, SHAKE128

If somehow openssl library is corrupted and sends back erroneous results, its possible the authentication will just not ever work.

Newly fixed archive ntpq + oenssl beahviour:
openssl includes all message digests and hands ntpq a sorted digest-list.
ntpq checks each one and includes each working digest. With a non-corrupted openssl, everything works fine and ntpq includes each into its list. Ends up with a list identical to the one above.

If somehow opensll library is corrupted and sends back erroneous results, ntpq will hopefully catch it by checking return code and include only those algos that appear to be working. Its possible authentication will work for ntpq.

The difference will be seen in ntpq + fips-openssl. ntpq will check return, and for fips-not-approved algos, return will indicate an error. So these algos will be skipped and ntpq will not include into its digest list. Resulting in a much shorter list of only fips-approved algos.

i.e.
ntpq> help keytype
function: set key type to use for authenticated requests, one of:
    SHA1, SHAKE128

Since md5 is ntpq's default auth algo, this will need to be changed to one of the above algos in the config files.
But I think it is somewhat understood that MD5 is bad in a FIPS environment.

See original description
Revision history for this message
Dariusz Gadomski (dgadomski) wrote :

FTR: EVP_add_digest(EVP_md5()); is not present in the Xenial build, hence there's no crash there.

tags: added: sts
Dariusz Gadomski (dgadomski)
Changed in openssl (Ubuntu Bionic):
importance: Undecided → Medium
information type: Public → Public Security
Revision history for this message
Dariusz Gadomski (dgadomski) wrote :

Changelog in bug #1553309 mentions "- debian/patches/openssl-1.0.2g-fips-md5-allow.patch: [PATCH 3/6] Allow md5 in fips mode."

I am however unaware of the context of this change (e.g. MD5 is not included here: [1])

[1] https://csrc.nist.gov/csrc/media/publications/fips/140/2/final/documents/fips1402annexa.pdf

Revision history for this message
Joy Latten (j-latten) wrote :

Investigating.

Dariusz Gadomski (dgadomski)
description: updated
Revision history for this message
Dariusz Gadomski (dgadomski) wrote :

@j-latten: please let me know if I can provide any help with this.

Revision history for this message
Joy Latten (j-latten) wrote :

It seems 2 things are happening to generate this issue

1.fips-openssl in bionic has md5 and md5_sha1 in fips digest list with explicit purpose of accommodating PRF use only in fips mode. But you must pass the flag, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW to successfully use them.

2. ntpq does not check return codes from EVP_ calls. It has,
    ctx = EVP_MD_CTX_new();
    EVP_DigestInit(ctx, EVP_get_digestbyname(name));
    EVP_DigestFinal(ctx, digest, &digest_len);
    EVP_MD_CTX_free(ctx);
    if (digest_len > (MAX_MAC_LEN - sizeof(keyid_t)))
        return;

EVP_DigestInit() would have returned 0 in this case indicating a failure.

Possible fixes:
1. in fips-libcrypto library remove md5 from fips digest list and keep md5_sha1 for PRF and mark as fips-allowed. Can still use md5 with EVP_MD_CTX_FLAG_NON_FIPS_ALLOW flag, but its just not in fips digest list.

Note: this fix can be put in fips-update ppa for availability. But, it may be a while before it is re-certified.

2. ntpq should check its return codes and do appropriate thing on error.

Revision history for this message
Joy Latten (j-latten) wrote :

Also, this is only applicable in bionic. Neither xenial nor focal experience this issue.

Revision history for this message
Joy Latten (j-latten) wrote :

I added return checks to ntpq code and this appears to solve the problem. Is it ok to make this an SRU?

Revision history for this message
Dariusz Gadomski (dgadomski) wrote :

Sure. Sounds good. Do you have it available in a ppa anywhere to give it a try?

Revision history for this message
Dariusz Gadomski (dgadomski) wrote :

Oh, I have found it: ppa:j-latten/joydevppa

Works perfectly. Thanks!

Revision history for this message
Joy Latten (j-latten) wrote :

Build log:
https://launchpad.net/~j-latten/+archive/ubuntu/joyppa/+build/19570468

Revision history for this message
Joy Latten (j-latten) wrote :

debdiff for bionic

Joy Latten (j-latten)
description: updated
Joy Latten (j-latten)
description: updated
Revision history for this message
Joy Latten (j-latten) wrote :

Testing:

There are no autopkgtests for ntp pkg and we do not run "make check" in the tests dir as part of the build. So, just in case it is applicable, I ran make check on my local build to ensure everything passes.

Revision history for this message
Joy Latten (j-latten) wrote :

Additional testing for ntpq authentication to ensure MD5 still works for ntpq in archive

NOTE: The shown testing is ntpq(with patch) + openssl from archive. To ensure all still works.
Testing with ntpq + fips-openssl was also done successfully.

VM-A (ntp server)

1. Edit /etc/ntp.keys to include,

1 SHA1 austintexas
2 MD5 cedarpark

2. Edit /etc/ntp.conf to include.

keys /etc/ntp.keys
trustedkey 2
controlkey 2
requestkey 2

3. restart ntp
sudo service ntp restart

VM-B (ntp client)

$ dpkg -l | grep ntp
ii ntp 1:4.2.8p10+dfsg-5ubuntu7.1+ppa1 amd64 Network Time Protocol daemon and utility programs

1. Edit /etc/ntp.keys to include,

1 SHA1 austintexas
2 MD5 cedarpark

2. Edit /etc/ntp.conf to include,
keys /etc/ntp.keys
server <VM-B ipaddress> key 2
trustedkey 2
controlkey 2
requestkey 2

3. I commented out all the "pool" entries in /etc/ntp.conf

4. restart ntp
sudo service ntp restart

On the client,

$ ntpq -c as

ind assid status conf reach auth condition last_event cnt
===========================================================
  1 46728 f014 yes yes ok reject reachable 1

Notice that "auth" is ok.

$ ntpq
ntpq> keytype
keytype is MD5 with 16 octet digests
ntpq> keyid 2
ntpq> ifstats
MD5 Password: <enter "cedarpark">
    interface name send
 # address/broadcast drop flag ttl mc received sent failed peers uptime
==============================================================================
  0 v6wildcard D 81 0 0 0 0 0 0 96
    [::]:123
  1 v4wildcard D 89 0 0 0 0 0 0 96
    0.0.0.0:123
  2 lo . 5 0 0 2 1 0 0 96
    127.0.0.1:123
  3 ens3 . 19 0 0 2 2 0 1 96
    192.168.122.105:123
  4 lo . 5 0 0 0 0 0 0 96
    [::1]:123
  5 ens3 . 11 0 0 0 0 0 0 96
    [fe80::5054:ff:fefe:b092%2]:123
ntpq>

Note: issuing "ifstats" requires authentication.

I also tested with SHA1 and it worked as well.

And last test on client,
ntpq -p

remote refid st t when poll reach delay offset jitter
==============================================================================
 192.168.122.106 204.11.201.12 3 u 56 64 7 1.541 2.723 0.826

Joy Latten (j-latten)
Changed in openssl (Ubuntu):
assignee: nobody → Joy Latten (j-latten)
Steve Beattie (sbeattie)
Changed in openssl (Ubuntu Bionic):
status: New → Confirmed
Changed in openssl (Ubuntu):
status: New → In Progress
Joy Latten (j-latten)
summary: - [fips] Not fully initialized digest segfaulting some client applications
+ [fips] ntpq segfaults when attempting to use MD5 from FIPS-openssl
+ library.
Joy Latten (j-latten)
description: updated
description: updated
Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

ACK on the debdiff in comment #11, uploaded with a slight LP tag fix for processing by the SRU team. Thanks!

Changed in openssl (Ubuntu Bionic):
status: Confirmed → In Progress
Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

Hello Dariusz, or anyone else affected,

Accepted ntp into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ntp/1:4.2.8p10+dfsg-5ubuntu7.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in openssl (Ubuntu):
status: In Progress → Fix Released
Changed in ntp (Ubuntu Bionic):
status: New → Fix Committed
tags: added: verification-needed verification-needed-bionic
Dariusz Gadomski (dgadomski)
tags: added: verification-done verification-done-bionic
removed: verification-needed verification-needed-bionic
Revision history for this message
Dariusz Gadomski (dgadomski) wrote :

I have verified it for Bionic using ntp 1:4.2.8p10+dfsg-5ubuntu7.2.

No segfault observed:
sudo ntpq -p
     remote refid st t when poll reach delay offset jitter
==============================================================================
 0.ubuntu.pool.n .POOL. 16 p - 64 0 0.000 0.000 0.000
 1.ubuntu.pool.n .POOL. 16 p - 64 0 0.000 0.000 0.000
 2.ubuntu.pool.n .POOL. 16 p - 64 0 0.000 0.000 0.000
 3.ubuntu.pool.n .POOL. 16 p - 64 0 0.000 0.000 0.000
 ntp.ubuntu.com .POOL. 16 p - 64 0 0.000 0.000 0.000
+tel50.oa.uj.edu 149.156.70.75 2 u 6 64 1 14.404 0.782 0.386
*SunSITE.icm.edu 210.100.177.101 2 u 5 64 1 12.239 0.138 0.645
(...)

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ntp - 1:4.2.8p10+dfsg-5ubuntu7.2

---------------
ntp (1:4.2.8p10+dfsg-5ubuntu7.2) bionic; urgency=medium

  * ntpq should check return code from libcrypto calls (LP: #1884265)
    - debian/patches/ntpq-openssl-check.patch

 -- Joy Latten <email address hidden> Thu, 09 Jul 2020 21:11:52 +0000

Changed in ntp (Ubuntu Bionic):
status: Fix Committed → Fix Released
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Update Released

The verification of the Stable Release Update for ntp has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Steve Beattie (sbeattie) wrote :

Closing ntp task for groovy.

Changed in ntp (Ubuntu):
status: New → Invalid
Changed in openssl (Ubuntu Bionic):
status: In Progress → Invalid
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.