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.

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

Add "EncryptionStatus" to each volume on Windows #1238

Merged

Conversation

Nimesh-Msys
Copy link
Contributor

@Nimesh-Msys Nimesh-Msys commented Aug 31, 2018

  • Changes to fetch Encryption Status of a volume
  • Fixed cases to handle WMI Exceptions
  • Added test cases
  • Minor code cleanup
  • Fixes MSYS-894

Signed-off-by: Nimesh nimesh.patni@msystechnologies.com

Description

By using GetConversionStatus method of Win32_EncryptableVolume class, we may fetch encryption status of a volume

Issues Resolved

#1226

Check List

@Nimesh-Msys Nimesh-Msys requested a review from a team August 31, 2018 12:47
@Nimesh-Msys Nimesh-Msys force-pushed the nimesh/MSYS-894_add_encryption_status branch 6 times, most recently from 049ccca to 0b2fd1a Compare September 1, 2018 05:04
@kmf
Copy link

kmf commented Sep 1, 2018

Cool!

@Nimesh-Msys Nimesh-Msys force-pushed the nimesh/MSYS-894_add_encryption_status branch from 0b2fd1a to dd1243f Compare September 3, 2018 12:51
@Nimesh-Msys
Copy link
Contributor Author

@stuartpreston , I tried few changes in AppVeyor config to connect with required namespace
As the reference states: If you receive the message Interactive mode set then you were able to connect to the namespace
Build log is displaying the expected result still there are failing specs on appveyor.
Could you please let me know your views on this.

@Nimesh-Msys Nimesh-Msys force-pushed the nimesh/MSYS-894_add_encryption_status branch from dd1243f to 2e0cff4 Compare September 4, 2018 07:36
@stuartpreston
Copy link

@Nimesh-Msys Your approach looks correct and I like where this is going, so I'm I wondering if this is an Appveyor issue with their default build. Thoughts:

  1. Have you tried using a Windows Server 2016 builder?
  2. The path to the namespace may be case sensitive on some platforms (?) (root vs Root etc?)

@Nimesh-Msys
Copy link
Contributor Author

Nimesh-Msys commented Sep 4, 2018

@stuartpreston , Thank you for the above points.
I've already checked (2). Path to the namespace is case insensitive. Root\CIMV2 and root\cimv2 both works well.

Tried with option (1): build is failing as:

Build worker image not found: Windows Server 2016

Also tried to build on Windows Server 2012, getting the same error:

The specified namespace name 'Root\CIMV2\Security\MicrosoftVolumeEncryption' is not a valid namespace name or does not exist.

According to some blogs, in case we are facing 'OLE error 80004001', There are chances that WMI is broken, and it needs to be rebuild.

@stuartpreston
Copy link

Can you try with Visual Studio 2017 which should have everything required for Windows Server 2016? See https://www.appveyor.com/docs/build-environment/#choosing-image-for-your-builds

@Nimesh-Msys Nimesh-Msys force-pushed the nimesh/MSYS-894_add_encryption_status branch 5 times, most recently from a55a373 to d4c70e7 Compare September 4, 2018 13:45
@stuartpreston
Copy link

FWIW - here's the output of the specs from my Windows10 machine. In case we can't get this working in Appveyor.

C:\projects\chef\ohai [MsysTechnologiesllc-nimesh/MSYS-894_add_encryption_status]> bundle exec rspec .\spec\unit\plugins\windows\filesystem_spec.rb
Run options:
  include {:focus=>true}
  exclude {:requires_root=>true, :unix_only=>true}

All examples were filtered out; ignoring {:focus=>true}

Ohai::System Windows Filesystem Plugin
  WmiLite::Wmi
    When initialized with no arguments
      will create an instance with default namespace ('root/cimv2')
      able to access instance of Win32_LogicalDisk
      unable to access instance of Win32_EncryptableVolume
    When initialized with an argument: (Root\CIMV2)
      will create an instance of namespace
      able to access instance of Win32_LogicalDisk
      unable to access instance of Win32_EncryptableVolume
    When initialized with an argument: (Root\CIMV2\Security\MicrosoftVolumeEncryption)
      will create an instance of namespace
      unable to access instance of Win32_LogicalDisk
      able to access instance of Win32_EncryptableVolume
  #logical_disk_properties
    with no arguments
      raises an error
    with wmi object of root namespace
      collects local disks information in Mash
      mash will contain expected information
    with wmi object of encryption namespace
      raises an error
  #encryptable_volume_properties
    with no arguments
      raises an error
    when Mash is empty
      and wmi object of root namespace
        raises an error
      and wmi object of encryption namespace
        raises an error
    when Mash is pre-filled with logical_disk_properties
      and wmi object of root namespace
        raises an error
      and wmi object of encryption namespace
        collects local disks information in Mash
        mash will contain expected information

Finished in 0.40645 seconds (files took 2.15 seconds to load)
19 examples, 0 failures

C:\projects\chef\ohai [MsysTechnologiesllc-nimesh/MSYS-894_add_encryption_status]> bundle exec ohai filesystem{
  "C:": {
    "kb_size": 511463911,
    "kb_available": 169724002,
    "kb_used": 341739909,
    "percent_used": 66,
    "mount": "C:",
    "fs_type": "ntfs",
    "volume_name": "",
    "encryption_status": "FullyEncrypted"
  }
}
C:\projects\chef\ohai [MsysTechnologiesllc-nimesh/MSYS-894_add_encryption_status]>

@Nimesh-Msys
Copy link
Contributor Author

Yes, It's working properly on my Win10 machine too. 👍

@Nimesh-Msys Nimesh-Msys force-pushed the nimesh/MSYS-894_add_encryption_status branch 5 times, most recently from b86724c to 253403b Compare September 5, 2018 09:25
@Nimesh-Msys Nimesh-Msys changed the title Add "EncryptionStatus" to each volume on Windows [WIP]Add "EncryptionStatus" to each volume on Windows Sep 6, 2018
@Nimesh-Msys Nimesh-Msys force-pushed the nimesh/MSYS-894_add_encryption_status branch from eeb2715 to 318cff4 Compare September 6, 2018 20:17
- Changes to fetch Encryption Status of a volume using GetConversionStatus method of Win32_EncryptableVolume class
- Handle wmi exceptions when a namespace or class is not accessible
- Added test cases
- Minor code fixes and cleanup
- Fixes MSYS-894

Signed-off-by: Nimesh <nimesh.patni@msystechnologies.com>
@Nimesh-Msys Nimesh-Msys force-pushed the nimesh/MSYS-894_add_encryption_status branch from 318cff4 to 489c34b Compare September 7, 2018 20:30
@Nimesh-Msys Nimesh-Msys changed the title [WIP]Add "EncryptionStatus" to each volume on Windows Add "EncryptionStatus" to each volume on Windows Sep 7, 2018
Copy link

@stuartpreston stuartpreston left a comment

Choose a reason for hiding this comment

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

LGTM, woud like @tas50 to approve for merge

@tas50 tas50 merged commit 7a5b2fc into chef:master Sep 9, 2018
@tas50
Copy link
Contributor

tas50 commented Sep 9, 2018

@Nimesh-Msys Thanks for breaking up the plugin and adding tests. This is in a much more maintainable state now.

@Nimesh-Msys
Copy link
Contributor Author

Thank you :)

@lock
Copy link

lock bot commented Jan 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants