This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 880279 - plexus-cipher: improper use of SecureRandom in PBECipher
plexus-cipher: improper use of SecureRandom in PBECipher
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: plexus-cipher (Show other bugs)
18
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Mikolaj Izdebski
Fedora Extras Quality Assurance
: Security
Depends On:
Blocks: 880257 880673
  Show dependency treegraph
 
Reported: 2012-11-26 10:50 EST by Florian Weimer
Modified: 2012-12-06 02:27 EST (History)
3 users (show)

See Also:
Fixed In Version: 1.5-11
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 880673 (view as bug list)
Environment:
Last Closed: 2012-12-06 01:59:53 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Proposed patch (2.89 KB, patch)
2012-11-27 09:44 EST, Mikolaj Izdebski
no flags Details | Diff

  None (edit)
Description Florian Weimer 2012-11-26 10:50:27 EST
Description of problem:

src/main/java/org/sonatype/plexus/components/cipher/PBECipher.java contains this code (slightly abbreviated):

    protected SecureRandom _secureRandom;


    public PBECipher() throws PlexusCipherException
    {
        try
        {
            _digester = MessageDigest.getInstance( DIGEST_ALG );
            if( System.getProperty( "os.name", "blah" ).toLowerCase().indexOf( "linux" ) != -1 )
                _onLinux = true;
            if( _onLinux )
                System.setProperty( "securerandom.source", "file:/dev/./urandom");
            else
                _secureRandom = new SecureRandom();
        }
        catch ( NoSuchAlgorithmException e )
        {
            throw new PlexusCipherException(e);
        }
    }

So the constructor sets a system property (bad), and does not initialize _secureRandom (worse).  Because of this, getSalt() falls back to Random (seeded by the current time) instead of SecureRandom:

    private byte[] getSalt( final int sz )
    throws NoSuchAlgorithmException, NoSuchProviderException
    {
        byte [] res = null;
        
        if( _secureRandom != null )
        {
            _secureRandom.setSeed( System.currentTimeMillis() );
            res = _secureRandom.generateSeed( sz );
        }
        else
        {
            res = new byte[ sz ];
            Random r = new Random( System.currentTimeMillis() );
            r.nextBytes( res );
        }

        return res;
    }

The if branch is not executed, but neither SecureRandom#setSeed(long) nor SecureRandom#generateSeed(int) are appropriate methods to call in this context.  You should allocate a byte array and call SecureRandom#nextBytes(byte[]).

getSalt() is called from here:

    public String encrypt64( final String clearText, final String password )
    throws PlexusCipherException
    {
        try
        {
            byte[] clearBytes = clearText.getBytes();
    
            byte[] salt = getSalt( SALT_SIZE );
            
            // spin it :)
            if( _secureRandom != null )
            {
                new SecureRandom().nextBytes( salt );
            }

Constructing a short-lived SecureRandom object is not a good idea because it is a very costly operation.

To fix this, no system properties should be examined or set, and a static SecureRandom instance should be used to generate the salts, using SecureRandom#nextBytes(byte[]).  This should simplify the code considerably.  The default SecureRandom implementation used to block on older/proprietary JDKs (and that's probably reason for this convoluted code), but that workaround should no longer be needed.

These bugs just decreases the randomness of the salt/IV, so they may not actually result in an exploitable security vulnerability.  But that depends on how this class is used.

Note that I haven't verified if this class actually implements a strong encryption scheme.

Version-Release number of selected component (if applicable):

plexus-cipher-1.5.8-fc18
Comment 1 Mikolaj Izdebski 2012-11-27 09:44:19 EST
Created attachment 652787 [details]
Proposed patch

I have replaced multiple generator objects with a single static SecureRandom which relies on Java to properly seed it. I removed OS detection as the new code should work properly on all OSes supported by Java. There should be no problems because of API/ABI changes. SecureRandom#nextBytes(byte[]) is thread safe.

Does this patch satisfy you?
Comment 2 Florian Weimer 2012-11-27 09:49:21 EST
(In reply to comment #1)
> Created attachment 652787 [details]
> Proposed patch
> 
> I have replaced multiple generator objects with a single static SecureRandom
> which relies on Java to properly seed it. I removed OS detection as the new
> code should work properly on all OSes supported by Java. There should be no
> problems because of API/ABI changes. SecureRandom#nextBytes(byte[]) is
> thread safe.
> 
> Does this patch satisfy you?

Looks good, thanks.
Comment 5 Mikolaj Izdebski 2012-11-27 10:10:12 EST
Fixed in plexus-cipher-1.5-11
Comment 6 Fedora Update System 2012-11-27 10:54:28 EST
plexus-cipher-1.5-11.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/plexus-cipher-1.5-11.fc16
Comment 7 Fedora Update System 2012-11-27 10:55:05 EST
plexus-cipher-1.5-11.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/plexus-cipher-1.5-11.fc17
Comment 8 Fedora Update System 2012-11-27 10:55:34 EST
plexus-cipher-1.5-11.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/plexus-cipher-1.5-11.fc18
Comment 9 Fedora Update System 2012-11-27 21:08:36 EST
Package plexus-cipher-1.5-11.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing plexus-cipher-1.5-11.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-19162/plexus-cipher-1.5-11.fc18
then log in and leave karma (feedback).
Comment 10 Fedora Update System 2012-12-06 01:59:56 EST
plexus-cipher-1.5-11.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 11 Fedora Update System 2012-12-06 02:05:31 EST
plexus-cipher-1.5-11.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 12 Fedora Update System 2012-12-06 02:21:53 EST
plexus-cipher-1.5-11.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.