Bug 782480

Summary:	Consumer security concerns
Product:	[Retired] Pulp	Reporter:	Jay Dobies <jason.dobies>
Component:	user-experience	Assignee:	Jeff Ortel <jortel>
Status:	CLOSED CURRENTRELEASE	QA Contact:	Preethi Thomas <pthomas>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	1.0.0	CC:	skarmark
Target Milestone:	---	Keywords:	Triaged
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2012-02-24 20:10:56 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:

Description Jay Dobies 2012-01-17 15:26:42 UTC

1. The register and retrieve certificate commands are separate calls. There's nothing to stop a rogue consumer from requesting the certificate for another consumer's ID. That would both give the ability to make calls on behalf of the compromised consumer as well as break the compromised consumer's ability to make calls since its certificate will be rendered invalid.

The fix is simple, the certificate should be returned on a successful register. There's no reason to split up those calls.

2. The combined certificate and private key are stored in the database. It's generally bad mojo to store private keys on the server. The typical usage is to return it to the caller and destroy it server-side.

The fix there is really simple. We store the concatenated cert and key in the database. We simply change it to only store the cert before the concatenation. The only potential hiccup is that we really should add a pulp-migrate script to run through and remove the keys from existing consumers and the parsing to split apart cert from key isn't fun.

Comment 1 Jeff Ortel 2012-01-18 23:37:20 UTC

Made the following changes:

- The Consumer.create() returns a consumer object where the "certificate" still contains both the private key and certificate (bundle).  However, the "certificate" only contains the certificate (not the key) when the object is stored in the DB.  I chose this approach instead of changing the return for better backward comparability in the REST API.

- Illuminated the Consumer.certificate() in all layers.

- Updated the agent shared secret function to only use the SHA256 of the certificate PEM.

- Updated the client register() to use the "certificate" contained in the returned Consumer object rather than making the follow up call to get the certificate.

- Added migrate script version:36 to migrate currently stored key and certificate bundles to only contain the certificate (key is stripped).

- All functionality uses the existing Bundle manipulation object in pulp.common.bundle.

- unit tests updated/expanded.

Comment 2 Jeff Ortel 2012-01-21 00:22:55 UTC

build: 0.259

Comment 3 Preethi Thomas 2012-01-25 20:32:15 UTC

verified
[root@preethi webservices]# rpm -q pulp
pulp-0.0.261-1.fc15.noarch


Type "help", "copyright", "credits" or "license" for more information.
>>> import pic
>>> pic.connect()
>>> pic.POST('/consumers/', { 'id':'elvis', 'description':'test'})
(201, {u'description': u'test', u'certificate': u'-----BEGIN RSA PRIVATE KEY-----\nMIICXQIBAAKBgQCstXZLMp15XBYIAT74o94JVwwprMAaMpga6O59oK2ggv/Z5TqF\nmqQ0v65oLF182oauzrGY1RRFvrTipbYWenU4wIkYXWnCcJIpFk6kFD0wGIqV43BD\nC/NqM0fbij3NXv7EoozGJljHZEDTWj8UUlAX0anC49iV1ZKBVuu7uEO7/QIDAQAB\nAoGBAI7RF9sjJdlfbtB7x0jwqQFsPCCSO+DuCZ3nFKBKKIndChlzVyt4L2V3RI/c\ncAp44nrXbUEGotbx1r69bY+1AAzsRT7ClnN2caZExzfGCo5w+nS2YXjLI7xldjx3\n97TN/eA1XsH3MXzV5LJvUA41hN6lOoGJEk/+gbF3L8QbUPPBAkEA1pWklLvktGrE\n2amUu9bH1KhtM8VLg0xAtEzHjon4pb/65X8zikwAhYa5n4ze1IfEdHVvgrsVPNzK\nvDj1vsLIeQJBAM4KytMrbuRnkyRUR+bboU1a5mOMUrCy12j+TMy86AkrEfCt5CHz\n1c88NeIGH8+tm3LG8qq4xILSQ460HkQnNqUCQGuNdYBW7LrBCQlPxgygCmi8Qn/A\nU6jrf0LfeOYooUfygX6l0t9uWJSUglVF9inwIrd8ZPfRbUOkJrlQk1uZpYECQG1N\nEFeBfOwxfb8R7qqq7CCrDfjVIbCWzurlrDwYIkdqz7OLq6/POCcdW8AxW4LCJ+p1\nW5nxNl3nyOrU5hFlc/kCQQCai+KxfIR7I8Cof6xsWBlPUuWecJbmRN6O9lGi8iCX\nM33wEXuWEYVrtRq5pa+BDRnrSAloYbNrboR1gRE2azo9\n-----END RSA PRIVATE KEY-----\n-----BEGIN CERTIFICATE-----\nMIICEzCB/AIBAjANBgkqhkiG9w0BAQUFADAUMRIwEAYDVQQDEwlsb2NhbGhvc3Qw\nHhcNMTIwMTI1MjAzMjMxWhcNMjIwMTIyMjAzMjMxWjAQMQ4wDAYDVQQDEwVlbHZp\nczCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEArLV2SzKdeVwWCAE++KPeCVcM\nKazAGjKYGujufaCtoIL/2eU6hZqkNL+uaCxdfNqGrs6xmNUURb604qW2Fnp1OMCJ\nGF1pwnCSKRZOpBQ9MBiKleNwQwvzajNH24o9zV7+xKKMxiZYx2RA01o/FFJQF9Gp\nwuPYldWSgVbru7hDu/0CAwEAATANBgkqhkiG9w0BAQUFAAOCAQEAmhwyQuWgtht8\nGDCdYZL2w485cf2a7BUp345R1uGDl74ppEdmyDNyTGv4dDmqBITVI1qZjBtsUrDV\nz1Ss6f3qQeKOr03Af82ZqIXzhuprpOsJsq6rKcseAvZGRi7Wu3er8iS1L8aeau72\nbqLmlv9xSrZklgs3dllmvwrROjRygNr1jcNTpuXOp4AdD39uea5+LEQrc0IUO4eu\ndPNtxKHV6XGc9HncI6S6gHEPvUNlbnvqEhEE3ey5iGH5kC1y3XJ7rOcd1VZwYG1c\nNZAZzfH6XyVQ80xXMt27PoVH/MVfYO/CYiecH5dY+DLfTRZ5eH6Kw9L9PHfCo2Wa\ndHSWjBxL0g==\n-----END CERTIFICATE-----', u'_ns': u'consumers', u'package_profile': [], u'capabilities': {}, u'key_value_pairs': {}, u'_id': u'elvis', u'id': u'elvis', u'repoids': []})
>>> 
>>> 
>>> pic.GET('/consumers/elvis/certificate')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pic.py", line 108, in GET
    return _request('GET', path)
  File "pic.py", line 95, in _request
    (response.status, response_body))
pic.RequestError: Server response: 404
not found
>>>

Comment 4 Jay Dobies 2012-01-25 20:39:33 UTC

For what it's worth, the verification should also check the database directly to make sure the private keys aren't stored.

Comment 5 Preethi Thomas 2012-02-24 20:10:56 UTC

Pulp v1.0 is released
Closed Current Release.