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

Implement Get Channel Cipher Suites command #51

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

dongx1x
Copy link
Contributor

@dongx1x dongx1x commented Aug 26, 2022

Implement the Get Channel Cipher Suites command and a function to get
proper Cipher Suite for authentication in new session. Currently
implementation prefer Cipher Suite 17 and fall back to Cipher Suite 3.

Reference: section 22.15, IPMI v2.0

Signed-off-by: Dong, Xiaocheng xiaocheng.dong@intel.com

Implement the `Get Channel Cipher Suites` command and a function to get
proper Cipher Suite for authentication in new session. Currently
implementation prefer Cipher Suite 17 and fall back to Cipher Suite 3.

Reference: section 22.15, IPMI v2.0

Signed-off-by: Dong, Xiaocheng <xiaocheng.dong@intel.com>
@dongx1x
Copy link
Contributor Author

dongx1x commented Aug 26, 2022

Hello @gebn, I implemented the Get Channel Cipher Suites command for gebn/bmc_exporter#63 and #50, please help review this patch, thanks!

@gebn gebn merged commit fc707b2 into gebn:master Aug 29, 2022
@gebn
Copy link
Owner

gebn commented Aug 29, 2022

Thanks @dongx1x! I've merged your commit and made a handful of wider changes on top - in particular a breaking API change to V2SessionOpts, which now takes CipherSuites []ipmi.CipherSuite, defaulting to 17 then 3 if nil. The previous behaviour was objectively broken, as Open Session only takes one of each payload. It will be a one-off pain for folks upgrading, but should make usage much more intuitive going forward.

/cc @lukeyeager - this should also free you from manually configuring cipher suite 17, do let me know if it doesn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants