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

ncm-ssh: disable insecure ssh ciphersuites and macs #1058

Merged
merged 7 commits into from
Dec 18, 2017
Merged

ncm-ssh: disable insecure ssh ciphersuites and macs #1058

merged 7 commits into from
Dec 18, 2017

Conversation

JensTimmerman
Copy link
Contributor

@JensTimmerman JensTimmerman commented Mar 1, 2017 •
edited
Loading

  • sha1, des, 3des and md5 should no longer be used!
  • This might break for people who are still using them, they should remove old ciphers, MAC's and key exchange algorithms.
  • also remove rc4, it should no longer be used since the Bar Mitzvah attack was published.

Replaces #983.

@JensTimmerman JensTimmerman mentioned this pull request Mar 1, 2017
Closed
stdweird
stdweird reviewed Mar 2, 2017
View reviewed changes
ncm-ssh/src/main/pan/components/ssh/schema.pan Outdated
@@ -10,18 +10,64 @@ include 'pan/types';

type ssh_preferred_authentication = string with match(SELF, '^(gssapi-with-mic|hostbased|publickey|keyboard-interactive|password)$');


type ssh_ciphers = string with is_valid_ssh_cipher(SELF);
Copy link
Member

@stdweird stdweird Mar 2, 2017 •
edited by jrha
Loading

Choose a reason for hiding this comment

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

you introduce a new type that already exists in metaconfig/ssh and it is not used in this template. make an issue to resolve this, and add a comment with the issue number here, and comment this type for now.
you can already check that the current definition in metaconfig /ssh is the same regexp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what exactly do I need to resolve?

This way of working was recommended to me by @wdpypere To keep it backwards compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did what now?

Copy link
Member

Choose a reason for hiding this comment

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

the types are unique. haveing this type here implies that you cannot include the metaconfig service that has the same type defined. there can be only one...

@jrha jrha added this to the 17.3 milestone Mar 2, 2017
@jrha jrha modified the milestones: 17.4, 17.3 Mar 27, 2017
@jrha
Copy link
Member

jrha commented Mar 27, 2017

Pushing back to 17.4 as we are trying to avoid user-facing changes in 17.3.

@jrha
Copy link
Member

jrha commented Jul 17, 2017

@JensTimmerman could you address @stdweird's comment please?

@stdweird
Copy link
Member

@JensTimmerman can you fix the typo in the title and prefix the commit message with ncm-ssh

@jrha jrha changed the title ncm-ssh: dissable insecure ssh ciphersuites and mac's ncm-ssh: disable insecure ssh ciphersuites and mac's Jul 25, 2017
@jrha jrha changed the title ncm-ssh: disable insecure ssh ciphersuites and mac's ncm-ssh: disable insecure ssh ciphersuites and macs Jul 25, 2017
@jrha
Copy link
Member

jrha commented Jul 25, 2017

@hpcugentbot retest this please

jrha
jrha previously approved these changes Jul 26, 2017
View reviewed changes
Copy link
Member

@jrha jrha left a comment

Choose a reason for hiding this comment

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

Travis test can be ignored, LGTM.

@JensTimmerman
Copy link
Contributor Author

@jrha I think I still need to address @stdweird's requests.

@jrha jrha dismissed their stale review July 27, 2017 13:17

comments left to be addressed

@jrha jrha modified the milestones: 17.8, 17.7 Aug 1, 2017
@jrha
Copy link
Member

jrha commented Aug 30, 2017

@JensTimmerman any news?

@jrha jrha modified the milestones: 17.8, 17.10 Sep 21, 2017
@jrha
Copy link
Member

jrha commented Oct 26, 2017

@JensTimmerman ping?

@JensTimmerman
Copy link
Contributor Author

@jrha @stdweird's comment isn't completely clear to me, so I'll await his clarification on what exactly needs to be resolved.

@jrha
Copy link
Member

jrha commented Oct 27, 2017

ok thanks

@jrha
Copy link
Member

jrha commented Nov 24, 2017 •
edited
Loading

@stdweird can you clarify?

@stdweird
Copy link
Member

@jrha i think it's clear now for @JensTimmerman he renamed the duplicate type names, to be removed in later PR when the types are part of template libary core

jrha
jrha approved these changes Dec 8, 2017
View reviewed changes
Copy link
Member

@jrha jrha left a comment

Choose a reason for hiding this comment

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

LGTM, will squash when merging.

@jrha jrha merged commit e583d6b into quattor:master Dec 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stdweird stdweird stdweird left review comments

@wdpypere wdpypere wdpypere left review comments

@jrha jrha jrha approved these changes

Assignees
No one assigned
Labels
backwards incompatible
Milestone
17.12
Development

Successfully merging this pull request may close these issues.
4 participants
@JensTimmerman @jrha @stdweird @wdpypere