This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours

Bug 880279

Summary: plexus-cipher: improper use of SecureRandom in PBECipher
Product: [Fedora] Fedora Reporter: Florian Weimer <fweimer>
Component: plexus-cipherAssignee: Mikolaj Izdebski <mizdebsk>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 18CC: huwang, jcapik, mizdebsk
Target Milestone: ---Keywords: Security
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
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:
Bug Depends On:    
Bug Blocks: 880257, 880673    
Attachments:
Description Flags
Proposed patch none

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.