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

EOS: Hides MACsec Key #2014

Merged
merged 8 commits into from
Jan 23, 2020
Merged

EOS: Hides MACsec Key #2014

merged 8 commits into from
Jan 23, 2020

Conversation

krisamundson
Copy link
Contributor

@krisamundson krisamundson commented Jan 14, 2020

Pre-Request Checklist

  • Passes rubocop code analysis (try rubocop --auto-correct)
  • Tests added or adapted (try rake test)
  • Changes are reflected in the documentation
  • User-visible changes appended to CHANGELOG.md

Description

Hides macsec secret key: the CKN and CAK.

  • Original Syntax Example
    mac security
       fips restrictions
       !
       profile default
          cipher aes256-gcm-xpn
          key 1ffab311 7 00C655A8E47F0E4756B32767F7C9A34854716C7F33F2E948A39CA9C00D0D0A0330
          mka session rekey-period 3600
          l2-protocol lldp bypass
  • Replacement Syntax Example
    mac security
       fips restrictions
       !
       profile default
          cipher aes256-gcm-xpn
          key <secret hidden>
          mka session rekey-period 3600
          l2-protocol lldp bypass

@codecov-io
Copy link

codecov-io commented Jan 14, 2020

Codecov Report

Merging #2014 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2014   +/-   ##
=======================================
  Coverage   63.32%   63.32%           
=======================================
  Files          30       30           
  Lines        1497     1497           
=======================================
  Hits          948      948           
  Misses        549      549

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da23915...d783293. Read the comment docs.

@krisamundson krisamundson requested a review from ytti January 14, 2020 18:32
@ytti
Copy link
Owner

ytti commented Jan 14, 2020

The change looks fine to me, I'm not too interested in secret hiding, I think it's antiapattern but people expect it because rancid did it, so we have it. So looks OK to me. But we do not want to tie this with release of new version so please just change the change log for master and keep version as is.

We will build docker container nontheless for every commit and those docker container version strings will include the commit hash.

Kris Amundson and others added 3 commits January 19, 2020 18:55
* Hides macsec secret key: the CKN and CAK.

* Original Syntax Example

    mac security
       fips restrictions
       !
       profile default
          cipher aes256-gcm-xpn
          key 1ffab311 7 00C655A8E47F0E4756B32767F7C9A34854716C7F33F2E948A39CA9C00D0D0A0330
          mka session rekey-period 3600
          l2-protocol lldp bypass

* Replacement Syntax Example

    mac security
       fips restrictions
       !
       profile default
          cipher aes256-gcm-xpn
          key <secret hidden>
          mka session rekey-period 3600
          l2-protocol lldp bypass
@wk
Copy link
Contributor

wk commented Jan 19, 2020

I've reverted the version.rb changes and updated CHANGELOG.md to make this ready to pull in.

@krisamundson is there any reason why the key is matched as \S and not as \h, seeing how it appears to be HEX? Are there other improvements that can be made to constrain the regexp to make sure it never hits a false positive?

@wk
Copy link
Contributor

wk commented Jan 22, 2020

This looks good to me now, let me know if you are happy with this change set and I'll merge it in!

@krisamundson
Copy link
Contributor Author

+1 from me. Thanks for the quick turnaround and improving my regex-fu.

@wk wk merged commit 29fbe3a into ytti:master Jan 23, 2020
@krisamundson krisamundson deleted the eos_macsec_secret branch January 23, 2020 18:32
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.

4 participants