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.

Sign up for GitHub

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

Jump to bottom

Fix/jenkins credentials #1029

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+142 −2
Open

Fix/jenkins credentials #1029

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
986ef4b
Hide credentials from log
zipkid Feb 25, 2022
87b6beb
Remove debug output exposing credentials to log.
zipkid Feb 28, 2022
7604748
Remove error handling here
zipkid Feb 28, 2022
4911282
Remove useless begin ... end block
zipkid Feb 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 152 additions & 0 deletions lib/puppet/type/jenkins_credentials.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,26 +51,140 @@

newproperty(:password) do
desc 'password - UsernamePasswordCredentialsImpl, CertificateCredentialsImpl'
def is_to_s(_current_value)
'[old password]'
end

def should_to_s(_new_value)
'[new password]'
end

def change_to_s(_current_value, _new_value)
begin
zipkid marked this conversation as resolved.
Show resolved Hide resolved
if _current_value == :absent
return "Added '#{name}' as #{should_to_s(_new_value)}"
elsif _new_value == :absent or _new_value == [:absent]
return "Removed '#{name}' from #{is_to_s(_current_value)}"
else
return "Changed '#{name}' from #{is_to_s(_current_value)} to #{should_to_s(_new_value)}"
end
end
end
end

newproperty(:private_key) do
desc 'ssh private key string - BasicSSHUserPrivateKey'
def is_to_s(_current_value)
'[old ssh private key string]'
end

def should_to_s(_new_value)
'[new ssh private key string]'
end

def change_to_s(_current_value, _new_value)
begin
if _current_value == :absent
return "Added '#{name}' as #{should_to_s(_new_value)}"
elsif _new_value == :absent or _new_value == [:absent]
return "Removed '#{name}' from #{is_to_s(_current_value)}"
else
return "Changed '#{name}' from #{is_to_s(_current_value)} to #{should_to_s(_new_value)}"
end
end
end
end

newproperty(:access_key) do
desc 'AWS access key - AWSCredentialsImpl, BrowserStackCredentials'
def is_to_s(_current_value)
'[old AWS access key]'
end

def should_to_s(_new_value)
'[new AWS access key]'
end

def change_to_s(_current_value, _new_value)
begin
if _current_value == :absent
return "Added '#{name}' as #{should_to_s(_new_value)}"
elsif _new_value == :absent or _new_value == [:absent]
return "Removed '#{name}' from #{is_to_s(_current_value)}"
else
return "Changed '#{name}' from #{is_to_s(_current_value)} to #{should_to_s(_new_value)}"
end
end
end
end

newproperty(:secret_key) do
desc 'AWS secret key - AWSCredentialsImpl'
def is_to_s(_current_value)
'[old AWS secret key]'
end

def should_to_s(_new_value)
'[new AWS secret key]'
end

def change_to_s(_current_value, _new_value)
begin
if _current_value == :absent
return "Added '#{name}' as #{should_to_s(_new_value)}"
elsif _new_value == :absent or _new_value == [:absent]
return "Removed '#{name}' from #{is_to_s(_current_value)}"
else
return "Changed '#{name}' from #{is_to_s(_current_value)} to #{should_to_s(_new_value)}"
end
end
end
end

newproperty(:passphrase) do
desc 'passphrase to unlock ssh private key - BasicSSHUserPrivateKey'
def is_to_s(_current_value)
'[old passphrase]'
end

def should_to_s(_new_value)
'[new passphrase]'
end

def change_to_s(_current_value, _new_value)
begin
if _current_value == :absent
return "Added '#{name}' as #{should_to_s(_new_value)}"
elsif _new_value == :absent or _new_value == [:absent]
return "Removed '#{name}' from #{is_to_s(_current_value)}"
else
return "Changed '#{name}' from #{is_to_s(_current_value)} to #{should_to_s(_new_value)}"
end
end
end
end

newproperty(:secret) do
desc 'secret string - StringCredentialsImpl'
def is_to_s(_current_value)
'[old secret string]'
end

def should_to_s(_new_value)
'[new secret string]'
end

def change_to_s(_current_value, _new_value)
begin
if _current_value == :absent
return "Added '#{name}' as #{should_to_s(_new_value)}"
elsif _new_value == :absent or _new_value == [:absent]
return "Removed '#{name}' from #{is_to_s(_current_value)}"
else
return "Changed '#{name}' from #{is_to_s(_current_value)} to #{should_to_s(_new_value)}"
end
end
end
end

newproperty(:file_name) do
Expand All @@ -79,10 +193,48 @@

newproperty(:content) do
desc 'content of file - FileCredentialsImpl, CertificateCredentialsImpl'
def is_to_s(_current_value)
'[old content]'
end

def should_to_s(_new_value)
'[new content]'
end

def change_to_s(_current_value, _new_value)
begin
if _current_value == :absent
return "Added '#{name}' as #{should_to_s(_new_value)}"
elsif _new_value == :absent or _new_value == [:absent]
return "Removed '#{name}' from #{is_to_s(_current_value)}"
else
return "Changed '#{name}' from #{is_to_s(_current_value)} to #{should_to_s(_new_value)}"
end
end
end
end

newproperty(:source) do
desc 'content of file - CertificateCredentialsImpl'
def is_to_s(_current_value)
'[old content]'
end

def should_to_s(_new_value)
'[new content]'
end

def change_to_s(_current_value, _new_value)
begin
if _current_value == :absent
return "Added '#{name}' as #{should_to_s(_new_value)}"
elsif _new_value == :absent or _new_value == [:absent]
return "Removed '#{name}' from #{is_to_s(_current_value)}"
else
return "Changed '#{name}' from #{is_to_s(_current_value)} to #{should_to_s(_new_value)}"
end
end
end
end

newproperty(:key_store_impl) do
Expand Down
8 changes: 6 additions & 2 deletions lib/puppet/x/jenkins/provider/cli.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,9 @@ def self.cli(command, options = {}, cli_pre_cmd = [])
false
end

Puppet.debug("#{sname} stdin:\n#{input}")
# This Puppet.debug shows the JSON with changed credentials in plain text.
# ToDo: Remove this debug output OR mask the credentials
# Puppet.debug("#{sname} stdin:\n#{input}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this contains credentials, you should note that it's written to a file just below and there's a chmod 644 rescue block, which makes those credentials world readable.

It looks like that is designed for the case where the jenkins user is not yet available. I wonder if that workaround should be removed and instead hard fail.

It should also be noted that there's no way to ensure the file really is removed. If execute_with_retry raises an exception it's not removed. That means in some cases the credentials could remain world readable until $TMPDIR is cleaned.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least this would stay on the server and not be shipped off to something like The Foreman where many more can see the credentials.


# a tempfile block arg is not used to simplify mock testing :/
tmp = Tempfile.open(sname)
Expand Down Expand Up @@ -210,7 +212,9 @@ def self.execute_with_retry(command, options = {}, cli_pre_cmd = [])
handler: handler
) do
result = execute_with_auth(cli_cmd, auth_cmd, options)
Puppet.debug("#{sname} command stdout:\n#{result}") unless result == ''
# This Puppet.debug shows the JSON with all credentials in plain text.
# ToDo: Remove this debug output OR mask the credentials
# Puppet.debug("#{sname} command stdout:\n#{result}") unless result == ''
return result
end
end
Expand Down