Created attachment 510067 [details]
Proposed pre initscript keyhandling patch from Kevan Carstensen (the reporter)



Preliminary patch proposal.

Created attachment 510068 [details]
Proposed pre mkdumprd keyhandling patch from Kevan Carstensen (the reporter)





Preliminary patch proposal.

Americo,

  also would it be possible you to let us know your opinion regarding proposed
patches:
[1] https://bugzilla.redhat.com/show_bug.cgi?id=716439#c7 and
[2] https://bugzilla.redhat.com/show_bug.cgi?id=716439#c8

Thank you, Jan.

(In reply to comment #12)
Jan, generally both patches look good, but one problem is that, can we have a default value for "sshkey"? Because it is a new config which users may not notice.

Thanks.

(In reply to comment #11)
> > > "True, kdump wants to submit the core _without_ interactions, but, it _does_
> > > interactions when you run 'service kdump propagate' before 'service kdump
> > > start' which is required when you use ssh dump."
> > > 
> > > So the issue you are experiencing may be result of improper using of the kdump
> > > service. The points we are trying to solve with them right now is if:
> > > i)  this is properly documented somewhere,
> > > ii) if it would be possible to reject the attempt to send the core file in
> > >     the scenario, when "service kdump start" was called, but "service kdump
> > >     propagate" was not called earlier.


I believe you would see a failure if you didn't run "service kdump propagate" first.

> I read this as "the core file will not be sent to an attacker
> impersonating the kdump target if kdump propagate is run before kdump
> start". Is this a correct reading? If so, I disagree; I don't see what
> kdump propagate has to do with whether or not an attacker impersonating
> the kdump server is detected in such a way as to stop the core file from
> being copied. At best, kdump propagate can refuse to copy an identity
> file if an attacker impersonates the kdump server at the time that the
> file is propagated. That will only happen in the case when the kdump
> server's host key is known ahead of time, and in any case is not a
> complete solution, since a host key mismatch does not stop a core file
> from being copied, so an attacker who impersonates a kdump server
> *after* the identity file is propagated will still receive cores.
> 

Yeah, seems you are right, will removing StrictHostKeyChecking=no cure the problem here?

> All things considered, I'd prefer my solution of explicitly naming the
> key as a configuration options, which eliminates the guessing. That
> said, I understand the regression that that change introduces. Note that
> kdump propagate, by default, generates a key called
> /root/.ssh/kdump_id_rsa, and that the name and location of this key
> aren't configurable by the end user. So one possible way of addressing
> the regression is to introduce an explicitly specified key in
> kdump.conf, but make it have a default value of /root/.ssh/kdump_id_rsa,
> which should account for users of previous versions who haven't changed
> the name of the kdump key.

Totally agreed.

Thank you for looking into this, Amerigo.

(In reply to comment #15)
> (In reply to comment #12)
> Jan, generally both patches look good, but one problem is that, can we have a
> default value for "sshkey"? Because it is a new config which users may not
> notice.

I would say it should be OK to implement this change under assumption it
is properly documented (even with the purpose / reasons behind this change)
on appropriate places [in kdump.conf manual page and also mentioning the
change in the Changelog].

Assuming Kevan already covered concern / question of preserving backward
compatibility (in other words):

"So one possible way of addressing the regression is to introduce an
explicitly specified key in kdump.conf, but make it have a default
value of /root/.ssh/kdump_id_rsa, which should account for users of
previous versions who haven't changed the name of the kdump key."

right? Or are we still afraid something could broke due this change?

> 
> Thanks.

(In reply to comment #16)
> (In reply to comment #11)
> > > > "True, kdump wants to submit the core _without_ interactions, but, it _does_
> > > > interactions when you run 'service kdump propagate' before 'service kdump
> > > > start' which is required when you use ssh dump."
> > > > 
> > > > So the issue you are experiencing may be result of improper using of the kdump
> > > > service. The points we are trying to solve with them right now is if:
> > > > i)  this is properly documented somewhere,
> > > > ii) if it would be possible to reject the attempt to send the core file in
> > > >     the scenario, when "service kdump start" was called, but "service kdump
> > > >     propagate" was not called earlier.
> 
> 
> I believe you would see a failure if you didn't run "service kdump propagate"
> first.

So is there some way how to block copying of core file in the scenario
Kevan suggests above, i.e. the host's key is mismatched after the identity
file was propagated? See also my reply under next paragraph.

> 
> > I read this as "the core file will not be sent to an attacker
> > impersonating the kdump target if kdump propagate is run before kdump
> > start". Is this a correct reading? If so, I disagree; I don't see what
> > kdump propagate has to do with whether or not an attacker impersonating
> > the kdump server is detected in such a way as to stop the core file from
> > being copied. At best, kdump propagate can refuse to copy an identity
> > file if an attacker impersonates the kdump server at the time that the
> > file is propagated. That will only happen in the case when the kdump
> > server's host key is known ahead of time, and in any case is not a
> > complete solution, since a host key mismatch does not stop a core file
> > from being copied, so an attacker who impersonates a kdump server
> > *after* the identity file is propagated will still receive cores.
> > 
> 
> Yeah, seems you are right, will removing StrictHostKeyChecking=no cure the
> problem here?

Amerigo, you earlier mentioned there isn't a specific reason why we would
want to add unknown hosts keys by default (maybe I just overlooked something).
But if there isn't such a reason, why not to change the "StrictHostKeyChecking"
value to "ask" (which according to ssh_config manual page would ensure
user is prompted for confirmation if there was host's key mismatch in between).

One of Python Zen rules (http://www.python.org/dev/peps/pep-0020/):
"In the face of ambiguity, refuse the temptation to guess."

So why not to let the final decision if copy the core file or not to the
end user?

> 
> > All things considered, I'd prefer my solution of explicitly naming the
> > key as a configuration options, which eliminates the guessing. That
> > said, I understand the regression that that change introduces. Note that
> > kdump propagate, by default, generates a key called
> > /root/.ssh/kdump_id_rsa, and that the name and location of this key
> > aren't configurable by the end user. So one possible way of addressing
> > the regression is to introduce an explicitly specified key in
> > kdump.conf, but make it have a default value of /root/.ssh/kdump_id_rsa,
> > which should account for users of previous versions who haven't changed
> > the name of the kdump key.
> 
> Totally agreed.

Ok, thank you. To summary:
==========================
* we right now need to decide how to deal with 'unintended / unathorized
  copying of core files to remote fake hosts' issue yet,
* create patch covering all of the above issues
* then assign CVE id / ids to these issues and create an advisory.

(In reply to comment #17)
The problem is that, previously users don't have sshkey in their kdump.conf, if we added it, their configurations will be broken. This seems rude.

(In reply to comment #18)
> Amerigo, you earlier mentioned there isn't a specific reason why we would
> want to add unknown hosts keys by default (maybe I just overlooked something).
> But if there isn't such a reason, why not to change the "StrictHostKeyChecking"
> value to "ask" (which according to ssh_config manual page would ensure
> user is prompted for confirmation if there was host's key mismatch in between).
...
> 
> So why not to let the final decision if copy the core file or not to the
> end user?

When will it prompt if we change to StrictHostKeyChecking=ask? I think it will be right before copying the vmcore?? If so, this is not acceptable, most users want to get the vmcore automatically, without any interactions.
Or I miss something?

Hi Amerigo,

  have you read my reply from https://bugzilla.redhat.com/show_bug.cgi?id=716439#c17 ?

(In reply to comment #20)
> (In reply to comment #17)
> The problem is that, previously users don't have sshkey in their kdump.conf, if
> we added it, their configurations will be broken. This seems rude.

Would the issue (break out of backward compatibility) still be here even if we
would use the default value of /root/.ssh/kdump_id_rsa for the new option?

Regarding documenting the change. As noted in #c17 we could describe the
necessity of this change in manual page and in Changelog, so it would be
better recognized.

(In reply to comment #21)
> (In reply to comment #18)
> > Amerigo, you earlier mentioned there isn't a specific reason why we would
> > want to add unknown hosts keys by default (maybe I just overlooked something).
> > But if there isn't such a reason, why not to change the "StrictHostKeyChecking"
> > value to "ask" (which according to ssh_config manual page would ensure
> > user is prompted for confirmation if there was host's key mismatch in between).
> ...
> > 
> > So why not to let the final decision if copy the core file or not to the
> > end user?
> 
> When will it prompt if we change to StrictHostKeyChecking=ask? I think it will
> be right before copying the vmcore?? If so, this is not acceptable, most users
> want to get the vmcore automatically, without any interactions.
> Or I miss something?

Yes, the expected moment of user interaction is prior submission of the kdump
core file to remote host (which would prevent unintended core dump submission,
when not desired, IOW in case the particular host's key got mismatched in between). 

Ad "most users want to get the vmcore automatically, without any interactions"
it depends. One more prompt screen can be viewed as pestering the user, but
I think if this would help preventing the kdump core file information leak
(it's submission to completely different host than originally intended) 
assuming this as being reasonable compromise.

Due #c22 & #c23. Amerigo, could you comment on those? Thanks, Jan.

In my use case, the copy process needs to happen without human intervention. The utility of kdump for my site is significantly reduced if a human needs to be present whenever a system panics in order for the vmcore to be copied, since we can't guarantee that for all situations (e.g., panics in the middle of the night). I guess that's -1 from me on requiring human intervention at copy time.

In normal operation, wouldn't we know the kdump destination's host key fingerprint by the end of the kdump propagate step? Why not use StrictHostKeyChecking=yes for all of the steps after the kdump propagate step, and then defer to the user's StrictHostKeyChecking setting (by not specifying an -o StrictHostKeyChecking argument) for kdump propagate? That pushes the user confirmation step back to an operation in which a user might be present, performs the confirmation step in accordance with the user's configured wishes, and remembers the results of the step for future interactions.

(In reply to comment #22)
> Would the issue (break out of backward compatibility) still be here even if we
> would use the default value of /root/.ssh/kdump_id_rsa for the new option?

No, that is why I insist we should have a default. :)

(In reply to comment #23)
> Ad "most users want to get the vmcore automatically, without any interactions"
> it depends. One more prompt screen can be viewed as pestering the user, but
> I think if this would help preventing the kdump core file information leak
> (it's submission to completely different host than originally intended) 
> assuming this as being reasonable compromise.

Yeah, I do understand the reason behind, but the fact is that from the beginning we always dump the core without interactions, if we changed the behaviour, users would complain.

(In reply to comment #26)
> (In reply to comment #22)
> > Would the issue (break out of backward compatibility) still be here even if we
> > would use the default value of /root/.ssh/kdump_id_rsa for the new option?
> 
> No, that is why I insist we should have a default. :)

Ok, so looks we agreed on this one already (use default value / location
and document the purpose of the new configuration file option in manual page &
changelog).

(In reply to comment #27)
> (In reply to comment #23)
> > Ad "most users want to get the vmcore automatically, without any interactions"
> > it depends. One more prompt screen can be viewed as pestering the user, but
> > I think if this would help preventing the kdump core file information leak
> > (it's submission to completely different host than originally intended) 
> > assuming this as being reasonable compromise.
> 
> Yeah, I do understand the reason behind, but the fact is that from the
> beginning we always dump the core without interactions, if we changed the
> behaviour, users would complain.

Ah, this is now more clear in context of Kevan's comment:
https://bugzilla.redhat.com/show_bug.cgi?id=716439#c25

Will follow up on that one.

Hi Kevan,

  thanks for your reaction.

(In reply to comment #25)
> In my use case, the copy process needs to happen without human intervention.
> The utility of kdump for my site is significantly reduced if a human needs to
> be present whenever a system panics in order for the vmcore to be copied, since
> we can't guarantee that for all situations (e.g., panics in the middle of the
> night). I guess that's -1 from me on requiring human intervention at copy time.
> 

Ok, understandable now why we might want to avoid human confirmation at copy time.

> In normal operation, wouldn't we know the kdump destination's host key
> fingerprint by the end of the kdump propagate step? Why not use
> StrictHostKeyChecking=yes for all of the steps after the kdump propagate step,
> and then defer to the user's StrictHostKeyChecking setting (by not specifying
> an -o StrictHostKeyChecking argument) for kdump propagate? That pushes the user
> confirmation step back to an operation in which a user might be present,
> performs the confirmation step in accordance with the user's configured wishes,
> and remembers the results of the step for future interactions.

Looked again at your "mkdumprd keyhandling patch" proposal:
https://bugzilla.redhat.com/attachment.cgi?id=510068&action=diff

I can see on couple of places your patch is removing "-o StrictHostKeyChecking=no". Ad "Why not use StrictHostKeyChecking=yes for all of the steps after the kdump propagate step" -- could you clarify, which exact
code part / section you meaning right now?

Also, ad "in accordance with the user's configured wishes, and remembers the results of the step for future interactions." -- not sure I agree here. Have
you meant future interactions for the same host here? Or future interactions
at common level? And if using this scenario, wouldn't be possible still to
bypass the 'saved future interactions scenario' by key mismatch? (probably
not, since we would be using -o StrictHostChecking=yes by default. But still,
couldn't there be any other issues with 'saved future interactions mechanism'?)

(In reply to comment #30) 
> Looked again at your "mkdumprd keyhandling patch" proposal:
> https://bugzilla.redhat.com/attachment.cgi?id=510068&action=diff
> 
> I can see on couple of places your patch is removing "-o
> StrictHostKeyChecking=no". Ad "Why not use StrictHostKeyChecking=yes for all of
> the steps after the kdump propagate step" -- could you clarify, which exact
> code part / section you meaning right now?

The steps either performed directly in mkdumprd or written to the kdump initrd for performance when the vmcore is copied. In other words, every ssh interaction that does not occur within the kdump initscript. Note that I do not specify -o StrictHostKeyChecking=yes in my patches. In the ssh configuration on my hosts, StrictHostKeyChecking=yes is used for all ssh operations, so there seemed to be little point in overriding that in kdump. Arguably kdump should avoid overriding the StrictHostKeyChecking setting in general, but StrictHostKeyChecking=yes is no less strict then what the user can configure, may possibly be safer than what the user has configured, and should work fine for kdump's use case.

> Also, ad "in accordance with the user's configured wishes, and remembers the
> results of the step for future interactions." -- not sure I agree here. Have
> you meant future interactions for the same host here? Or future interactions
> at common level? And if using this scenario, wouldn't be possible still to
> bypass the 'saved future interactions scenario' by key mismatch? (probably
> not, since we would be using -o StrictHostChecking=yes by default. But still,
> couldn't there be any other issues with 'saved future interactions mechanism'?)

I mean future interactions with the same host. Specifically, I think kdump should use ssh in such a way as to cause a user to confirm at most once that a host key is appropriate for the kdump destination. After that, kdump should work without confirmation if the kdump destination's host key is unchanged, and should not work at all in the event of a key mismatch. Essentially, we ask the user to say that a host key fingerprint belongs to the kdump destination, and then expect the kdump server to have the host key fingerprint that the user says it should for all future interactions, failing if it doesn't.

Did you have any other issues in mind? Maybe I've missed something.

(In reply to comment #31)
> (In reply to comment #30) 
> > Looked again at your "mkdumprd keyhandling patch" proposal:
> > https://bugzilla.redhat.com/attachment.cgi?id=510068&action=diff
> > 
> > I can see on couple of places your patch is removing "-o
> > StrictHostKeyChecking=no". Ad "Why not use StrictHostKeyChecking=yes for all of
> > the steps after the kdump propagate step" -- could you clarify, which exact
> > code part / section you meaning right now?
> 
> The steps either performed directly in mkdumprd or written to the kdump initrd
> for performance when the vmcore is copied. In other words, every ssh
> interaction that does not occur within the kdump initscript. Note that I do not
> specify -o StrictHostKeyChecking=yes in my patches. In the ssh configuration on
> my hosts, StrictHostKeyChecking=yes is used for all ssh operations, so there
> seemed to be little point in overriding that in kdump. Arguably kdump should
> avoid overriding the StrictHostKeyChecking setting in general, but
> StrictHostKeyChecking=yes is no less strict then what the user can configure,
> may possibly be safer than what the user has configured, and should work fine
> for kdump's use case.

IOW you propose just to remove -o StrictHostKeyChecking=no from the kdump scripts, so the user could decide, which level of security they want (i.e. the ScriptHostChecking value would be taken from system's sshd configuration file).

This looks reasonable to me.

Amerigo, what do you think? Do you agree with the removal and if not, have other possibility of fixing this in mind?

Amerigo, also would some changes to proposed patches above be necessary due the summary in this (and above comments), or would those be sufficient? Thanks, Jan.

> 
> > Also, ad "in accordance with the user's configured wishes, and remembers the
> > results of the step for future interactions." -- not sure I agree here. Have
> > you meant future interactions for the same host here? Or future interactions
> > at common level? And if using this scenario, wouldn't be possible still to
> > bypass the 'saved future interactions scenario' by key mismatch? (probably
> > not, since we would be using -o StrictHostChecking=yes by default. But still,
> > couldn't there be any other issues with 'saved future interactions mechanism'?)
> 
> I mean future interactions with the same host. Specifically, I think kdump
> should use ssh in such a way as to cause a user to confirm at most once that a
> host key is appropriate for the kdump destination. After that, kdump should
> work without confirmation if the kdump destination's host key is unchanged, and
> should not work at all in the event of a key mismatch. Essentially, we ask the
> user to say that a host key fingerprint belongs to the kdump destination, and
> then expect the kdump server to have the host key fingerprint that the user
> says it should for all future interactions, failing if it doesn't.
> 
> Did you have any other issues in mind?

Not really. Was only wondering how the host key mismatch case would be handled in this case.

> Maybe I've missed something.

(In reply to comment #32)
> IOW you propose just to remove -o StrictHostKeyChecking=no from the kdump
> scripts, so the user could decide, which level of security they want (i.e. the
> ScriptHostChecking value would be taken from system's sshd configuration file).
> 
> This looks reasonable to me.
> 
> Amerigo, what do you think? Do you agree with the removal and if not, have
> other possibility of fixing this in mind?

What is the default value for StrictHostKeyChecking? I assume it's "yes"? If so, scp will just fail if the key mismatches? If this is true, I think removing it is fine.

(In reply to comment #33)
> (In reply to comment #32)
> > IOW you propose just to remove -o StrictHostKeyChecking=no from the kdump
> > scripts, so the user could decide, which level of security they want (i.e. the
> > ScriptHostChecking value would be taken from system's sshd configuration file).
> > 
> > This looks reasonable to me.
> > 
> > Amerigo, what do you think? Do you agree with the removal and if not, have
> > other possibility of fixing this in mind?
> 
> What is the default value for StrictHostKeyChecking?

It's 'ask', thus:

"If this flag is set to “ask”, new host keys will be added to the user known host files only after the user has confirmed that is what they really want to do, and ssh will refuse to connect to hosts whose host key has changed.  The host keys of known hosts will be verified automatically in all cases."

which would address the issue (the unintended kdump core file propagation, when host key changed). 

> I assume it's "yes"? If
> so, scp will just fail if the key mismatches? If this is true, I think removing
> it is fine.

Amerigo, Kevan, since it not being 'yes', but rather 'ask' would this be acceptable? If so, I would allocate CVE ids to the flaws and would create advisory template for this, so we could address these.

Thanks, Jan.

StrictHostKeyChecking=ask seems to behave like StrictHostKeyChecking=yes when used in conjunction with BatchMode=yes, so they're basically equivalent as far as kdump's use of ssh is concerned. So a default of 'ask' seems fine to me.

Thanks, Jan and Kevan!
It is okay as long as it doesn't break the rule that we always dump vmcore without any interactions.

If I'm not mistaken, we've resolved the design issues associated with this problem. Can either of you give me a rough idea of when the fix might be pushed out, or when the advisory might be released?

Thanks.

Kevan, we don't need to update the above patches? I am waiting for either of you to update it. :)

The patches above don't specify a default sshkey value, and they don't do a good job of solving the kernel image permissions issue (in that they chmod after the kernel image is written, leaving a brief window in which the kernel image can possibly be read by anyone). They also don't touch the manpage or Changelog. I think they do everything else that we ended up agreeing on. 

Who will update the patches? I don't know when I'll have time to update them; they work fine for my installation, so they're rather low on my priority list. I can review updates made by other people, if that helps.

Kevan, I will try to update it next Monday, if Jan doesn't have time.
Thanks!

Created attachment 517557 [details]
Updated patch

Kevan, please review the updated patch.

Your original patches have some conflicts, I applied it manually. Please check if I miss something.

I have tested it, I got the vmcore successfully via ssh.

Thanks.

Brew build,
https://brewweb.devel.redhat.com/taskinfo?taskID=3545551

In addition to the description of sshkey in the kdump.conf manpage, you could also add a commented out sshkey directive to kdump.conf so folks who don't read the manpage know it exists. Not essential, but it's done for other configuration options.

The patch looks good to me, otherwise. Thanks for updating it. Can you say if this will be backported to RHEL5, or is that someone else's department?

RHEL6 has the same problem.

Great, thanks for that confirmation.

It seems we would need three CVE ids in this case:

1. Use  StrictHostKeyChecking in kdump/mkdumprd, when transffering the crash
image to a remote kdump server. This will make sure that a remote malicious
kdump server cannot impersonate the intended, correct kdump server to obtain
the kdump core files.

2. kdump/mkdumprd creates vmcore files which are world-readable. A
non-previldged user can easily access this file to extract important
information like ssh keys used by the root user.

3. kdump/mkdumprd copies all the .ssh keys of root user on the vmcore file.
This may include keys which are not-required and may be confidential to the
root user also.

Kevan,

Hi, Sorry for the long silence on this issue. Jan was on PTO for some time and then feel sick, therefore he could not look at this one.

I have been looking at this bug and suggested in comment #56 that we get 3 CVE ids for this issue. We will also look into fixing this issue at the earliest.

Amerigo,

Hi, looking at your patch (comment #41), there is a change in the behaviour of mkdumprd in which instead of copying over the entire .ssh directory into the vmcore image, we are copying only the ssh-key required for the ssh communication between the kdump host and the server.

The default key is assumed to be kdump_id_rsa (or any thing else as defined by "sshkey" directive in the patched version). Would all previous users of kexec also have their kexec ssh-keys to be kdump_id_rsa. I am concerned about breaking existing configs for our users.

Hello, Huzaifa,

Previous users which don't have "sshkey" specified will not notice this change, the default kdump_id_rsa will be used.

We have assigned three CVE ids to these issues


1. CVE-2011-3588

> 1. Use  StrictHostKeyChecking in kdump/mkdumprd, when transffering the crash
> image to a remote kdump server. This will make sure that a remote malicious
> kdump server cannot impersonate the intended, correct kdump server to obtain
> the kdump core files.
> 

2. CVE-2011-3589

> 2. kdump/mkdumprd creates vmcore files which are world-readable. A
> non-previldged user can easily access this file to extract important
> information like ssh keys used by the root user.
> 

3. CVE-2011-3590

> 3. kdump/mkdumprd copies all the .ssh keys of root user on the vmcore file.
> This may include keys which are not-required and may be confidential to the
> root user also.

Hi Kevan,

We intend to fix this issue in rhel-5 and rhel-6 soon. We would also like to fix the fedora packages as well. Are you ok with us making this bug open?

(This is currently an embargoed issue/bug, meaning not open to general public view)

That's fine with me.

Created kexec-tools tracking bugs for this issue

Affects: fedora-all [bug 743474]

An incorrect CVE identifier of CVE-2011-2390 has been also used:
[1] http://www.openwall.com/lists/oss-security/2011/10/05/1

to reference the third issue. This is an incorrect one and CVE-2011-3590:
[2] https://bugzilla.redhat.com/show_bug.cgi?id=716439#c61

should be used for references of the following kexec-tools issue:

"3. kdump/mkdumprd copies all the .ssh keys of root user on the vmcore file.
This may include keys which are not-required and may be confidential to the
root user also."

[3] http://www.openwall.com/lists/oss-security/2011/10/10/1

This issue has been addressed in following products:

  Red Hat Enterprise Linux 6

Via RHSA-2011:1532 https://rhn.redhat.com/errata/RHSA-2011-1532.html

This issue has been addressed in following products:

  Red Hat Enterprise Linux 5

Via RHSA-2012:0152 https://rhn.redhat.com/errata/RHSA-2012-0152.html