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

[juju] Add /etc/juju to collection #770

Closed
wants to merge 1 commit into from

Conversation

freyes
Copy link
Contributor

@freyes freyes commented Feb 18, 2016

Since juju-core 1.25.0 /etc/juju is used to store certificates,
whithout this patch just the old certificates kept under /var/lib/juju
are collected.

Signed-off-by: Felipe Reyes felipe.reyes@canonical.com

Since juju-core 1.25.0 /etc/juju is used to store certificates,
whithout this patch just the old certificates kept under /var/lib/juju
are collected.

Signed-off-by: Felipe Reyes <felipe.reyes@canonical.com>
@adam-stokes
Copy link

ACK from me, waiting on second ack from @bmr-cymru or others who've more familiarity with juju. Maybe @karibou ?

@bmr-cymru
Copy link
Member

I'm not familiar enough with Juju to review this - the main general concern with this type of change though is that the path is used to store certificates. We need to know if any of these are secret and if so exclude them from the set of collected files.

@karibou
Copy link
Contributor

karibou commented Feb 19, 2016

Hi,

Le 19/02/2016 10:25, Bryn M. Reeves a écrit :

I'm not familiar enough with Juju to review this - the main general concern with
this type of change though is that the path is used to store certificates. We
need to know if any of these are secret and if so exclude them from the set of
collected files.


Reply to this email directly or view it on GitHub
#770 (comment).

I'll try to give it a look.

Kind regards,

...Louis

Louis Bouchard
Software engineer, Cloud & Sustaining eng.
Canonical Ltd
Ubuntu developer Debian Maintainer
GPG : 429D 7A3B DD05 B6F8 AF63 B9C4 8B3D 867C 823E 7A61

@karibou
Copy link
Contributor

karibou commented Feb 19, 2016

Bryn,

The cert collected is a self-signed certificate used by rsyslog for its communication. And for prior versions of the plugin, it was held in /var/lib/juju which we already collect.

On top of this, the cert itself might be required for debugging purposes.

I don't think that it poses a security issue in that context.

Kind regards,

...Louis

@bmr-cymru
Copy link
Member

That probably warrants a comment in that case then - just to document the fact that although there are certificates here that are used for authentication that they are not considered problematic.

@freyes
Copy link
Contributor Author

freyes commented Feb 19, 2016

On Fri, 19 Feb 2016 05:33:21 -0800
"Bryn M. Reeves" notifications@github.com wrote:

That probably warrants a comment in that case then - just to document
the fact that although there are certificates here that are used for
authentication that they are not considered problematic.

Sounds good to me, I'll add the comment and refresh the patch.

@bmr-cymru
Copy link
Member

👍

@adam-stokes
Copy link

@freyes sorry, did you get a chance to update the patch yet?

@bmr-cymru
Copy link
Member

Ack from me subject to the updated patch - would be good to have this in 3.3 so needs to be asap (or one of us can respin the patch).

@adam-stokes
Copy link

Closing this out due to lack of activity and remaining conflicts. Please feel free to open a new PR with any futures changes you'd like to see in this plugin.

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

Successfully merging this pull request may close these issues.

None yet

4 participants