x509_certificate and key_rsa resource #1567

chris-rock · 2017-03-15T22:35:37Z

This PR fixes #1459

Now you can define tests like:


describe x509_certificate('trials/x509/cert.pem') do
  it { should be_certificate }
  it { should be_valid }
  its('fingerprint') { should eq '62b137bdf427e7273dc2e487877b3033e4c8ce17' }
  its('signature_algorithm') { should eq 'sha1WithRSAEncryption' }
  its('validity_in_days') { should_not be < 100 }
  its('validity_in_days') { should be >= 100 }
  its('subject_dn') { should eq '/C=DE/ST=Berlin/L=Berlin/O=InSpec/OU=Chef Software, Inc/CN=inspec.io/emailAddress=support@chef.io' }
  its('subject.C') { should eq 'DE' }
  its('subject.emailAddress') { should_not be_empty }
  its('subject.emailAddress') { should eq 'support@chef.io' }
  its('issuer_dn') { should eq '/C=DE/ST=Berlin/L=Berlin/O=InSpec/OU=Chef Software, Inc/CN=inspec.io/emailAddress=support@chef.io' }
  its('key_length') { should be >= 2048 }
  its('extensions.subjectKeyIdentifier') { should cmp 'A5:16:0B:12:F4:48:0F:06:6C:32:29:67:98:12:DF:3D:0D:75:9D:5C' }
end

describe x509_certificate('trials/x509/x505.md') do
  it { should_not be_certificate }
end

describe key_rsa('trials/x509/key.pem', 'passphrase') do
  it { should be_private }
  it { should be_public }
end

which will result in:

$ inspec exec trials/x509.rb

Profile: tests from trials/x509.rb
Version: (not specified)
Target:  local://


  x509_certificate trials/x509/cert.pem
     ✔  should be certificate
     ✔  should be valid
     ✔  fingerprint should eq "62b137bdf427e7273dc2e487877b3033e4c8ce17"
     ✔  signature_algorithm should eq "sha1WithRSAEncryption"
     ✔  validity_in_days should not be < 100
     ✔  validity_in_days should be >= 100
     ✔  subject_dn should eq "/C=DE/ST=Berlin/L=Berlin/O=InSpec/OU=Chef Software, Inc/CN=inspec.io/emailAddress=support@chef.io"
     ✔  subject.C should eq "DE"
     ✔  subject.emailAddress should not be empty
     ✔  subject.emailAddress should eq "support@chef.io"
     ✔  issuer_dn should eq "/C=DE/ST=Berlin/L=Berlin/O=InSpec/OU=Chef Software, Inc/CN=inspec.io/emailAddress=support@chef.io"
     ✔  key_length should be >= 2048
     ✔  extensions.subjectKeyIdentifier should cmp == "A5:16:0B:12:F4:48:0F:06:6C:32:29:67:98:12:DF:3D:0D:75:9D:5C"
  x509_certificate trials/x509/x505.md
     ✔  should not be certificate
  rsa_key trials/x509/key.pem
     ✔  should be private
     ✔  should be public

Test Summary: 16 successful, 0 failures, 0 skipped

TODO:

unit tests
integration tests

aaronlippold · 2017-03-16T01:09:13Z

lib/resources/key_rsa.rb

+    def private?
+      return if @key.nil?
+      @key.private?
+    end


So are we saying that a certificate is valid if it is public? I think we may need to refine what we mean by 'valid' just so it is clear. In the DoD sense, a key is 'valid' is it was issues and signed by an approved known trusted CA - i.e. one of the valid root or intermediate authorities.

Certificates check the following: certificate? && (now >= not_before && now <= not_after)

aaronlippold · 2017-03-16T01:18:00Z

lib/resources/x509.rb

+      !@cert.nil?
+    end
+
+    # @see https://tools.ietf.org/html/rfc5280#page-23


Small thing, If possible I would like to be able to say, its(... OU, CN, O, L, or CN ) { should match *DOD*}

aaronlippold · 2017-03-16T01:19:10Z

lib/resources/key_rsa.rb

+    def private?
+      return if @key.nil?
+      @key.private?
+    end


http://iasecontent.disa.mil/pki-pke/Certificates_PKCS7_v5.0u1_DoD.zip has examples of valid DoD certs which should give a good idea about the things we would scan for.

Would be good to verify what specifically needs to be done to use that.

aaronlippold

Overall I think this looks good, I added a couple of comments and a few examples of things that I may be able to make a bit more clear - i.e. what does valid mean? Valid may be a combination of things, signed by and issued by an organization of name x or be signed with a signature that matches a specific email. I added a link to the PKCS#7 dod public certs so you can look at them to see how the three certs are connected to each other - i.e. through issuer and subject - and that may clarify that we want to make 'valid' a different keyword for the core test. The only real question I have is what does 'valid' test?

aaronlippold · 2017-03-16T01:42:31Z

One part that I would also have is the ability to check a certificates md5,sha-1, and she-256 fingerprints. That would be another way to validate that the cert is the real thing.

DOD has published locations where you can get fingerprint lists of released and issued certs.

chris-rock · 2017-03-16T09:17:06Z

As discussed with @trickyearlobe I am going to merge his efforts with my efforts

chris-rock · 2017-03-21T18:19:18Z

I refactored my implementation to use the base implementation from @trickyearlobe

chris-rock · 2017-03-21T18:37:41Z

cc @DeltaWhy for a review request

adamleff

This looks really awesome, @chris-rock and @trickyearlobe. I have some comments/questions I've left in my review. I'll be happy to re-review when ready.

adamleff · 2017-03-21T21:56:24Z

docs/resources/key_rsa.md

+
+### private_key (String)
+
+The `provate_key` property returns the private key or the RSA key pair.


Spelling error: private_key instead of provate_key

Also should have an example like the other methods for consistency.

Thank you. I agree.

adamleff · 2017-03-21T21:58:41Z

docs/resources/x509_certificate.md

+      its('issuer_cn') { should match "CN=Acme Trust CA" }
+    end
+
+### parsed.XX


Should be issuer.XX?

correct :-)

adamleff · 2017-03-21T21:59:58Z

lib/resources/x509_certificate.rb

+    example "
+      describe x509_certificate('/etc/pki/www.mywebsite.com.pem') do
+        its('subject') { should match /CN=My Website/ }
+        its('days_remaining') { should be > 30 }


validity_in_days? I don't see a days_remaining method.

adamleff · 2017-03-21T22:00:09Z

docs/resources/x509_certificate.md

+An `x509_certificate` resource block declares a certificate `key file` to be tested.
+
+    describe x509_certificate('mycertificate.pem') do
+      its('days_remaining') { should be > 30 }


validity_in_days? I don't see a days_remaining method.

Oh yeah, it is validity_in_days

adamleff · 2017-03-21T22:00:45Z

docs/resources/x509_certificate.md

+
+### serial (Integer)
+
+The `serial` property exposes the serial number of the certificate. The serial number is set by the CA during the signing process and should be unique within that CA.


Let's include an example for consistency with the rest of the documentation.

adamleff · 2017-03-21T22:01:58Z

docs/resources/x509_certificate.md

+      its('extensions').length) {should eq 3 }
+
+      # Check what extension categories we have
+      its('extensions) { should include 'keyUsage' }


Missing closing ' on each of the remaining lines in the its argument.

adamleff · 2017-03-21T22:02:57Z

docs/resources/x509_certificate.md

+      its('extensions) { should include 'subjectAltName' }
+
+      # Check examples of basic 'keyUsage'
+      its('extensions['keyUsage']) { should include "Digital Signature" }


Any particular reason you're using hash notation instead of method/dot notation for these examples?

nope, will change it

adamleff · 2017-03-21T22:04:39Z

lib/resources/key_rsa.rb

+      @key = nil
+      @passphrase = passphrase
+
+      return skip_resource 'Unable to find private key' unless @key_file.exist?


Since a provided key could be public, should this instead say:

"Unable to find key file #{@key_path}"

adamleff · 2017-03-21T22:07:30Z

lib/resources/key_rsa.rb

+      begin
+        @key = OpenSSL::PKey.read(@key_file.content, @passphrase)
+      rescue OpenSSL::PKey::RSAError => _
+        return skip_resource 'Unable to load private key'


Same comment as above re: public vs. private keys.

adamleff · 2017-03-21T22:08:55Z

lib/resources/x509_certificate.rb

+      @extensions = nil
+
+      file = inspec.file(filename)
+      return if file.nil? || !file.exist? || file.content.nil?


We skip_resource in the key_rsa resource if the file doesn't exist or is nil. We should probably do the same here, yes?

Great idea @adamleff

chris-rock · 2017-03-22T08:18:02Z

Berksfile

@@ -5,3 +5,4 @@ cookbook 'apt'
 cookbook 'os_prepare', path: './test/cookbooks/os_prepare'
 cookbook 'runit', github: 'hw-cookbooks/runit'
 cookbook 'ssh-hardening', git: 'https://github.com/dev-sec/chef-ssh-hardening.git'
+cookbook 'openssl', '~> 6'


Switch to openssl cookbook 7.x since chef-boneyard/openssl#54 is solved now

chris-rock · 2017-03-22T08:21:23Z

docs/resources/key_rsa.md

+
+### private_key (String)
+
+The `provate_key` property returns the private key or the RSA key pair.


Thank you. I agree.

chris-rock · 2017-03-22T08:23:02Z

docs/resources/x509_certificate.md

+An `x509_certificate` resource block declares a certificate `key file` to be tested.
+
+    describe x509_certificate('mycertificate.pem') do
+      its('days_remaining') { should be > 30 }


Oh yeah, it is validity_in_days

chris-rock · 2017-03-22T08:23:31Z

docs/resources/x509_certificate.md

+      its('issuer_cn') { should match "CN=Acme Trust CA" }
+    end
+
+### parsed.XX


correct :-)

chris-rock · 2017-03-22T08:35:50Z

docs/resources/x509_certificate.md

+      its('extensions) { should include 'subjectAltName' }
+
+      # Check examples of basic 'keyUsage'
+      its('extensions['keyUsage']) { should include "Digital Signature" }


nope, will change it

chris-rock · 2017-03-22T08:40:21Z

lib/resources/x509_certificate.rb

+      @extensions = nil
+
+      file = inspec.file(filename)
+      return if file.nil? || !file.exist? || file.content.nil?


Great idea @adamleff

* Includes unit tests * Includes 2 new resources * Includes documentation Signed-off-by: Richard Nixon <richard.nixon@btinternet.com>

chris-rock · 2017-03-22T09:02:38Z

@adamleff I've addressed all your feedback. Let me know if there is anything else I can do.

Signed-off-by: Christoph Hartmann <chris@lollyrock.com>

adamleff

This is wonderful! I'm super-excited for these new resources. Excellent work.

chris-rock added the in progress label Mar 15, 2017

chris-rock requested review from arlimus, adamleff and aaronlippold March 15, 2017 22:36

aaronlippold reviewed Mar 16, 2017

View reviewed changes

aaronlippold suggested changes Mar 16, 2017

View reviewed changes

chris-rock force-pushed the chris-rock/x509 branch 2 times, most recently from dbed14e to 9152908 Compare March 21, 2017 18:18

chris-rock force-pushed the chris-rock/x509 branch 2 times, most recently from 1c52b29 to 7f16ea5 Compare March 21, 2017 18:32

chris-rock removed the request for review from arlimus March 21, 2017 18:36

chris-rock changed the title ~~WIP: x509_certificate and key_rsa resource~~ x509_certificate and key_rsa resource Mar 21, 2017

chris-rock force-pushed the chris-rock/x509 branch 2 times, most recently from ac78b87 to a7561ab Compare March 21, 2017 21:20

adamleff suggested changes Mar 21, 2017

View reviewed changes

chris-rock commented Mar 22, 2017

View reviewed changes

Initial support for x509_certificate and rsa_key

f66f0b3

* Includes unit tests * Includes 2 new resources * Includes documentation Signed-off-by: Richard Nixon <richard.nixon@btinternet.com>

chris-rock force-pushed the chris-rock/x509 branch from a7561ab to 9f09410 Compare March 22, 2017 09:01

chris-rock force-pushed the chris-rock/x509 branch 3 times, most recently from 9dcc3a1 to 84a3e3d Compare March 22, 2017 10:17

chris-rock added 2 commits March 22, 2017 11:44

refactor x509 resources and rsa key

d2f000e

Signed-off-by: Christoph Hartmann <chris@lollyrock.com>

x509 integration tests

a96059a

Signed-off-by: Christoph Hartmann <chris@lollyrock.com>

chris-rock force-pushed the chris-rock/x509 branch from 84a3e3d to a96059a Compare March 22, 2017 10:44

adamleff approved these changes Mar 22, 2017

View reviewed changes

adamleff merged commit 730de65 into master Mar 22, 2017

adamleff deleted the chris-rock/x509 branch March 22, 2017 11:30

adamleff removed the in progress label Mar 22, 2017


		### private_key (String)

		The `provate_key` property returns the private key or the RSA key pair.


		### serial (Integer)

		The `serial` property exposes the serial number of the certificate. The serial number is set by the CA during the signing process and should be unique within that CA.

x509_certificate and key_rsa resource #1567

x509_certificate and key_rsa resource #1567

Conversation

chris-rock commented Mar 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronlippold left a comment

Choose a reason for hiding this comment

aaronlippold commented Mar 16, 2017 • edited Loading

chris-rock commented Mar 16, 2017

chris-rock commented Mar 21, 2017

chris-rock commented Mar 21, 2017

adamleff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris-rock commented Mar 22, 2017

adamleff left a comment

Choose a reason for hiding this comment

chris-rock commented Mar 15, 2017 •

edited

Loading

aaronlippold commented Mar 16, 2017 •

edited

Loading