Add Tencent cloud support

[jwang11]

Enable ucd to support Tencent Cloud environment where meta-data service is at metadata.tencentyun.com, but not openstack style 169.254.169.254 This commit involves creating a tencent user and provisioning the ssh public key.

[ahkok]

Wow, so, no, I really do not want this. This is very insecure.

There is NO guarantee that a HTTP (non-HTTPS) request to a domain name is resolvable securely and reasonably safe to even talk to.

This is really bad security. Local Link addresses are used by openstack for a reason, because it's makes sure that at least there is a reasonable level of trust that you're not talking to a random attacker.

If this has to come from a named host, I would certainly require HTTPS to be used. If they can't offer HTTPS, I would not want to ever use an SSH key provided by this service since it can absolutely not be trusted.

[jwang11]

Ok, so I'll ask Tencent to provide a fix IP for meta-data service

[ahkok]

I think that's the proper way to a resolution.

[ahkok]

Note, if it's not a Link Local IP address, the same concern applies. Either the address needs to be Link Local or this should use TLS/SSL instead.

[jwang11]

Tencent promise metadata.tencentyun.com mapped to fixed 169.254.0.23, which should be Link Local

[ahkok]

But the domain name is NOT guaranteed local link.

You can send a patch with 169.254.0.23 as the host name, but not with a hostname that may resolve to that IP address.

At least then there's a reasonable guard against an MITM attack.

:)

[ahkok]

Ok, that is now reasonable.

Please leave this PR open as is, I will be making some changes to the project this week and incorporating the functionality you need for it...

[ahkok]

Ok, so, please have a look at this instead:

https://github.com/clearlinux/micro-config-drive/commit/31e30b5b295086c788ce23dbff1dcdf06a24beba

This is on a branch - it's not 100% ready yet, but instead of duplicating all the code again, this makes it manageable and easy to understand what is going on.

We'll need testing obviously, and make sure it doesn't break AWS and OCI at the same time.

[jwang11]

The code re-org make sense to me. I'll test it in Tencent cloud meanwhile.

[ahkok]

31e30b5b295086c788ce23dbff1dcdf06a24beba 01d4102d9932f215150703a54b6a7ab85d6d7c79

[ahkok]

@jwang11 did you verify this functions properly?

[jwang11]

There is a problem in below code when parsing Tencent opensshkey. Tencent sshkey include only one line with EOF ending, but no \r\n. Thus, fgets return NULL although buf already get correct sshkey, and break from loop.

    /* Insert cloud-config header above SSH key. */
    len = strlen(config[conf].cloud_config_header);
    write(out, config[conf].cloud_config_header, len);

    for (;;) {
        if (cl == 0) {
            break;
        }
        char buf[512];
        char *r = fgets(buf, sizeof(buf), f);
        size_t len = strlen(buf);
        cl -= (long int)len;
        if (r == 0) {
            break;
        }
        if (write(out, buf, len) < (ssize_t)len) {

Suggest check by

if (r == 0 && len ==0)

[ahkok]

On 8/15/19 1:22 AM, Jing Wang wrote:

There is a problem in below code when parsing Tencent opensshkey. Tencent sshkey include only one line with EOF ending, but no \r\n. Thus, fgets return NULL although buf already get correct sshkey. Suggest check len == 0 along with r == 0.

That doesn't compute:

RETURN VALUE fgets() returns s on success, and NULL on error or when end of file oc‐curs while no characters have been read.

in other words, on the first pass through the loop, it will get EOF, but it will read characters, thus it will return *s (buf) and not NULL.

In the second pass, it will return NULL immediately without anything in buf.

did you try print out e.g. fprintf(stderr, "len: %d\n", len)

if len == 0, then by definition buf should be \0 too?

This code is definitely shaky, yes, it certainly needs some adjustment, but, I'm not sure your reading is correct either.

Auke

[jwang11]

The way I located the code is actually based on result from Tencent cloud debugging. I found the execution path break unexpectedly as below in the first loop, thus buf is not written actually.

        if (r == 0) {
            break;
        }

I embedded some debug code

        char *r = fgets(buf, sizeof(buf), f);
        size_t len = strlen(buf);
        printf("len=%lu\n", len);
        cl -= (long int)len;
        if (r == 0) {
            printf("buf=%s\n", buf);
            break;
        }

output confirm my guess,

len=224
buf=ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDzytTP0KRIGviHqsoclofywBHtBpSDAuCVkqvei9aos2qLJfXgtBAlwrpabvF8h5vtzYcpUX6DylwbQDpKC9b3TGbFL3niLV0uz2L+RDJrp
d8ZwpjKYU6vPNe8mOoNlAlSEPUyT6t5c03cXOjYG5KkLJdDIG+6otlrxIIzAnlW5w== skey_435993
[57998.307989] micro-config-drive version: 36
[57998.308886] userdata: Looking for shebang file /var/lib/cloud/tencent-user-data

[ahkok]

Yeah, I'm not doubting the outcome of your experiment, but it completely doesn't match with the manual of fgets and therefore we are both not properly understanding the circumstances and outcome.

I created a simple test program like this:

#include <stdio.h>
#include <string.h>

int main()
{
    FILE *f = fopen("testfile", "r");
    char buf[512];
    char *c = fgets(buf, sizeof(buf), f);
    printf("%d %s %lu\n", (char)*c, buf, strlen(buf));
}

If I compile this, and feed it various data, I get the following:

1. printf "" > testfile result: 0 �/�� 6 (NULL returned, data is garbage, strlen is random) -> fgets returns NULL according to man page.
2. printf "ABC" > testfile result: -96 ABC 3 (no newline, data is 3 bytes, strlen is 3) -> fgets returns pointer to buf despite no newline.
3. printf "ABC\n123" > testfile -> fgets returns pointer to buf with newline. result: -112 ABC 4 (nothing weird here).

In other words, the manual of fgets page appears to be correct.

I'm not disputing your results, but, something else is going on here and I can't wrap my head around it, and making an uneducated change that doesn't match with the documentation isn't my style.

Is there more output than you pasted here?

[ahkok]

I'm thinking we're going completely wrong about this problem anyway. We should assume that we need to write the whole content verbatim anyway, instead of parsing things line by line. There's some major terrible handling and useless code in this section anyway, so maybe I'll knock it all over.

[ahkok]

Try d2451d2 - this uses the old OCI method which was needed because I think we ran into a similar problem with their data service (somehow, it still confuses me). This doesn't use fgets and may just be the solution here, and I think we'll need it anyway given past experiences with OCI.