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

ENT-10347: Fixed changing perms noise in no-noise sequential test #5386

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

larsewi
Copy link
Contributor

@larsewi larsewi commented Nov 27, 2023

Happens on Debian 12:

info: Socket "/var/cfengine/state/./cf-execd.sockets/runagent.socket" had permissions 0700, changed it to 0600
info: Directory "/var/cfengine/state/./cf-execd.sockets" had permissions 0750, changed it to 0700

Merge together with:

@larsewi larsewi changed the title Fixed changing perms noise in no-noise sequential test ENT-10347: Fixed changing perms noise in no-noise sequential test Nov 27, 2023
@larsewi larsewi marked this pull request as draft November 27, 2023 14:05
@larsewi
Copy link
Contributor Author

larsewi commented Nov 27, 2023

@cf-bottom Jenkins please :)

@cf-bottom
Copy link

cf-bottom commented Nov 27, 2023

cf-execd/cf-execd.c Outdated Show resolved Hide resolved
@larsewi larsewi force-pushed the deb12 branch 2 times, most recently from d3c5ee9 to 2fea659 Compare December 1, 2023 09:05
@@ -667,6 +655,7 @@ static int SetupRunagentSocket(const ExecdConfig *execd_config)
}
else
{
safe_chmod(sock_info.sun_path, (mode_t) 0600);
Copy link
Contributor

@vpodzime vpodzime Dec 1, 2023

Choose a reason for hiding this comment

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

I think fchmod(runagent_socket,...) would be better.

Copy link
Contributor Author

@larsewi larsewi Dec 4, 2023

Choose a reason for hiding this comment

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

fchmod() did not seem to work. Tried locally with both fchmod() and chmod() using this little program:

#include <stdio.h>
#include <stdlib.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <assert.h>
#include <string.h>
#include <unistd.h>

int main(void) {
    const char *path = "bogus.sock";
    int sock = socket(AF_LOCAL, SOCK_STREAM, 0);
    assert(sock >= 0);
    struct sockaddr_un sock_info;
    memset(&sock_info, 0, sizeof(struct sockaddr_un));
    strncpy(sock_info.sun_path, path, sizeof(sock_info.sun_path));
    sock_info.sun_family = AF_LOCAL;
    unlink(path);
    int ret = bind(sock, (const struct sockaddr *)&sock_info, sizeof(sock_info));
    assert(ret == 0);
    chmod(path, (mode_t) 0600);
    close(sock);
    return 0;
}

Only the latter one actually changed the permissions. Thus I'm changing it back to use safe_chmod().

@larsewi
Copy link
Contributor Author

larsewi commented Dec 1, 2023

@cf-bottom Jenkins please :)

@cf-bottom
Copy link

@larsewi larsewi requested a review from vpodzime December 1, 2023 14:44
@larsewi larsewi marked this pull request as ready for review December 1, 2023 14:44
craigcomstock
craigcomstock previously approved these changes Dec 1, 2023
vpodzime
vpodzime previously approved these changes Dec 4, 2023
Copy link
Contributor

@vpodzime vpodzime left a comment

Choose a reason for hiding this comment

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

ACK

Happens on Debian 12:
```
info: Socket "/var/cfengine/state/./cf-execd.sockets/runagent.socket" had permissions 0700, changed it to 0600
info: Directory "/var/cfengine/state/./cf-execd.sockets" had permissions 0750, changed it to 0700
```

Ticket: ENT-10347
Changelog: None
Signed-off-by: Lars Erik Wik <[email protected]>
@larsewi
Copy link
Contributor Author

larsewi commented Dec 4, 2023

@cf-bottom jenkins please :)

@cf-bottom
Copy link

@larsewi larsewi requested a review from vpodzime December 4, 2023 11:53
@larsewi
Copy link
Contributor Author

larsewi commented Dec 4, 2023

^ Only known FR tests failures

@larsewi larsewi merged commit 574818c into cfengine:master Dec 4, 2023
48 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants